Re: [PATCH tip/locking/core v4 1/6] powerpc: atomic: Make *xchg and *cmpxchg a full barrier
flat view
From: Paul E. McKenney <hidden>
Date: 2015-10-14 20:19:15
Also in:
lkml, stable
On Wed, Oct 14, 2015 at 11:55:56PM +0800, Boqun Feng wrote:
quoted hunk ↗ jump to hunk
According to memory-barriers.txt, xchg, cmpxchg and their atomic{,64}_ versions all need to imply a full barrier, however they are now just RELEASE+ACQUIRE, which is not a full barrier. So replace PPC_RELEASE_BARRIER and PPC_ACQUIRE_BARRIER with PPC_ATOMIC_ENTRY_BARRIER and PPC_ATOMIC_EXIT_BARRIER in __{cmp,}xchg_{u32,u64} respectively to guarantee a full barrier semantics of atomic{,64}_{cmp,}xchg() and {cmp,}xchg(). This patch is a complement of commit b97021f85517 ("powerpc: Fix atomic_xxx_return barrier semantics"). Acked-by: Michael Ellerman <mpe@ellerman.id.au> Cc: <redacted> # 3.4+ Signed-off-by: Boqun Feng <redacted> --- arch/powerpc/include/asm/cmpxchg.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h index ad6263c..d1a8d93 100644 --- a/arch/powerpc/include/asm/cmpxchg.h +++ b/arch/powerpc/include/asm/cmpxchg.h@@ -18,12 +18,12 @@ __xchg_u32(volatile void *p, unsigned long val) unsigned long prev; __asm__ __volatile__( - PPC_RELEASE_BARRIER + PPC_ATOMIC_ENTRY_BARRIER
This looks to be the lwsync instruction.
quoted hunk ↗ jump to hunk
"1: lwarx %0,0,%2 \n" PPC405_ERR77(0,%2) " stwcx. %3,0,%2 \n\ bne- 1b" - PPC_ACQUIRE_BARRIER + PPC_ATOMIC_EXIT_BARRIER
And this looks to be the sync instruction.
: "=&r" (prev), "+m" (*(volatile unsigned int *)p) : "r" (p), "r" (val) : "cc", "memory");
Hmmm... Suppose we have something like the following, where "a" and "x" are both initially zero: CPU 0 CPU 1 ----- ----- WRITE_ONCE(x, 1); WRITE_ONCE(a, 2); r3 = xchg(&a, 1); smp_mb(); r3 = READ_ONCE(x); If xchg() is fully ordered, we should never observe both CPUs' r3 values being zero, correct? And wouldn't this be represented by the following litmus test? PPC SB+lwsync-RMW2-lwsync+st-sync-leading "" { 0:r1=1; 0:r2=x; 0:r3=3; 0:r10=0 ; 0:r11=0; 0:r12=a; 1:r1=2; 1:r2=x; 1:r3=3; 1:r10=0 ; 1:r11=0; 1:r12=a; } P0 | P1 ; stw r1,0(r2) | stw r1,0(r12) ; lwsync | sync ; lwarx r11,r10,r12 | lwz r3,0(r2) ; stwcx. r1,r10,r12 | ; bne Fail0 | ; mr r3,r11 | ; Fail0: | ; exists (0:r3=0 /\ a=2 /\ 1:r3=0) I left off P0's trailing sync because there is nothing for it to order against in this particular litmus test. I tried adding it and verified that it has no effect. Am I missing something here? If not, it seems to me that you need the leading lwsync to instead be a sync. Of course, if I am not missing something, then this applies also to the value-returning RMW atomic operations that you pulled this pattern from. If so, it would seem that I didn't think through all the possibilities back when PPC_ATOMIC_EXIT_BARRIER moved to sync... In fact, I believe that I worried about the RMW atomic operation acting as a barrier, but not as the load/store itself. :-/ Thanx, Paul
quoted hunk ↗ jump to hunk
@@ -61,12 +61,12 @@ __xchg_u64(volatile void *p, unsigned long val) unsigned long prev; __asm__ __volatile__( - PPC_RELEASE_BARRIER + PPC_ATOMIC_ENTRY_BARRIER "1: ldarx %0,0,%2 \n" PPC405_ERR77(0,%2) " stdcx. %3,0,%2 \n\ bne- 1b" - PPC_ACQUIRE_BARRIER + PPC_ATOMIC_EXIT_BARRIER : "=&r" (prev), "+m" (*(volatile unsigned long *)p) : "r" (p), "r" (val) : "cc", "memory");@@ -151,14 +151,14 @@ __cmpxchg_u32(volatile unsigned int *p, unsigned long old, unsigned long new) unsigned int prev; __asm__ __volatile__ ( - PPC_RELEASE_BARRIER + PPC_ATOMIC_ENTRY_BARRIER "1: lwarx %0,0,%2 # __cmpxchg_u32\n\ cmpw 0,%0,%3\n\ bne- 2f\n" PPC405_ERR77(0,%2) " stwcx. %4,0,%2\n\ bne- 1b" - PPC_ACQUIRE_BARRIER + PPC_ATOMIC_EXIT_BARRIER "\n\ 2:" : "=&r" (prev), "+m" (*p)@@ -197,13 +197,13 @@ __cmpxchg_u64(volatile unsigned long *p, unsigned long old, unsigned long new) unsigned long prev; __asm__ __volatile__ ( - PPC_RELEASE_BARRIER + PPC_ATOMIC_ENTRY_BARRIER "1: ldarx %0,0,%2 # __cmpxchg_u64\n\ cmpd 0,%0,%3\n\ bne- 2f\n\ stdcx. %4,0,%2\n\ bne- 1b" - PPC_ACQUIRE_BARRIER + PPC_ATOMIC_EXIT_BARRIER "\n\ 2:" : "=&r" (prev), "+m" (*p)-- 2.5.3