RFR (S): 8182703: Correct G1 barrier queue lock orderings

[Kim Barrett]

On 2017-07-06 22:15, Kim Barrett wrote:
>> On Jul 6, 2017, at 4:11 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>> 
>>> On Jul 4, 2017, at 10:00 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>> The lock ranking changes look good.
>> I'm going to retract that.
>> 
>> How does these new lock rankings interact with various assertions that
>> rank() == or != Mutex::special?  I'm not sure those places handle
>> these new ranks properly.  (I'm not sure those places handle
>> Mutex::event rank properly either.)

> On Jul 7, 2017, at 11:17 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
> 
> […]
> All in all, I believe that the deadlock detecion system has some redundant, and some confusing checks that involve the lock rank Mutex::special. But I do believe that it works and would detect deadlocks, but could do with some reworking to make it more explicit. And that is invariant of the new access rank and applies equally to the event rank.
> 
> However, since these access locks play well with the current deadlock detection as they do not do anything illegal, and even if use of these locks did indeed do illegal things, it would still be detected by the deadlock detection system, it is reasonable to say that refactoring the deadlock detection system is a separate RFE?
> 
> Specifically, clarifying the deadlock detection system by removing redundant checks, not checking for safepoint-safe state in try_lock as well as explicitly listing special and below locks as illegal when verifying Thread::check_for_valid_safepoint_state(), regardles of whether allow_vm_block() is true or not. Sounds like a separate RFE to me!

Thanks for the additional analysis. I agree that so long as one does
what one is supposed to (e.g. these locks always need to avoid
safepoint checks), there won't be any undesired assertions.  And I
also agree there won't be any bad consequences (e.g. incorrect code
possibly slipping through) from misuse, though the indicative failures
might not always be where one might prefer.

I don't think the redundant checks are necessarily bad, as they make
it more obvious to future readers what the requirements are at various
levels.  However, I agree it should be a separate RFE to do some
cleanup in this area, particularly where [non-]equality with
Mutex::special ought to be an ordered comparison.