• Peter Zijlstra's avatar
    locking/atomic: Fix atomic_try_cmpxchg() semantics · 44fe8445
    Peter Zijlstra authored
    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):
      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: default avatarDmitry Vyukov <dvyukov@google.com>
    Signed-off-by: default avatarPeter 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: a9ebf306 ("locking/atomic: Introduce atomic_try_cmpxchg()")
    Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
atomic.h 30 KB