RFR: 8187227: __m68k_cmpxchg() is not being used correctly

[John Paul Adrian Glaubitz]

Hi!

On m68k, linux-zero has platform-specific implementations for compare_and_swap(),
add_and_fetch() and lock_test_and_set(). These functions are all using __m68k_cmpxchg()
which is basically a wrapper around the m68k assembly instruction CAS.

Currently, all three functions make incorrect assumptions about how CAS and its
wrapper actually work and consequently use __m68k_cmpxchg() incorrectly. The source
code comment for __m68_cmpxchg() states:

  * Atomically store newval in *ptr if *ptr is equal to oldval for user space.
  * Returns newval on success and oldval if no exchange happened.
  * This implementation is processor specific and works on
  * 68020 68030 68040 and 68060.

However, looking at the documentation for the CAS instruction on m68k [1] and the
implementation of __m68k_cmpxchg(), this is actually not how the function works.
It does not return the update value on a successful exchange but rather the contents
the compare operand, i.e. oldval. If no exchange happened, it will actually return
the contents of the memory location. newval is never returned and consequently
testing for "newval" in compare_and_swap(), add_and_fetch() and lock_test_and_set()
is a bug.

I have preapred a patch that fixes this issue by making correct use of __m68k_cmpxchg()
in compare_and_swap(), add_and_fetch() and lock_test_and_set(). This patch has been
tested to work on Debian m68k.

Additional notes:

In a previous of RFR of this bug - which I cannot find anymore - David Holmes was
questioning whether the use of the loops within the atomic helper functions was
correct. After thinking about the problem again, the answer is very simple: The
CAS operation, while guaranteed to perform the swap atomically, is actually not
guaranteed to succeed on first attempt trying to swap a memory operand with a
direct operand. Hence, we have to keep trying to perform the compare-and-swap
operation until it succeeds. That's what the loops are foor.

Updated webrev can be found in [2].

Thanks,
Adrian

> [1] http://68k.hax.com/CAS
> [2] http://cr.openjdk.java.net/~glaubitz/8187227/webrev.02/

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz at debian.org
`. `'   Freie Universitaet Berlin - glaubitz at physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913