[PATCH] powerpc: use lwsync in lock functions

Anton Blanchard anton at samba.org
Fri Jan 13 15:37:17 EST 2006


eieio is only a store - store ordering. When used to order an unlock
operation loads may leak out of the critical region. This is potentially
buggy, one example is if a user wants to atomically read a couple of
values.

We can solve this with an lwsync which orders everything except store - load.

I removed the (now unused) EIEIO_ON_SMP macros and the c versions
isync_on_smp and eieio_on_smp now we dont use them. I also removed some
old comments that were used to identify inline spinlocks in assembly,
they dont make sense now our locks are out of line.

Another interesting thing was that read_unlock was using an eieio even
though the rest of the spinlock code had already been converted to
use lwsync. 

Signed-off-by: Anton Blanchard <anton at samba.org>
---

Index: build/include/asm-powerpc/atomic.h
===================================================================
--- build.orig/include/asm-powerpc/atomic.h	2006-01-13 12:01:03.000000000 +1100
+++ build/include/asm-powerpc/atomic.h	2006-01-13 12:01:04.000000000 +1100
@@ -36,7 +36,7 @@
 	int t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	lwarx	%0,0,%2		# atomic_add_return\n\
 	add	%0,%1,%0\n"
 	PPC405_ERR77(0,%2)
@@ -72,7 +72,7 @@
 	int t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	lwarx	%0,0,%2		# atomic_sub_return\n\
 	subf	%0,%1,%0\n"
 	PPC405_ERR77(0,%2)
@@ -106,7 +106,7 @@
 	int t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	lwarx	%0,0,%1		# atomic_inc_return\n\
 	addic	%0,%0,1\n"
 	PPC405_ERR77(0,%1)
@@ -150,7 +150,7 @@
 	int t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	lwarx	%0,0,%1		# atomic_dec_return\n\
 	addic	%0,%0,-1\n"
 	PPC405_ERR77(0,%1)
@@ -204,7 +204,7 @@
 	int t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	lwarx	%0,0,%1		# atomic_dec_if_positive\n\
 	addic.	%0,%0,-1\n\
 	blt-	2f\n"
@@ -253,7 +253,7 @@
 	long t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	ldarx	%0,0,%2		# atomic64_add_return\n\
 	add	%0,%1,%0\n\
 	stdcx.	%0,0,%2 \n\
@@ -287,7 +287,7 @@
 	long t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	ldarx	%0,0,%2		# atomic64_sub_return\n\
 	subf	%0,%1,%0\n\
 	stdcx.	%0,0,%2 \n\
@@ -319,7 +319,7 @@
 	long t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	ldarx	%0,0,%1		# atomic64_inc_return\n\
 	addic	%0,%0,1\n\
 	stdcx.	%0,0,%1 \n\
@@ -361,7 +361,7 @@
 	long t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	ldarx	%0,0,%1		# atomic64_dec_return\n\
 	addic	%0,%0,-1\n\
 	stdcx.	%0,0,%1\n\
@@ -386,7 +386,7 @@
 	long t;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	ldarx	%0,0,%1		# atomic64_dec_if_positive\n\
 	addic.	%0,%0,-1\n\
 	blt-	2f\n\
Index: build/include/asm-powerpc/bitops.h
===================================================================
--- build.orig/include/asm-powerpc/bitops.h	2006-01-13 12:01:03.000000000 +1100
+++ build/include/asm-powerpc/bitops.h	2006-01-13 12:01:04.000000000 +1100
@@ -112,7 +112,7 @@
 	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:"	PPC_LLARX "%0,0,%3		# test_and_set_bit\n"
 	"or	%1,%0,%2 \n"
 	PPC405_ERR77(0,%3)
@@ -134,7 +134,7 @@
 	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:"	PPC_LLARX "%0,0,%3		# test_and_clear_bit\n"
 	"andc	%1,%0,%2 \n"
 	PPC405_ERR77(0,%3)
@@ -156,7 +156,7 @@
 	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:"	PPC_LLARX "%0,0,%3		# test_and_change_bit\n"
 	"xor	%1,%0,%2 \n"
 	PPC405_ERR77(0,%3)
Index: build/include/asm-powerpc/futex.h
===================================================================
--- build.orig/include/asm-powerpc/futex.h	2006-01-13 12:01:03.000000000 +1100
+++ build/include/asm-powerpc/futex.h	2006-01-13 12:01:04.000000000 +1100
@@ -11,7 +11,7 @@
 
 #define __futex_atomic_op(insn, ret, oldval, uaddr, oparg) \
   __asm__ __volatile ( \
-	SYNC_ON_SMP \
+	LWSYNC_ON_SMP \
 "1:	lwarx	%0,0,%2\n" \
 	insn \
 	PPC405_ERR77(0, %2) \
Index: build/include/asm-powerpc/spinlock.h
===================================================================
--- build.orig/include/asm-powerpc/spinlock.h	2006-01-13 12:01:03.000000000 +1100
+++ build/include/asm-powerpc/spinlock.h	2006-01-13 14:47:15.000000000 +1100
@@ -46,7 +46,7 @@
 
 	token = LOCK_TOKEN;
 	__asm__ __volatile__(
-"1:	lwarx		%0,0,%2		# __spin_trylock\n\
+"1:	lwarx		%0,0,%2\n\
 	cmpwi		0,%0,0\n\
 	bne-		2f\n\
 	stwcx.		%1,0,%2\n\
@@ -124,8 +124,8 @@
 
 static __inline__ void __raw_spin_unlock(raw_spinlock_t *lock)
 {
-	__asm__ __volatile__(SYNC_ON_SMP"	# __raw_spin_unlock"
-			     : : :"memory");
+	__asm__ __volatile__("# __raw_spin_unlock\n\t"
+				LWSYNC_ON_SMP: : :"memory");
 	lock->slock = 0;
 }
 
@@ -167,7 +167,7 @@
 	long tmp;
 
 	__asm__ __volatile__(
-"1:	lwarx		%0,0,%1		# read_trylock\n"
+"1:	lwarx		%0,0,%1\n"
 	__DO_SIGN_EXTEND
 "	addic.		%0,%0,1\n\
 	ble-		2f\n"
@@ -192,7 +192,7 @@
 
 	token = WRLOCK_TOKEN;
 	__asm__ __volatile__(
-"1:	lwarx		%0,0,%2	# write_trylock\n\
+"1:	lwarx		%0,0,%2\n\
 	cmpwi		0,%0,0\n\
 	bne-		2f\n"
 	PPC405_ERR77(0,%1)
@@ -249,8 +249,9 @@
 	long tmp;
 
 	__asm__ __volatile__(
-	"eieio				# read_unlock\n\
-1:	lwarx		%0,0,%1\n\
+	"# read_unlock\n\t"
+	LWSYNC_ON_SMP
+"1:	lwarx		%0,0,%1\n\
 	addic		%0,%0,-1\n"
 	PPC405_ERR77(0,%1)
 "	stwcx.		%0,0,%1\n\
@@ -262,8 +263,8 @@
 
 static __inline__ void __raw_write_unlock(raw_rwlock_t *rw)
 {
-	__asm__ __volatile__(SYNC_ON_SMP"	# write_unlock"
-			     : : :"memory");
+	__asm__ __volatile__("# write_unlock\n\t"
+				LWSYNC_ON_SMP: : :"memory");
 	rw->lock = 0;
 }
 
Index: build/include/asm-powerpc/synch.h
===================================================================
--- build.orig/include/asm-powerpc/synch.h	2006-01-13 12:01:03.000000000 +1100
+++ build/include/asm-powerpc/synch.h	2006-01-13 14:31:13.000000000 +1100
@@ -2,6 +2,8 @@
 #define _ASM_POWERPC_SYNCH_H 
 #ifdef __KERNEL__
 
+#include <linux/stringify.h>
+
 #ifdef __powerpc64__
 #define __SUBARCH_HAS_LWSYNC
 #endif
@@ -12,20 +14,12 @@
 #    define LWSYNC	sync
 #endif
 
-
-/*
- * Arguably the bitops and *xchg operations don't imply any memory barrier
- * or SMP ordering, but in fact a lot of drivers expect them to imply
- * both, since they do on x86 cpus.
- */
 #ifdef CONFIG_SMP
-#define EIEIO_ON_SMP	"eieio\n"
 #define ISYNC_ON_SMP	"\n\tisync"
-#define SYNC_ON_SMP	__stringify(LWSYNC) "\n"
+#define LWSYNC_ON_SMP	__stringify(LWSYNC) "\n"
 #else
-#define EIEIO_ON_SMP
 #define ISYNC_ON_SMP
-#define SYNC_ON_SMP
+#define LWSYNC_ON_SMP
 #endif
 
 static inline void eieio(void)
@@ -38,14 +32,5 @@
 	__asm__ __volatile__ ("isync" : : : "memory");
 }
 
-#ifdef CONFIG_SMP
-#define eieio_on_smp()	eieio()
-#define isync_on_smp()	isync()
-#else
-#define eieio_on_smp()	__asm__ __volatile__("": : :"memory")
-#define isync_on_smp()	__asm__ __volatile__("": : :"memory")
-#endif
-
 #endif /* __KERNEL__ */
 #endif	/* _ASM_POWERPC_SYNCH_H */
-
Index: build/include/asm-powerpc/system.h
===================================================================
--- build.orig/include/asm-powerpc/system.h	2006-01-13 12:01:03.000000000 +1100
+++ build/include/asm-powerpc/system.h	2006-01-13 12:01:04.000000000 +1100
@@ -212,7 +212,7 @@
 	unsigned long prev;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	lwarx	%0,0,%2 \n"
 	PPC405_ERR77(0,%2)
 "	stwcx.	%3,0,%2 \n\
@@ -232,7 +232,7 @@
 	unsigned long prev;
 
 	__asm__ __volatile__(
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	ldarx	%0,0,%2 \n"
 	PPC405_ERR77(0,%2)
 "	stdcx.	%3,0,%2 \n\
@@ -287,7 +287,7 @@
 	unsigned int prev;
 
 	__asm__ __volatile__ (
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	lwarx	%0,0,%2		# __cmpxchg_u32\n\
 	cmpw	0,%0,%3\n\
 	bne-	2f\n"
@@ -311,7 +311,7 @@
 	unsigned long prev;
 
 	__asm__ __volatile__ (
-	EIEIO_ON_SMP
+	LWSYNC_ON_SMP
 "1:	ldarx	%0,0,%2		# __cmpxchg_u64\n\
 	cmpd	0,%0,%3\n\
 	bne-	2f\n\



More information about the Linuxppc64-dev mailing list