kern/173322: [patch] Inline atomic operations in modules

Peter Jeremy peter at rulingia.com
Sat Nov 3 06:40:02 UTC 2012 

>Number:         173322
>Category:       kern
>Synopsis:       [patch] Inline atomic operations in modules
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Sat Nov 03 06:40:00 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Peter Jeremy
>Release:        FreeBSD 8.3-STABLE amd64
>Organization:
FreeBSD
>Environment:
System: FreeBSD server.rulingia.com 8.3-STABLE FreeBSD 8.3-STABLE #18 r237444M: Sun Jul 8 10:47:08 EST 2012 root at server.rulingia.com:/var/obj/usr/src/sys/server amd64

>Description:
	In r49999, atomic operations on x86 were changed so that they
	were not inlined within kernel modules but instead generated calls
	to kernel functions.  This allowed kernel modules to run on both
	UP and SMP systems without incurring the overhead of unnecessary
	LOCK instructions, though at the expense of having call/return
	overheads.

	At the time, this seemed a reasonable tradeoff - SMP systems
	were not common and LOCK prefixes were extremely slow on some
	CPUs.

	SMP systems are now far more common - probably more common than
	UP systems on the desktop or in servers.  Whilst LOCK prefixes
	are still slow, the impact of LOCK prefixes is probably lower
	now than a decade ago.

	Overall, it would seem that the gains from inlining the atomic
	operations (saving the call/return overheads) outweigh the
	cost of unnecessary LOCK prefixes on UP systems (though this
	still needs to be measured).

	Note that all architectures except for x86 always inline
	atomic operations and do not provide callable versions.

>How-To-Repeat:
	Code inspection.

>Fix:
	This patch changes i386 and amd64 atomic.h to provide inline
	versions of atomic operations when invoked to build KLDs.
	It retains the provision to create non-inlined versions of
	the atomic operations to support existing KLDs.  (My suggestion
	is that, if this change is worthwhile, this change be MFC'd
	then the head code be updated to remove the non-inlined
	atomic operations and that change not be MFC'd - which would
	align x86 with all other architectures).  Note that this patch
	has been compile tested (and the resultant object code quickly
	sanity checked), it has not been tested yet.

Index: head/sys/amd64/amd64/atomic.c
===================================================================
--- head/sys/amd64/amd64/atomic.c	(revision 242498)
+++ head/sys/amd64/amd64/atomic.c	(working copy)
@@ -33,11 +33,11 @@
  */
 #include <sys/types.h>
 
-/* Firstly make atomic.h generate prototypes as it will for kernel modules */
-#define KLD_MODULE
+/* Firstly make atomic.h generate prototypes */
+#define WANT_PROTOTYPES
 #include <machine/atomic.h>
 #undef _MACHINE_ATOMIC_H_	/* forget we included it */
-#undef KLD_MODULE
+#undef WANT_PROTOTYPES
 #undef ATOMIC_ASM
 
 /* Make atomic.h generate public functions */
Index: head/sys/amd64/include/atomic.h
===================================================================
--- head/sys/amd64/include/atomic.h	(revision 242498)
+++ head/sys/amd64/include/atomic.h	(working copy)
@@ -64,14 +64,13 @@
  */
 
 /*
- * The above functions are expanded inline in the statically-linked
- * kernel.  Lock prefixes are generated if an SMP kernel is being
- * built.
- *
- * Kernel modules call real functions which are built into the kernel.
- * This allows kernel modules to be portable between UP and SMP systems.
+ * The above functions are all expanded inline if the compiler
+ * supports the gcc extensions to asm().  Lock prefixes are generated
+ * except for UP kernels (ie kernel modules and userland references
+ * always have lock prefixes).  This allows kernel modules and
+ * userland code to be portable between UP and SMP systems.
  */
-#if defined(KLD_MODULE) || !defined(__GNUCLIKE_ASM)
+#if defined(WANT_PROTOTYPES) || !defined(__GNUCLIKE_ASM)
 #define	ATOMIC_ASM(NAME, TYPE, OP, CONS, V)			\
 void atomic_##NAME##_##TYPE(volatile u_##TYPE *p, u_##TYPE v);	\
 void atomic_##NAME##_barr_##TYPE(volatile u_##TYPE *p, u_##TYPE v)
@@ -86,13 +85,13 @@
 #define	ATOMIC_STORE(TYPE)					\
 void		atomic_store_rel_##TYPE(volatile u_##TYPE *p, u_##TYPE v)
 
-#else /* !KLD_MODULE && __GNUCLIKE_ASM */
+#else /* !WANT_PROTOTYPES && __GNUCLIKE_ASM */
 
 /*
- * For userland, always use lock prefixes so that the binaries will run
- * on both SMP and !SMP systems.
+ * For userland and kernel modules, always use lock prefixes so that
+ * the binaries will run on both SMP and !SMP systems.
  */
-#if defined(SMP) || !defined(_KERNEL)
+#if defined(SMP) || !defined(_KERNEL) || defined(KLD_MODULE)
 #define	MPLOCKED	"lock ; "
 #else
 #define	MPLOCKED
@@ -231,7 +230,7 @@
 }							\
 struct __hack
 
-#if defined(_KERNEL) && !defined(SMP)
+#if defined(_KERNEL) && !defined(SMP) && !defined(KLD_MODULE)
 
 #define	ATOMIC_LOAD(TYPE, LOP)				\
 static __inline u_##TYPE				\
@@ -245,7 +244,7 @@
 }							\
 struct __hack
 
-#else /* !(_KERNEL && !SMP) */
+#else /* !(_KERNEL && !SMP && !KLD_MODULE) */
 
 #define	ATOMIC_LOAD(TYPE, LOP)				\
 static __inline u_##TYPE				\
@@ -263,9 +262,9 @@
 }							\
 struct __hack
 
-#endif /* _KERNEL && !SMP */
+#endif /* _KERNEL && !SMP && !KLD_MODULE */
 
-#endif /* KLD_MODULE || !__GNUCLIKE_ASM */
+#endif /* WANT_PROTOTYPES || !__GNUCLIKE_ASM */
 
 ATOMIC_ASM(set,	     char,  "orb %b1,%0",  "iq",  v);
 ATOMIC_ASM(clear,    char,  "andb %b1,%0", "iq", ~v);
Index: head/sys/i386/i386/atomic.c
===================================================================
--- head/sys/i386/i386/atomic.c	(revision 242498)
+++ head/sys/i386/i386/atomic.c	(working copy)
@@ -33,11 +33,11 @@
  */
 #include <sys/types.h>
 
-/* Firstly make atomic.h generate prototypes as it will for kernel modules */
-#define KLD_MODULE
+/* Firstly make atomic.h generate prototypes */
+#define WANT_PROTOTYPES
 #include <machine/atomic.h>
 #undef _MACHINE_ATOMIC_H_	/* forget we included it */
-#undef KLD_MODULE
+#undef WANT_PROTOTYPES
 #undef ATOMIC_ASM
 
 /* Make atomic.h generate public functions */
Index: head/sys/i386/include/atomic.h
===================================================================
--- head/sys/i386/include/atomic.h	(revision 242498)
+++ head/sys/i386/include/atomic.h	(working copy)
@@ -64,14 +64,13 @@
  */
 
 /*
- * The above functions are expanded inline in the statically-linked
- * kernel.  Lock prefixes are generated if an SMP kernel is being
- * built.
- *
- * Kernel modules call real functions which are built into the kernel.
- * This allows kernel modules to be portable between UP and SMP systems.
+ * The above functions are all expanded inline if the compiler
+ * supports the gcc extensions to asm().  Lock prefixes are generated
+ * except for UP kernels (ie kernel modules and userland references
+ * always have lock prefixes).  This allows kernel modules and
+ * userland code to be portable between UP and SMP systems.
  */
-#if defined(KLD_MODULE) || !defined(__GNUCLIKE_ASM)
+#if defined(WANT_PROTOTYPES) || !defined(__GNUCLIKE_ASM)
 #define	ATOMIC_ASM(NAME, TYPE, OP, CONS, V)			\
 void atomic_##NAME##_##TYPE(volatile u_##TYPE *p, u_##TYPE v);	\
 void atomic_##NAME##_barr_##TYPE(volatile u_##TYPE *p, u_##TYPE v)
@@ -84,13 +83,13 @@
 #define	ATOMIC_STORE(TYPE)					\
 void		atomic_store_rel_##TYPE(volatile u_##TYPE *p, u_##TYPE v)
 
-#else /* !KLD_MODULE && __GNUCLIKE_ASM */
+#else /* !WANT_PROTOTYPES && __GNUCLIKE_ASM */
 
 /*
- * For userland, always use lock prefixes so that the binaries will run
- * on both SMP and !SMP systems.
+ * For userland and kernel modules, always use lock prefixes so that
+ * the binaries will run on both SMP and !SMP systems.
  */
-#if defined(SMP) || !defined(_KERNEL)
+#if defined(SMP) || !defined(_KERNEL) || defined(KLD_MODULE)
 #define	MPLOCKED	"lock ; "
 #else
 #define	MPLOCKED
@@ -301,7 +300,7 @@
 }							\
 struct __hack
 
-#if defined(_KERNEL) && !defined(SMP)
+#if defined(_KERNEL) && !defined(SMP) && !defined(KLD_MODULE)
 
 #define	ATOMIC_LOAD(TYPE, LOP)				\
 static __inline u_##TYPE				\
@@ -315,7 +314,7 @@
 }							\
 struct __hack
 
-#else /* !(_KERNEL && !SMP) */
+#else /* !(_KERNEL && !SMP && !KLD_MODULE) */
 
 #define	ATOMIC_LOAD(TYPE, LOP)				\
 static __inline u_##TYPE				\
@@ -333,9 +332,9 @@
 }							\
 struct __hack
 
-#endif /* _KERNEL && !SMP */
+#endif /* _KERNEL && !SMP && !KLD_MODULE */
 
-#endif /* KLD_MODULE || !__GNUCLIKE_ASM */
+#endif /* WANT_PROTOTYPES || !__GNUCLIKE_ASM */
 
 ATOMIC_ASM(set,	     char,  "orb %b1,%0",  "iq",  v);
 ATOMIC_ASM(clear,    char,  "andb %b1,%0", "iq", ~v);
>Release-Note:
>Audit-Trail:
>Unformatted:

More information about the freebsd-bugs mailing list