summaryrefslogtreecommitdiffstats
path: root/arch/x86/include/asm/cmpxchg.h
diff options
context:
space:
mode:
authorPeter Zijlstra <peterz@infradead.org>2017-03-27 13:54:38 +0200
committerIngo Molnar <mingo@kernel.org>2017-03-30 09:35:54 +0200
commit44fe84459faf1a7781595b7c64cd36daf2f2827d (patch)
tree1fa467e66d19c23bfea059c11ebb4fee9386de41 /arch/x86/include/asm/cmpxchg.h
parentlockdep: Fix per-cpu static objects (diff)
downloadlinux-44fe84459faf1a7781595b7c64cd36daf2f2827d.tar.xz
linux-44fe84459faf1a7781595b7c64cd36daf2f2827d.zip
locking/atomic: Fix atomic_try_cmpxchg() semantics
Dmitry noted that the new atomic_try_cmpxchg() primitive is broken when the old pointer doesn't point to the local stack. He writes: "Consider a classical lock-free stack push: node->next = atomic_read(&head); do { } while (!atomic_try_cmpxchg(&head, &node->next, node)); This code is broken with the current implementation, the problem is with unconditional update of *__po. In case of success it writes the same value back into *__po, but in case of cmpxchg success we might have lose ownership of some memory locations and potentially over what __po has pointed to. The same holds for the re-read of *__po. " He also points out that this makes it surprisingly different from the similar C/C++ atomic operation. After investigating the code-gen differences caused by this patch; and a number of alternatives (Linus dislikes this interface lots), we arrived at these results (size x86_64-defconfig/vmlinux): GCC-6.3.0: 10735757 cmpxchg 10726413 try_cmpxchg 10730509 try_cmpxchg + patch 10730445 try_cmpxchg-linus GCC-7 (20170327): 10709514 cmpxchg 10704266 try_cmpxchg 10704266 try_cmpxchg + patch 10704394 try_cmpxchg-linus From this we see that the patch has the advantage of better code-gen on GCC-7 and keeps the interface roughly consistent with the C language variant. Reported-by: Dmitry Vyukov <dvyukov@google.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: linux-kernel@vger.kernel.org Fixes: a9ebf306f52c ("locking/atomic: Introduce atomic_try_cmpxchg()") Signed-off-by: Ingo Molnar <mingo@kernel.org>
Diffstat (limited to 'arch/x86/include/asm/cmpxchg.h')
-rw-r--r--arch/x86/include/asm/cmpxchg.h5
1 files changed, 3 insertions, 2 deletions
diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
index fb961db51a2a..d90296d061e8 100644
--- a/arch/x86/include/asm/cmpxchg.h
+++ b/arch/x86/include/asm/cmpxchg.h
@@ -212,8 +212,9 @@ extern void __add_wrong_size(void)
default: \
__cmpxchg_wrong_size(); \
} \
- *_old = __old; \
- success; \
+ if (unlikely(!success)) \
+ *_old = __old; \
+ likely(success); \
})
#define __try_cmpxchg(ptr, pold, new, size) \