Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

locking/xchg/alpha: Remove superfluous memory barriers from the _local() variants

The following two commits:

79d442461df74 ("locking/xchg/alpha: Clean up barrier usage by using smp_mb() in place of __ASM__MB")
472e8c55cf662 ("locking/xchg/alpha: Fix xchg() and cmpxchg() memory ordering bugs")

... ended up adding unnecessary barriers to the _local() variants on Alpha,
which the previous code took care to avoid.

Fix them by adding the smp_mb() into the cmpxchg() macro rather than into the
____cmpxchg() variants.

Reported-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-alpha@vger.kernel.org
Fixes: 472e8c55cf662 ("locking/xchg/alpha: Fix xchg() and cmpxchg() memory ordering bugs")
Fixes: 79d442461df74 ("locking/xchg/alpha: Clean up barrier usage by using smp_mb() in place of __ASM__MB")
Link: http://lkml.kernel.org/r/1519704058-13430-1-git-send-email-parri.andrea@gmail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>

authored by

Andrea Parri and committed by
Ingo Molnar
fbfcd019 bd5c0ba2

+16 -31
+16 -4
arch/alpha/include/asm/cmpxchg.h
··· 38 38 #define ____cmpxchg(type, args...) __cmpxchg ##type(args) 39 39 #include <asm/xchg.h> 40 40 41 + /* 42 + * The leading and the trailing memory barriers guarantee that these 43 + * operations are fully ordered. 44 + */ 41 45 #define xchg(ptr, x) \ 42 46 ({ \ 47 + __typeof__(*(ptr)) __ret; \ 43 48 __typeof__(*(ptr)) _x_ = (x); \ 44 - (__typeof__(*(ptr))) __xchg((ptr), (unsigned long)_x_, \ 45 - sizeof(*(ptr))); \ 49 + smp_mb(); \ 50 + __ret = (__typeof__(*(ptr))) \ 51 + __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \ 52 + smp_mb(); \ 53 + __ret; \ 46 54 }) 47 55 48 56 #define cmpxchg(ptr, o, n) \ 49 57 ({ \ 58 + __typeof__(*(ptr)) __ret; \ 50 59 __typeof__(*(ptr)) _o_ = (o); \ 51 60 __typeof__(*(ptr)) _n_ = (n); \ 52 - (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_, \ 53 - (unsigned long)_n_, sizeof(*(ptr)));\ 61 + smp_mb(); \ 62 + __ret = (__typeof__(*(ptr))) __cmpxchg((ptr), \ 63 + (unsigned long)_o_, (unsigned long)_n_, sizeof(*(ptr)));\ 64 + smp_mb(); \ 65 + __ret; \ 54 66 }) 55 67 56 68 #define cmpxchg64(ptr, o, n) \
-27
arch/alpha/include/asm/xchg.h
··· 12 12 * Atomic exchange. 13 13 * Since it can be used to implement critical sections 14 14 * it must clobber "memory" (also for interrupts in UP). 15 - * 16 - * The leading and the trailing memory barriers guarantee that these 17 - * operations are fully ordered. 18 - * 19 15 */ 20 16 21 17 static inline unsigned long ··· 19 23 { 20 24 unsigned long ret, tmp, addr64; 21 25 22 - smp_mb(); 23 26 __asm__ __volatile__( 24 27 " andnot %4,7,%3\n" 25 28 " insbl %1,%4,%1\n" ··· 33 38 ".previous" 34 39 : "=&r" (ret), "=&r" (val), "=&r" (tmp), "=&r" (addr64) 35 40 : "r" ((long)m), "1" (val) : "memory"); 36 - smp_mb(); 37 41 38 42 return ret; 39 43 } ··· 42 48 { 43 49 unsigned long ret, tmp, addr64; 44 50 45 - smp_mb(); 46 51 __asm__ __volatile__( 47 52 " andnot %4,7,%3\n" 48 53 " inswl %1,%4,%1\n" ··· 56 63 ".previous" 57 64 : "=&r" (ret), "=&r" (val), "=&r" (tmp), "=&r" (addr64) 58 65 : "r" ((long)m), "1" (val) : "memory"); 59 - smp_mb(); 60 66 61 67 return ret; 62 68 } ··· 65 73 { 66 74 unsigned long dummy; 67 75 68 - smp_mb(); 69 76 __asm__ __volatile__( 70 77 "1: ldl_l %0,%4\n" 71 78 " bis $31,%3,%1\n" ··· 75 84 ".previous" 76 85 : "=&r" (val), "=&r" (dummy), "=m" (*m) 77 86 : "rI" (val), "m" (*m) : "memory"); 78 - smp_mb(); 79 87 80 88 return val; 81 89 } ··· 84 94 { 85 95 unsigned long dummy; 86 96 87 - smp_mb(); 88 97 __asm__ __volatile__( 89 98 "1: ldq_l %0,%4\n" 90 99 " bis $31,%3,%1\n" ··· 94 105 ".previous" 95 106 : "=&r" (val), "=&r" (dummy), "=m" (*m) 96 107 : "rI" (val), "m" (*m) : "memory"); 97 - smp_mb(); 98 108 99 109 return val; 100 110 } ··· 123 135 * Atomic compare and exchange. Compare OLD with MEM, if identical, 124 136 * store NEW in MEM. Return the initial value in MEM. Success is 125 137 * indicated by comparing RETURN with OLD. 126 - * 127 - * The leading and the trailing memory barriers guarantee that these 128 - * operations are fully ordered. 129 - * 130 - * The trailing memory barrier is placed in SMP unconditionally, in 131 - * order to guarantee that dependency ordering is preserved when a 132 - * dependency is headed by an unsuccessful operation. 133 138 */ 134 139 135 140 static inline unsigned long ··· 130 149 { 131 150 unsigned long prev, tmp, cmp, addr64; 132 151 133 - smp_mb(); 134 152 __asm__ __volatile__( 135 153 " andnot %5,7,%4\n" 136 154 " insbl %1,%5,%1\n" ··· 147 167 ".previous" 148 168 : "=&r" (prev), "=&r" (new), "=&r" (tmp), "=&r" (cmp), "=&r" (addr64) 149 169 : "r" ((long)m), "Ir" (old), "1" (new) : "memory"); 150 - smp_mb(); 151 170 152 171 return prev; 153 172 } ··· 156 177 { 157 178 unsigned long prev, tmp, cmp, addr64; 158 179 159 - smp_mb(); 160 180 __asm__ __volatile__( 161 181 " andnot %5,7,%4\n" 162 182 " inswl %1,%5,%1\n" ··· 173 195 ".previous" 174 196 : "=&r" (prev), "=&r" (new), "=&r" (tmp), "=&r" (cmp), "=&r" (addr64) 175 197 : "r" ((long)m), "Ir" (old), "1" (new) : "memory"); 176 - smp_mb(); 177 198 178 199 return prev; 179 200 } ··· 182 205 { 183 206 unsigned long prev, cmp; 184 207 185 - smp_mb(); 186 208 __asm__ __volatile__( 187 209 "1: ldl_l %0,%5\n" 188 210 " cmpeq %0,%3,%1\n" ··· 195 219 ".previous" 196 220 : "=&r"(prev), "=&r"(cmp), "=m"(*m) 197 221 : "r"((long) old), "r"(new), "m"(*m) : "memory"); 198 - smp_mb(); 199 222 200 223 return prev; 201 224 } ··· 204 229 { 205 230 unsigned long prev, cmp; 206 231 207 - smp_mb(); 208 232 __asm__ __volatile__( 209 233 "1: ldq_l %0,%5\n" 210 234 " cmpeq %0,%3,%1\n" ··· 217 243 ".previous" 218 244 : "=&r"(prev), "=&r"(cmp), "=m"(*m) 219 245 : "r"((long) old), "r"(new), "m"(*m) : "memory"); 220 - smp_mb(); 221 246 222 247 return prev; 223 248 }