Request for reviews (S): 6863420: os::javaTimeNanos() go backward on Solaris x86

[Vladimir Kozlov]

Thank you, David

I will update Atomic::load().

Vladimir

David Holmes - Sun Microsystems wrote:
> Hi Vladimir,
> 
> This is a good catch in some quite old code! I wonder if there are any 
> other word-tearing bugs lurking ... "normal" CAS-loops wouldn't be 
> affected because the word-tearing would cause the CAS to fail, but any 
> "optimized" ones where the value normally updated by CAS is simply read 
> and used, would suffer the same problem as here.
> 
> But it seems odd to define a platform-independent Atomic::load(jlong, 
> jlong) function but then only implement it for 32-bit Solaris x86 and 
> use conditional compilation at the call site. It would seem much cleaner 
> to always use Atomic::load(jlong, jlong) and have it evaporate to 
> nothing on platforms for which it is a direct load. Given the spread of 
> the JDK I suspect there are other platforms where non-atomic 64-bit 
> loads could be an issue.
> 
> Cheers,
> David Holmes
> 
> Vladimir Kozlov said the following on 07/24/09 10:56:
>> http://cr.openjdk.java.net/~kvn/6863420/webrev.00
>>
>> Fixed 6863420: os::javaTimeNanos() go backward on Solaris x86
>>
>> Problem:
>> The problem is non atomic load from max_hrtime in getTimeNanos()
>> on platforms where long is kept in 2 32-bit register.
>> The loaded value could be invalid (> current time) since registers
>> are loaded separately and store could happened in between.
>> It could be returned as result if it is greater then current time.
>> And if the next call getTimeNanos() returns the correct time
>> it will be less then previous result.
>>
>> Solution:
>> Use new atomic long load method Atomic::load() which uses
>> FPU to move long value atomically.
>>
>> Added the regression test which shows the problem.
>>
>> Reviewed by:
>>
>> Fix verified (y/n): y, regression test
>>
>> Other testing:
>> JPRT
>>