[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR: 8274282: Clarify special wait assert [v2]
From:       David Holmes <dholmes () openjdk ! java ! net>
Date:       2021-09-29 4:37:35
Message-ID: w9CPOjhoDpenY8jI4YxhfdFU6Y8qu97jkYCt4VRga6w=.d0cb8bf9-32bd-4b09-9e3d-d9374e223ba9 () github ! com
[Download RAW message or body]

On Fri, 24 Sep 2021 16:59:18 GMT, Coleen Phillimore <coleenp@openjdk.org> wrote:

> > This change removes the assert and tests for Mutex::wait() only allowed with \
> > greater than nosafepoint ranked held locks. Tested with tier1-3.
> 
> Coleen Phillimore has refreshed the contents of this pull request, and previous \
> commits have been removed. The incremental views will show differences compared to \
> the previous content of the PR. The pull request contains one new commit since the \
> last revision: 
> Fix wait checking code.

Hi Coleen,

The rationale is not quite right to me here. See comments below.

Thanks,
David

src/hotspot/share/runtime/mutex.cpp line 388:

> 386:     Mutex* least = get_least_ranked_lock_besides_this(locks_owned);
> 387:     // We enforce not holding locks of rank nosafepoint or lower while waiting \
>                 for JavaThreads,
> 388:     // because the held lock has a NoSafepointVerifier so waiting on a lock \
> will block out safepoints.

The NSV is not the reason, it is  a mechanism to detect violation of the rules. The \
reason we must not block in a JavaThread is because it will block without a safepoint \
check and without becoming safepoint-safe, thus blocking out safepoints while the \
wait is in progress.

Also note that this applies to any wait() on a nosafepoint lock, not just one done \
while holding a safepoint lock.

src/hotspot/share/runtime/mutex.cpp line 389:

> 387:     // We enforce not holding locks of rank nosafepoint or lower while waiting \
>                 for JavaThreads,
> 388:     // because the held lock has a NoSafepointVerifier so waiting on a lock \
>                 will block out safepoints.
> 389:     // For NonJavaThreads, we enforce not waiting on the tty lock since that \
> could block progress also.

You are checking this for all threads not just NJTs. Is the comment wrong or the \
code?

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

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


[prev in list] [next in list] [prev in thread] [next in thread]