RFR(s): 8212108: SafepointSynchronizer never ending counter (big enough)

[Robbin Ehn]

Hi David,

On 11/22/18 3:37 AM, David Holmes wrote:
> Hi Robbin,
> 
> On 21/11/2018 11:45 pm, Robbin Ehn wrote:
>> Hi all, please review.
>>
>> The counter can loop around, it would mainly be an issue in reading
>> such flight- recorder recording. I have two upcoming features which
>> uses the fact that it is always increasing >
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8212108
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rehn/8212108/webrev/
> 
> I don't understand the logic for safepoint_counter_addr(). Why is endian-ness an 
> issue here and why do you need to treat it as 32-bit in that case??

Because it is suppose to return the address to 32-bit counter. Returning the
address to the 4 low bytes save me for updating a lot of assembly code in
jniFastGetField.

> 
> I've been trying to see if there could be any issues with word-tearing on 32-bit 
> systems ... I can't convince myself you may not see false indicators if the 
> count is read at the same time as being updated and the 32-bit count is going to 
> rollover from 0xFFFFFFFF to 0x00000000.

The jniFastGetField should only checks if it's different.
It does:
load counter
if counter is odd (safepoint) => slowpath
speculative load value (which handles e.g. SIGSEGV/SIGBUS)
load counter
if counter != counter => slowpath
/* value is good */

> 
> I'm not familiar with JFR types - is "ulong" guaranteed to be 64-bit?

It translate to an u8, so yes.

> 
>> Passes t1-3 and the two updated FR tests.
> 
> Does any of that actually run long enough to overflow a 32-bit counter?

We have no tests that run long enough to rollover.

Thanks, Robbin

> 
> Thanks,
> David
> 
>>
>> Thanks Robbin