RFR: 8272480: Remove Mutex::access rank [v3]

[Coleen Phillimore]

On Thu, 26 Aug 2021 23:33:13 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix test.
>
> src/hotspot/share/runtime/mutex.hpp line 50:
> 
>> 48:        service        = event          +   3,
>> 49:        stackwatermark = service        +   3,
>> 50:        tty            = stackwatermark +   3,
> 
> So as we discussed on IM the tty lock is really a problem. The existing ranking means that you cannot use the tty when holding the Service_lock - and I'm very surprised that doesn't actually happen in our code. The tty really should be a leaf lock, but when we have sections of code that explicitly acquire it so that atomic logging output can be generated, then there is a risk of acquiring other locks while the tty lock is held. I'm not sure this can be solved as long as the tty lock is part of the ranking system.
> 
> Note this is just a comment as you haven't changed anything in that regard.

Yes, the tty lock is in an unfortunate place in the hierarchy.  The tty lock should not be held around the chunks of code it's held around.  Or there should be a higher level lock that protects this logging and tty lock only protects the write.  I don't know how to fix this right now.
Since changing to UL, we don't tty->print very much except in the compiler code so it's not surprising that nothing asserts for the Service_lock taking out tty lock.

> src/hotspot/share/runtime/mutexLocker.cpp line 323:
> 
>> 321:   def(JfrBuffer_lock               , PaddedMutex  , leaf,        true,  _safepoint_check_never);
>> 322:   def(JfrStream_lock               , PaddedMutex  , nonleaf + 1, false, _safepoint_check_never);
>> 323:   def(JfrStacktrace_lock           , PaddedMutex  , stackwatermark-1,  true,  _safepoint_check_never);
> 
> Not clear why this one has to be less than the SWM rather than just remain less than tty?

The reason this was tty-2 was that I assume that this lock is taken when the stackwatermark lock is held, when stackwatermark was tty-1, and not when the tty lock is held, since it's not compiler code.
When I make it the same rank as stackwatermark I get:
     failed: Attempting to acquire lock JfrStacktrace_lock/6 out of order with lock StackWatermark_lock/6 -- possible deadlock

-------------

PR: https://git.openjdk.java.net/jdk/pull/5256