>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
[email protected]:/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:
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"
