Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 2 Feb 2017 18:01:59 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Alexandre Martins <alexandre.martins@stormshield.eu>
Cc:        Freebsd arm <freebsd-arm@freebsd.org>
Subject:   Re: Counter API
Message-ID:  <20170202160159.GN2092@kib.kiev.ua>
In-Reply-To: <2478341.4YUvIvO0Dr@pc-alex>
References:  <2478341.4YUvIvO0Dr@pc-alex>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Feb 02, 2017 at 04:38:08PM +0100, Alexandre Martins wrote:
> Hi all,
> 
> Our companie is curently tracking performance issue with our ARM product.
> 
> We have seen that counter API was migrated from intrinsic to atomic.
> 
> https://svnweb.freebsd.org/base?view=revision&revision=269406
> 
> But there is still the calls to critical_enter/leave in the macro 
> counter_enter/leave.
> 
> Is that needed ? Can we remove that ?

Try this.  I did not even compiled the patch.

diff --git a/sys/arm/include/counter.h b/sys/arm/include/counter.h
index c4da91f7a14..950516e1616 100644
--- a/sys/arm/include/counter.h
+++ b/sys/arm/include/counter.h
@@ -31,12 +31,9 @@
 
 #include <sys/pcpu.h>
 #include <machine/atomic.h>
-#ifdef INVARIANTS
-#include <sys/proc.h>
-#endif
 
-#define	counter_enter()	critical_enter()
-#define	counter_exit()	critical_exit()
+#define	counter_enter()	do {} while (0)
+#define	counter_exit()	do {} while (0)
 
 #ifdef IN_SUBR_COUNTER_C
 
@@ -55,7 +52,7 @@ counter_u64_fetch_inline(uint64_t *p)
 	int i;
 
 	r = 0;
-	for (i = 0; i < mp_ncpus; i++)
+	CPU_FOREACH(i)
 		r += counter_u64_read_one((uint64_t *)p, i);
 
 	return (r);
@@ -78,18 +75,13 @@ counter_u64_zero_inline(counter_u64_t c)
 }
 #endif
 
-#define	counter_u64_add_protected(c, inc)	do {	\
-	CRITICAL_ASSERT(curthread);			\
-	atomic_add_64((uint64_t *)zpcpu_get(c), (inc));	\
-} while (0)
+#define	counter_u64_add_protected(c, inc)	counter_u64_add(c, inc)
 
 static inline void
 counter_u64_add(counter_u64_t c, int64_t inc)
 {
 
-	counter_enter();
-	counter_u64_add_protected(c, inc);
-	counter_exit();
+	atomic_add_64((uint64_t *)zpcpu_get(c), inc);
 }
 
 #endif	/* ! __MACHINE_COUNTER_H__ */
diff --git a/sys/arm64/include/counter.h b/sys/arm64/include/counter.h
index 9d56cce3d3d..cfa521f9b73 100644
--- a/sys/arm64/include/counter.h
+++ b/sys/arm64/include/counter.h
@@ -30,12 +30,10 @@
 #define	_MACHINE_COUNTER_H_
 
 #include <sys/pcpu.h>
-#ifdef INVARIANTS
-#include <sys/proc.h>
-#endif
+#include <machine/atomic.h>
 
-#define	counter_enter()	critical_enter()
-#define	counter_exit()	critical_exit()
+#define	counter_enter()	do {} while (0)
+#define	counter_exit()	do {} while (0)
 
 #ifdef IN_SUBR_COUNTER_C
 static inline uint64_t
@@ -52,13 +50,12 @@ counter_u64_fetch_inline(uint64_t *p)
 	int i;
 
 	r = 0;
-	for (i = 0; i < mp_ncpus; i++)
+	CPU_FOREACH(i)
 		r += counter_u64_read_one((uint64_t *)p, i);
 
 	return (r);
 }
 
-/* XXXKIB might interrupt increment */
 static void
 counter_u64_zero_one_cpu(void *arg)
 {
@@ -76,18 +73,13 @@ counter_u64_zero_inline(counter_u64_t c)
 }
 #endif
 
-#define	counter_u64_add_protected(c, inc)	do {	\
-	CRITICAL_ASSERT(curthread);			\
-	*(uint64_t *)zpcpu_get(c) += (inc);		\
-} while (0)
+#define	counter_u64_add_protected(c, inc)	counter_u64_add(c, inc)
 
 static inline void
 counter_u64_add(counter_u64_t c, int64_t inc)
 {
 
-	counter_enter();
-	counter_u64_add_protected(c, inc);
-	counter_exit();
+	atomic_add_64((uint64_t *)zpcpu_get(c), inc);
 }
 
 #endif	/* ! _MACHINE_COUNTER_H_ */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170202160159.GN2092>