RFR 8213150: Add verification for locking by VMThread

[coleen.phillimore at oracle.com]

Hey Robbin, what do you think?

On 9/24/19 10:48 AM, coleen.phillimore at oracle.com wrote:

>
> On 9/24/19 2:28 AM, Robbin Ehn wrote:
>> Hi Coleen,
>>
>> First, a side note, I notice that we forgot to change, e.g.:
>> Monitor::_safepoint_check_never to Mutex::_safepoint_check_never in 
>> some places.
>
> I'll fix these two:
>
> http://cr.openjdk.java.net/~coleenp/2019/8213150.02/webrev/src/hotspot/share/utilities/events.hpp.udiff.html 
>
>
> http://cr.openjdk.java.net/~coleenp/2019/8213150.02/webrev/src/hotspot/share/utilities/concurrentHashTable.inline.hpp.udiff.html 
>
>
> And file an RFE for the ones in mutexLocker.cpp.
>
> I'd left Monitor::_safepoint_check_never in places where the code had 
> new Monitor(etc, Monitor::_safepoint_check_never); ie. the code was 
> initializing a Monitor, but there are a lot of mechanical changes that 
> we could do to change them all.

I had a version that changed all Monitor::_no_safepoint_check_flag, and 
Monitor::_safepoint_check_never to Mutex::* but it looked bad changing 
it in the code that had:

MonitorLocker ml(Lock, Monitor::_no_safepoint_check_flag);

so I didn't.

open webrev at http://cr.openjdk.java.net/~coleenp/2019/8231472.01/webrev
bug link https://bugs.openjdk.java.net/browse/JDK-8231472

Worth doing?

thanks,
Coleen

>
> thanks!
> Coleen
>
>>
>>>
>>> I'm going to work on the issues with 'special' locks as part of : 
>>> https://bugs.openjdk.java.net/browse/JDK-8184732 next, but the 
>>> rewriting (below) removed these two uses of this ranking check.
>>
>> Ok
>>
>>>
>>>>
>>>> do_safepoint_check is checked four times (once via not_allowed), 
>>>> can you please re-write this method, or better yet remove the bool 
>>>> input.
>>>> The bool is hardcoded so can you just create two methods, with one 
>>>> helper method for the common checks. (yes there will be more code, 
>>>> but readable :) )
>>>
>>> This is a good suggestion.  I've rewritten it to be a lot clearer, 
>>> and am retesting it now with tier1 (all Oracle platforms) and 
>>> tier2,3 linux-x64-debug.
>>>
>>> incremental webrev at 
>>> http://cr.openjdk.java.net/~coleenp/2019/8213150.02.incr/webrev
>>> full webrev at 
>>> http://cr.openjdk.java.net/~coleenp/2019/8213150.02/webrev
>>
>> Thanks!
>>
>> /Robbin
>>
>>>
>>> Thanks,
>>> Coleen
>>>
>>>>
>>>> Thanks, Robbin
>>>>
>>>> On 9/20/19 11:36 PM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: extend verification for all locking not just 
>>>>> VMOperations, and fix CLDG lock to not be taken by VM thread.
>>>>>
>>>>> See bug comments for more details about this change.
>>>>>
>>>>> Tested with tier1 all Oracle platforms, tier2-8 linux-x64-debug.
>>>>>
>>>>> open webrev at 
>>>>> http://cr.openjdk.java.net/~coleenp/2019/8213150.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8213150
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>
>