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

x86: Fix and improve cmpxchg_double{,_local}()

Just like the per-CPU ones they had several
problems/shortcomings:

Only the first memory operand was mentioned in the asm()
operands, and the 2x64-bit version didn't have a memory clobber
while the 2x32-bit one did. The former allowed the compiler to
not recognize the need to re-load the data in case it had it
cached in some register, while the latter was overly
destructive.

The types of the local copies of the old and new values were
incorrect (the types of the pointed-to variables should be used
here, to make sure the respective old/new variable types are
compatible).

The __dummy/__junk variables were pointless, given that local
copies of the inputs already existed (and can hence be used for
discarded outputs).

The 32-bit variant of cmpxchg_double_local() referenced
cmpxchg16b_local().

At once also:

- change the return value type to what it really is: 'bool'
- unify 32- and 64-bit variants
- abstract out the common part of the 'normal' and 'local' variants

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/4F01F12A020000780006A19B@nat28.tlf.novell.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>

authored by

Jan Beulich and committed by
Ingo Molnar
cdcd6298 adaf4ed2

+25 -91
+23
arch/x86/include/asm/cmpxchg.h
··· 207 207 #define add_smp(ptr, inc) __add((ptr), (inc), LOCK_PREFIX) 208 208 #define add_sync(ptr, inc) __add((ptr), (inc), "lock; ") 209 209 210 + #define __cmpxchg_double(pfx, p1, p2, o1, o2, n1, n2) \ 211 + ({ \ 212 + bool __ret; \ 213 + __typeof__(*(p1)) __old1 = (o1), __new1 = (n1); \ 214 + __typeof__(*(p2)) __old2 = (o2), __new2 = (n2); \ 215 + BUILD_BUG_ON(sizeof(*(p1)) != sizeof(long)); \ 216 + BUILD_BUG_ON(sizeof(*(p2)) != sizeof(long)); \ 217 + VM_BUG_ON((unsigned long)(p1) % (2 * sizeof(long))); \ 218 + VM_BUG_ON((unsigned long)((p1) + 1) != (unsigned long)(p2)); \ 219 + asm volatile(pfx "cmpxchg%c4b %2; sete %0" \ 220 + : "=a" (__ret), "+d" (__old2), \ 221 + "+m" (*(p1)), "+m" (*(p2)) \ 222 + : "i" (2 * sizeof(long)), "a" (__old1), \ 223 + "b" (__new1), "c" (__new2)); \ 224 + __ret; \ 225 + }) 226 + 227 + #define cmpxchg_double(p1, p2, o1, o2, n1, n2) \ 228 + __cmpxchg_double(LOCK_PREFIX, p1, p2, o1, o2, n1, n2) 229 + 230 + #define cmpxchg_double_local(p1, p2, o1, o2, n1, n2) \ 231 + __cmpxchg_double(, p1, p2, o1, o2, n1, n2) 232 + 210 233 #endif /* ASM_X86_CMPXCHG_H */
-46
arch/x86/include/asm/cmpxchg_32.h
··· 166 166 167 167 #endif 168 168 169 - #define cmpxchg8b(ptr, o1, o2, n1, n2) \ 170 - ({ \ 171 - char __ret; \ 172 - __typeof__(o2) __dummy; \ 173 - __typeof__(*(ptr)) __old1 = (o1); \ 174 - __typeof__(o2) __old2 = (o2); \ 175 - __typeof__(*(ptr)) __new1 = (n1); \ 176 - __typeof__(o2) __new2 = (n2); \ 177 - asm volatile(LOCK_PREFIX "cmpxchg8b %2; setz %1" \ 178 - : "=d"(__dummy), "=a" (__ret), "+m" (*ptr)\ 179 - : "a" (__old1), "d"(__old2), \ 180 - "b" (__new1), "c" (__new2) \ 181 - : "memory"); \ 182 - __ret; }) 183 - 184 - 185 - #define cmpxchg8b_local(ptr, o1, o2, n1, n2) \ 186 - ({ \ 187 - char __ret; \ 188 - __typeof__(o2) __dummy; \ 189 - __typeof__(*(ptr)) __old1 = (o1); \ 190 - __typeof__(o2) __old2 = (o2); \ 191 - __typeof__(*(ptr)) __new1 = (n1); \ 192 - __typeof__(o2) __new2 = (n2); \ 193 - asm volatile("cmpxchg8b %2; setz %1" \ 194 - : "=d"(__dummy), "=a"(__ret), "+m" (*ptr)\ 195 - : "a" (__old), "d"(__old2), \ 196 - "b" (__new1), "c" (__new2), \ 197 - : "memory"); \ 198 - __ret; }) 199 - 200 - 201 - #define cmpxchg_double(ptr, o1, o2, n1, n2) \ 202 - ({ \ 203 - BUILD_BUG_ON(sizeof(*(ptr)) != 4); \ 204 - VM_BUG_ON((unsigned long)(ptr) % 8); \ 205 - cmpxchg8b((ptr), (o1), (o2), (n1), (n2)); \ 206 - }) 207 - 208 - #define cmpxchg_double_local(ptr, o1, o2, n1, n2) \ 209 - ({ \ 210 - BUILD_BUG_ON(sizeof(*(ptr)) != 4); \ 211 - VM_BUG_ON((unsigned long)(ptr) % 8); \ 212 - cmpxchg16b_local((ptr), (o1), (o2), (n1), (n2)); \ 213 - }) 214 - 215 169 #define system_has_cmpxchg_double() cpu_has_cx8 216 170 217 171 #endif /* _ASM_X86_CMPXCHG_32_H */
-43
arch/x86/include/asm/cmpxchg_64.h
··· 20 20 cmpxchg_local((ptr), (o), (n)); \ 21 21 }) 22 22 23 - #define cmpxchg16b(ptr, o1, o2, n1, n2) \ 24 - ({ \ 25 - char __ret; \ 26 - __typeof__(o2) __junk; \ 27 - __typeof__(*(ptr)) __old1 = (o1); \ 28 - __typeof__(o2) __old2 = (o2); \ 29 - __typeof__(*(ptr)) __new1 = (n1); \ 30 - __typeof__(o2) __new2 = (n2); \ 31 - asm volatile(LOCK_PREFIX "cmpxchg16b %2;setz %1" \ 32 - : "=d"(__junk), "=a"(__ret), "+m" (*ptr) \ 33 - : "b"(__new1), "c"(__new2), \ 34 - "a"(__old1), "d"(__old2)); \ 35 - __ret; }) 36 - 37 - 38 - #define cmpxchg16b_local(ptr, o1, o2, n1, n2) \ 39 - ({ \ 40 - char __ret; \ 41 - __typeof__(o2) __junk; \ 42 - __typeof__(*(ptr)) __old1 = (o1); \ 43 - __typeof__(o2) __old2 = (o2); \ 44 - __typeof__(*(ptr)) __new1 = (n1); \ 45 - __typeof__(o2) __new2 = (n2); \ 46 - asm volatile("cmpxchg16b %2;setz %1" \ 47 - : "=d"(__junk), "=a"(__ret), "+m" (*ptr) \ 48 - : "b"(__new1), "c"(__new2), \ 49 - "a"(__old1), "d"(__old2)); \ 50 - __ret; }) 51 - 52 - #define cmpxchg_double(ptr, o1, o2, n1, n2) \ 53 - ({ \ 54 - BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ 55 - VM_BUG_ON((unsigned long)(ptr) % 16); \ 56 - cmpxchg16b((ptr), (o1), (o2), (n1), (n2)); \ 57 - }) 58 - 59 - #define cmpxchg_double_local(ptr, o1, o2, n1, n2) \ 60 - ({ \ 61 - BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ 62 - VM_BUG_ON((unsigned long)(ptr) % 16); \ 63 - cmpxchg16b_local((ptr), (o1), (o2), (n1), (n2)); \ 64 - }) 65 - 66 23 #define system_has_cmpxchg_double() cpu_has_cx16 67 24 68 25 #endif /* _ASM_X86_CMPXCHG_64_H */
+2 -2
mm/slub.c
··· 368 368 VM_BUG_ON(!irqs_disabled()); 369 369 #ifdef CONFIG_CMPXCHG_DOUBLE 370 370 if (s->flags & __CMPXCHG_DOUBLE) { 371 - if (cmpxchg_double(&page->freelist, 371 + if (cmpxchg_double(&page->freelist, &page->counters, 372 372 freelist_old, counters_old, 373 373 freelist_new, counters_new)) 374 374 return 1; ··· 402 402 { 403 403 #ifdef CONFIG_CMPXCHG_DOUBLE 404 404 if (s->flags & __CMPXCHG_DOUBLE) { 405 - if (cmpxchg_double(&page->freelist, 405 + if (cmpxchg_double(&page->freelist, &page->counters, 406 406 freelist_old, counters_old, 407 407 freelist_new, counters_new)) 408 408 return 1;