Date: Mon, 15 Jan 2001 09:25:45 +1100 From: Peter Jeremy <peter.jeremy@alcatel.com.au> To: Mark Murray <mark@grondar.za> Cc: jhb@FreeBSD.ORG, current@FreeBSD.ORG Subject: Re: Atomic breakage? Message-ID: <20010115092544.U91029@gsmx07.alcatel.com.au> In-Reply-To: <200101142102.f0EL2OI25280@gratis.grondar.za>; from mark@grondar.za on Sun, Jan 14, 2001 at 11:02:28PM %2B0200 References: <200101142102.f0EL2OI25280@gratis.grondar.za>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2001-Jan-14 23:02:28 +0200, Mark Murray <mark@grondar.za> wrote:
>Hi John
>
>There seems to be same breakage in the atomic stuff:
>
>link_elf: symbol atomic_load_acq_int undefined
>KLD file random.ko - could not finalize loading
>
>I back out the latest commit to sys/i386/include/atomic.h, and things
>work a bit better (on my laptop).
Basically, the problem is that some of the recent commits have broken
the interface between sys/i386/include/atomic.h and sys/i386/i386/atomic.c
The latter file builds non-inlined versions of the atomic functions
defined in atomic.h. This means that atomic.h must be laid out in a
suitable manner so that the non-inlined functions are built by
atomic.c. (Modules use explicit function calls, rather than inlined
atomic functions to remove the need to have distinct UP and SMP
versions.)
The layout of atomic.h should look like:
#ifdef KLD_MODULE
#defines expanding to prototypes.
#else
#defines expanding to inline function definitions
#endif
List of atomic functions (invoking above macros)
Due to incompatibilities between __asm in different versions of gcc,
several different versions of various macros (and expansions) are
necessary.
atomic.c will include atomic.h with "static" and "__inline" #define'd
to nothing to build a set of atomic functions with external
visibility.
The problem is that over time, atomic.h has been expanded and things
have gotten a bit confused.
Mark's reported breakage is a result of the new atomic_cmpset_int().
This has been defined in the !KLD_MODULE section (but only for new
versions of gcc - which should be OK since it'll never be used in
conjunction with the old gcc) and a suitable prototype has been
inserted in the KLD_MODULE section. The problem is that a pile of
#defines have been incorrectly put in this section, rather than
outside the KLD_MODULE section. This means that modules are
left with dangling references to these functions.
Two (untested) patches are attached, the first is simpler (and
equivalent to the current code), but I think it will report type
mismatches.
And for BDE's benefit - atomic.h is broken for IA32's with 64-bit
longs. (I believe that can be fixed for Pentiums and above using
CMPXCHG8B, but I can't test the code).
Index: atomic.h
===================================================================
RCS file: /home/CVSROOT/src/sys/i386/include/atomic.h,v
retrieving revision 1.16
diff -u -r1.16 atomic.h
--- atomic.h 2000/10/28 00:28:15 1.16
+++ atomic.h 2001/01/14 22:14:31
@@ -151,12 +151,6 @@
}
#endif /* defined(I386_CPU) */
-#define atomic_cmpset_long atomic_cmpset_int
-#define atomic_cmpset_acq_int atomic_cmpset_int
-#define atomic_cmpset_rel_int atomic_cmpset_int
-#define atomic_cmpset_acq_long atomic_cmpset_acq_int
-#define atomic_cmpset_rel_long atomic_cmpset_rel_int
-
#else
/* gcc <= 2.8 version */
#define ATOMIC_ASM(NAME, TYPE, OP, V) \
@@ -222,6 +216,12 @@
#undef ATOMIC_ASM
+#define atomic_cmpset_long atomic_cmpset_int
+#define atomic_cmpset_acq_int atomic_cmpset_int
+#define atomic_cmpset_rel_int atomic_cmpset_int
+#define atomic_cmpset_acq_long atomic_cmpset_acq_int
+#define atomic_cmpset_rel_long atomic_cmpset_rel_int
+
#ifndef WANT_FUNCTIONS
#define ATOMIC_ACQ_REL(NAME, TYPE) \
static __inline void \
Index: atomic.h
===================================================================
RCS file: /home/CVSROOT/src/sys/i386/include/atomic.h,v
retrieving revision 1.16
diff -u -r1.16 atomic.h
--- atomic.h 2000/10/28 00:28:15 1.16
+++ atomic.h 2001/01/14 22:20:30
@@ -151,12 +151,6 @@
}
#endif /* defined(I386_CPU) */
-#define atomic_cmpset_long atomic_cmpset_int
-#define atomic_cmpset_acq_int atomic_cmpset_int
-#define atomic_cmpset_rel_int atomic_cmpset_int
-#define atomic_cmpset_acq_long atomic_cmpset_acq_int
-#define atomic_cmpset_rel_long atomic_cmpset_rel_int
-
#else
/* gcc <= 2.8 version */
#define ATOMIC_ASM(NAME, TYPE, OP, V) \
@@ -223,6 +217,22 @@
#undef ATOMIC_ASM
#ifndef WANT_FUNCTIONS
+
+static __inline long
+atomic_cmpset_long(volatile u_long *dst, u_long exp, u_long src)
+{
+ return (atomic_cmpset_int((volatile u_int *)dst, (u_int)exp, (u_int)src));
+}
+
+#define atomic_cmpset_acq_int(dst, exp, src) \
+ atomic_cmpset_int(dst, exp, src)
+#define atomic_cmpset_rel_int(dst, exp, src) \
+ atomic_cmpset_int(dst, exp, src)
+#define atomic_cmpset_acq_long(dst, exp, src) \
+ atomic_cmpset_long(dst, exp, src)
+#define atomic_cmpset_rel_long(dst, exp, src) \
+ atomic_cmpset_long(dst, exp, src)
+
#define ATOMIC_ACQ_REL(NAME, TYPE) \
static __inline void \
atomic_##NAME##_acq_##TYPE(volatile u_##TYPE *p, u_##TYPE v)\
Peter
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010115092544.U91029>