RFR: 8273300: Check Mutex ranking during a safepoint [v2]
David Holmes
dholmes at openjdk.java.net
Sun Sep 12 22:36:48 UTC 2021
On Fri, 10 Sep 2021 21:15:29 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>> This change checks lock ranking during a safepoint. For some reason, safepoint checking was excluded, probably from the days where Safepoint_lock and Threads_lock were used.
>> Because of checking during a safepoint, some locks had to get lower ranks. The CR has the details of which locks these were. The Service_lock complicates things because it's held during oops_do, which may take out other G1 locks.
>> This was built and tested with Shenandoah. Thanks to @zhengyu123 for the changes in Shenandoah.
>> Tests run tier1-8.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix Shenandoah mismerge
Hi Coleen,
The core change seems clean and simple, but the fanout of the ranking changes is far less clear and some of this looks like it could/should be separated out - see comments below.
If I see a lock with a rank service-1, then should I infer that lock can be acquired while the service (or notification) lock is held? And that if lock A is service-1 and lock B is service-2, then B can be acquired while holding A?
Thanks,
David
src/hotspot/share/memory/universe.cpp line 125:
> 123: LatestMethodCache* Universe::_do_stack_walk_cache = NULL;
> 124:
> 125: bool Universe::_verify_in_progress = false;
This cleanup seems completely unrelated to your mutex change and is best left to a separate cleanup RFE.
src/hotspot/share/memory/universe.cpp line 1116:
> 1114: }
> 1115: if (should_verify_subset(Verify_CodeCache)) {
> 1116: MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
Is this needed to allow the new rankings to work? And is this enabled by the _verify_in_progress change? If so I'd rather see all of that related stuff changed first in a separate RFE that can easily be independently backported.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5467
More information about the shenandoah-dev
mailing list