Request for review (S): 8008382: Remove redundant use of Atomic::add(jlong, jlong *) in create_new_gc_id()

Erik Helin erik.helin at
Tue Feb 19 12:35:09 UTC 2013


On 02/19/2013 11:15 AM, Bengt Rutisson wrote:
> Erik Helin pointed out a potential race in CMS where the concurrent
> cycle may be trying to increment the GC id in parallel with a young
> collection.

I just would like to add that Mikael Gerdin also investigated the race, 
thanks Mikael!

On 02/19/2013 11:15 AM, Bengt Rutisson wrote:
> This is scary for other reasons as well but removing the
> atomic call exposes it. We concluded that the safe change is to move the
> call to register_gc_start() in to the CMS VM operation. This way we get
> the VM operation synchronization and can be sure that two threads are
> never incrementing the GC id in parallel.
> Updated webrev here:

Looks good!


> Bengt
> On 2/18/13 2:21 PM, Bengt Rutisson wrote:
>> Hi all,
>> Could I have a couple of reviews for this change?
>> There is no need to use atomics in create_new_gc_id() since it is not
>> called by multiple threads in parallel. Also, Atomic::add(jlong, jlong
>> *) is broken for ARM.
>> We should remove the use of Atomic::add in create_new_gc_id() for now.
>> If we need this to be called by multiple threads in the future we have
>> to reconsider how this should be done. Either adding a lock or doing
>> some kind of 32 bit atomic work.
>> Thanks,
>> Bengt

More information about the hotspot-gc-dev mailing list