Skip to content

Commit

Permalink
[Backport] 8166197: assert(RelaxAssert || w != Thread::current()->_Mu…
Browse files Browse the repository at this point in the history
…texEvent) failed: invariant (#648)

Summary: Add load-acquire and store-release on share variable _OnDeck to ensure Mutex work on AArch64.

Testing: No testcase becuase the frequency is occasionally when run G1 gc with high cpu usage.

Reviewers: kuaiwei, yuleil

Issue: #647
  • Loading branch information
lingjun-cg authored Jun 21, 2024
1 parent 3907663 commit cf827ca
Showing 1 changed file with 26 additions and 21 deletions.
47 changes: 26 additions & 21 deletions hotspot/src/share/vm/runtime/mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ void Monitor::ILock (Thread * Self) {
ParkEvent * const ESelf = Self->_MutexEvent ;
assert (_OnDeck != ESelf, "invariant") ;

// As an optimization, spinners could conditionally try to set ONDECK to _LBIT
// As an optimization, spinners could conditionally try to set _OnDeck to _LBIT
// Synchronizer.cpp uses a similar optimization.
if (TrySpin (Self)) goto Exeunt ;

Expand All @@ -466,22 +466,22 @@ void Monitor::ILock (Thread * Self) {
OrderAccess::fence() ;

// Optional optimization ... try barging on the inner lock
if ((NativeMonitorFlags & 32) && CASPTR (&_OnDeck, NULL, UNS(Self)) == 0) {
goto OnDeck_LOOP ;
if ((NativeMonitorFlags & 32) && CASPTR (&_OnDeck, NULL, UNS(ESelf)) == 0) {
goto OnDeck_LOOP;
}

if (AcquireOrPush (ESelf)) goto Exeunt ;

// At any given time there is at most one ondeck thread.
// ondeck implies not resident on cxq and not resident on EntryList
// Only the OnDeck thread can try to acquire -- contended for -- the lock.
// Only the OnDeck thread can try to acquire -- contend for -- the lock.
// CONSIDER: use Self->OnDeck instead of m->OnDeck.
// Deschedule Self so that others may run.
while (_OnDeck != ESelf) {
ParkCommon (ESelf, 0) ;
while (OrderAccess::load_ptr_acquire(&_OnDeck) != ESelf) {
ParkCommon(ESelf, 0);
}

// Self is now in the ONDECK position and will remain so until it
// Self is now in the OnDeck position and will remain so until it
// manages to acquire the lock.
OnDeck_LOOP:
for (;;) {
Expand All @@ -505,8 +505,8 @@ void Monitor::ILock (Thread * Self) {
// A. Shift or defer dropping the inner lock until the subsequent IUnlock() operation.
// This might avoid potential reacquisition of the inner lock in IUlock().
// B. While still holding the inner lock, attempt to opportunistically select
// and unlink the next ONDECK thread from the EntryList.
// If successful, set ONDECK to refer to that thread, otherwise clear ONDECK.
// and unlink the next OnDeck thread from the EntryList.
// If successful, set OnDeck to refer to that thread, otherwise clear OnDeck.
// It's critical that the select-and-unlink operation run in constant-time as
// it executes when holding the outer lock and may artificially increase the
// effective length of the critical section.
Expand All @@ -532,16 +532,16 @@ void Monitor::IUnlock (bool RelaxAssert) {
// safety or lock release consistency.
OrderAccess::release_store(&_LockWord.Bytes[_LSBINDEX], 0); // drop outer lock

OrderAccess::storeload ();
ParkEvent * const w = _OnDeck ;
assert (RelaxAssert || w != Thread::current()->_MutexEvent, "invariant") ;
OrderAccess::storeload();
ParkEvent * const w = _OnDeck; // raw load as we will just return if non-NULL
assert(RelaxAssert || w != Thread::current()->_MutexEvent, "invariant");
if (w != NULL) {
// Either we have a valid ondeck thread or ondeck is transiently "locked"
// by some exiting thread as it arranges for succession. The LSBit of
// OnDeck allows us to discriminate two cases. If the latter, the
// responsibility for progress and succession lies with that other thread.
// For good performance, we also depend on the fact that redundant unpark()
// operations are cheap. That is, repeated Unpark()ing of the ONDECK thread
// operations are cheap. That is, repeated Unpark()ing of the OnDeck thread
// is inexpensive. This approach provides implicit futile wakeup throttling.
// Note that the referent "w" might be stale with respect to the lock.
// In that case the following unpark() is harmless and the worst that'll happen
Expand Down Expand Up @@ -589,9 +589,14 @@ void Monitor::IUnlock (bool RelaxAssert) {
assert (RelaxAssert || w != Thread::current()->_MutexEvent, "invariant") ;
_EntryList = w->ListNext ;
// as a diagnostic measure consider setting w->_ListNext = BAD
assert (UNS(_OnDeck) == _LBIT, "invariant") ;
_OnDeck = w ; // pass OnDeck to w.
// w will clear OnDeck once it acquires the outer lock
assert(UNS(_OnDeck) == _LBIT, "invariant");

// Pass OnDeck role to w, ensuring that _EntryList has been set first.
// w will clear _OnDeck once it acquires the outer lock.
// Note that once we set _OnDeck that thread can acquire the mutex, proceed
// with its critical section and then enter this code to unlock the mutex. So
// you can have multiple threads active in IUnlock at the same time.
OrderAccess::release_store_ptr(&_OnDeck, w);

// Another optional optimization ...
// For heavily contended locks it's not uncommon that some other
Expand Down Expand Up @@ -839,8 +844,8 @@ int Monitor::IWait (Thread * Self, jlong timo) {
// ESelf is now on the cxq, EntryList or at the OnDeck position.
// The following fragment is extracted from Monitor::ILock()
for (;;) {
if (_OnDeck == ESelf && TrySpin(Self)) break ;
ParkCommon (ESelf, 0) ;
if (OrderAccess::load_ptr_acquire(&_OnDeck) == ESelf && TrySpin(Self)) break;
ParkCommon(ESelf, 0);
}
assert (_OnDeck == ESelf, "invariant") ;
_OnDeck = NULL ;
Expand Down Expand Up @@ -1047,11 +1052,11 @@ void Monitor::jvm_raw_lock() {

// At any given time there is at most one ondeck thread.
// ondeck implies not resident on cxq and not resident on EntryList
// Only the OnDeck thread can try to acquire -- contended for -- the lock.
// Only the OnDeck thread can try to acquire -- contend for -- the lock.
// CONSIDER: use Self->OnDeck instead of m->OnDeck.
for (;;) {
if (_OnDeck == ESelf && TrySpin(NULL)) break ;
ParkCommon (ESelf, 0) ;
if (OrderAccess::load_ptr_acquire(&_OnDeck) == ESelf && TrySpin(NULL)) break;
ParkCommon(ESelf, 0);
}

assert (_OnDeck == ESelf, "invariant") ;
Expand Down

0 comments on commit cf827ca

Please sign in to comment.