RFR(S) 8176363: Incorrect lock order for G1 PtrQueue related locks

[Mikael Gerdin]

Hi Coleen,

On 2017-03-08 19:50, coleen.phillimore at oracle.com wrote:
> This looks good.
>
> I filed https://bugs.openjdk.java.net/browse/JDK-8176393   Can you add
> some info to it?

Thanks for the review and I'll see what I can add there.

/Mikael

>
> thanks,
> Coleen
>
> On 3/8/17 9:16 AM, Mikael Gerdin wrote:
>> Hi all,
>>
>> Please review this change to the lock rank of the locks taken in the
>> G1 pre and post write barriers.
>>
>> The rank of these locks have long been a problem since even though
>> they are leaf locks semantically they have been ranked as "nonleaf"
>> locks in the lock rank system and this has caused several issues over
>> the years where a thread holding a VM mutex and attempting to write an
>> oop would in some rare cases hit a deadlock warning due to it
>> acquiring one of the CBL monitors.
>>
>> Now this problem has come up yet again with the weak JNI handles
>> bugfix where a lock rank assertion was hit yet again due to the fact
>> that some code was holding a leaf lock while resolving a weak JNI handle.
>>
>> I suggest that the ranks of the involved locks are changed to "leaf -
>> 1", allowing them to be acquired by threads holding "leaf" locks. This
>> should not cause any further problems since reducing the ranks only
>> allows the locks to be taken in more places. Both pairs of locks (the
>> SATB and DirtyCardQ ones) have an associated FL_lock which protects a
>> free list of buffers which is acquired while holding the respective
>> CBL monitors but the FL_locks have the "special" lock rank and so they
>> will still order correctly with the new "leaf - 1" ranks.
>>
>> There is some horrible stuff going on in
>> locking_enqueue_completed_buffer but I've chosen to not change that at
>> this point in time even though it relates to the relative ranks of
>> each pair of locks.
>>
>> Testing:
>> JPRT, HS tiers 2 and 3 were tested with a patch to acquire the locks
>> even more often than what is normal to make sure that the code was
>> tested properly.
>> Some testing on HS tiers 4, 5 and 6 (Linux x64)
>> JDK tiers 1, 2 and 3
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8176363
>> Webrev: http://cr.openjdk.java.net/~mgerdin/8176363/webrev.0/
>>
>> Thanks
>> /Mikael
>