(S) RFR: 8157904: Atomic::cmpxchg for jbyte is missing a fence on initial failure
David Holmes
david.holmes at oracle.com
Mon Aug 22 07:54:03 UTC 2016
Bug: https://bugs.openjdk.java.net/browse/JDK-8157904
webrev: http://cr.openjdk.java.net/~dholmes/8157904/webrev/
An earlier code review noticed that the default shared implementation of
Atomic::cmpxchg(jbyte*) was missing the required post-memory-barrier in
case of an initial failure:
while (cur_as_bytes[offset] == comparand) {
jint res = cmpxchg(new_val, dest_int, cur, order);
...
}
if we never enter the while loop we don't do a cmpxchg and so we have no
memory barrier.
The simple fix is to invert things and use a do { } while () loop. That
way we always execute at least one real cmpxchg and so get the required
memory barrier. For that to work we also have to preload comparand into
the initial jint value we expect to see - as Kim pointed out.
Additionally Kim updated the code to get rid of C-style casts and direct
pointer arithmetic. I kept some of the intermediate locals for improved
readability.
Testing:
The only platform that potentially uses the shared implementation is
solaris_sparc (the others all have specialized asm variants). However as
I don't have a sparc system to readily test on I had to tweak an
atomic_linux_x86.hpp to use this on linux-x86.
Then it turns out that we don't have any existing callers of the
cmpxchg(byte) variant, because at the Java-level in Unsafe it has also
been written in terms of cmpxchg(int) [this will probably be fixed so
that intrinsics can be used]. So I had to modify Unsafe.java and
unsafe.cpp to add in a variant that uses Atomic::cmpxchg(jbyte*), and
then I modified the VarHandles code to use those new variants and then
ran the VarHandles tests. Those tests passed with both the updated
shared variant and the linux-x86 specific version.
Thanks,
David
More information about the hotspot-runtime-dev
mailing list