RFR (S): 8008314 - Unimplemented() Atomic::load breaks the applications

David Holmes david.holmes at oracle.com
Mon Feb 18 23:08:43 PST 2013


Hi Jesper,

The jlong version of Atomic::add not only relies on a jlong version of 
Atomic::load but also a jlong version of Atomic::cmpxchg - facilities 
that are only conditionally available on 32-bit platforms. Further, the 
jlong version of add implements the wrong semantics by returning the 
previous value rather than the updated value as it should (as per 
atomic.hpp).

Given that the jlong Atomic::add will not be used once this is pushed 
(assuming Bengt pushes 8008382 first), could I ask that you also delete 
it from atomic.cpp/hpp as part of this change. That way no one will be 
tempted to use it again. I'd also suggest adding a comment in atomic.hpp:

// Atomic operations on jlong types are not available on all 32-bit
// platforms. If atomic ops on jlongs are defined here they must only
// be used from code that verifies they are available at runtime and
// can provide an alternative action if not - see supports_cx8() for
// a means to test availability

There remain Atomic::store(jlong) and Atomic::load(jlong) operations 
that violate this rule but the code involved is not presently used on 
platforms that don't support them. This obviously needs fixing.

Thanks,
David

On 19/02/2013 4:25 PM, Jesper Wilhelmsson wrote:
> Hi,
>
> Please review the following change:
>
> Webrev: http://cr.openjdk.java.net/~jwilhelm/8008314/
>
> Bug: https://jbs.oracle.com/bugs/browse/JDK-8008314
>
>
> The use of atomic::add(jlong,jlong*) breaks everything on ARM since it
> uses atomic::load(jlong) which isn't implemented.
>
> The add was used to update a counter in rotating log file handling, but
> it turns out that the counter that was updated doesn't need to be a jlong.
>
> The counter is used to indicate that it is time to do a log file
> rotation and by adding a saftey catch for overflow, we can use an uintx
> for the counter.
>
> In practice this means that the upper "limit" for the log file size
> becomes UINX_MAX-1 (used to be UINT_MAX) since I change from < to <= in
> the comparison to the limit, but since this is not a hard limit the
> difference will only be noticeable when someone has written UINT_MAX
> characters to the log file and the next write will end up in the next
> log file part instead of in the same (which one could argue is a more
> correct behavior).
>
> Sine I changed all places where the counter was used I also fixed a typo
> in its name.
>
> Thanks,
> /Jesper


More information about the hotspot-dev mailing list