[aarch64-port-dev ] 8153107: Unbalanced recursive locking
Andrey Petushkov
andrey.petushkov at gmail.com
Thu Jun 14 11:59:11 UTC 2018
So then if you prefer to leave the different logic in shared code between
quick and slow paths I believe the fix for cpu/arm implementation (and
removal of unnecessary workaround for cpu/aarch64) should look like this:
--- CUT ---
diff -r db65921e9a9b src/hotspot/cpu/arm/macroAssembler_arm.cpp
--- a/src/hotspot/cpu/arm/macroAssembler_arm.cpp Thu Jun 07 06:27:09 2018
-0400
+++ b/src/hotspot/cpu/arm/macroAssembler_arm.cpp Thu Jun 14 14:56:38 2018
+0300
@@ -3014,8 +3014,8 @@
mov(Rscratch, SP);
sub(Rscratch, Rmark, Rscratch);
ands(Rscratch, Rscratch, imm);
- b(done, ne); // exit with failure
- str(Rscratch, Address(Rbox,
BasicLock::displaced_header_offset_in_bytes())); // set to zero
+ // set to zero if recursive lock, set to non zero otherwise (see
discussion in JDK-8153107)
+ str(Rscratch, Address(Rbox,
BasicLock::displaced_header_offset_in_bytes()));
b(done);
#else
@@ -3025,7 +3025,8 @@
sub(Rscratch, Rmark, SP, eq);
movs(Rscratch, AsmOperand(Rscratch, lsr,
exact_log2(os::vm_page_size())), eq);
// If still 'eq' then recursive locking OK
- str(Rscratch, Address(Rbox,
BasicLock::displaced_header_offset_in_bytes()), eq); // set to zero
+ // set to zero if recursive lock, set to non zero otherwise (see
discussion in JDK-8153107)
+ str(Rscratch, Address(Rbox,
BasicLock::displaced_header_offset_in_bytes()));
b(done);
#endif
diff -r db65921e9a9b src/hotspot/share/runtime/sharedRuntime.cpp
--- a/src/hotspot/share/runtime/sharedRuntime.cpp Thu Jun 07 06:27:09 2018
-0400
+++ b/src/hotspot/share/runtime/sharedRuntime.cpp Thu Jun 14 14:56:38 2018
+0300
@@ -1983,11 +1983,7 @@
// Handles the uncommon case in locking, i.e., contention or an inflated
lock.
JRT_BLOCK_ENTRY(void, SharedRuntime::complete_monitor_locking_C(oopDesc*
_obj, BasicLock* lock, JavaThread* thread))
- // Disable ObjectSynchronizer::quick_enter() in default config
- // on AARCH64 and ARM until JDK-8153107 is resolved.
- if (ARM_ONLY((SyncFlags & 256) != 0 &&)
- AARCH64_ONLY((SyncFlags & 256) != 0 &&)
- !SafepointSynchronize::is_synchronizing()) {
+ if (!SafepointSynchronize::is_synchronizing()) {
// Only try quick_enter() if we're not trying to reach a safepoint
// so that the calling thread reaches the safepoint more quickly.
if (ObjectSynchronizer::quick_enter(_obj, thread, lock)) return;
--- END CUT ---
Tested on jdk10/port-aarch32 codebase
Regards,
Andrey
On Wed, Jun 13, 2018 at 8:26 PM Andrey Petushkov <andrey.petushkov at gmail.com>
wrote:
> Ok, I shall I admit I did not dig deep enough. Yes, there is platform
> specifics which plays a role here. That is: all platforms except Oracle ARM
> (all bitness) and aarch32 fill-in stack lock’s DHW with result of
> same-stack-page-lock probe result. Which is always !=0 when failed.
> Oracle’s ARM and aarch32 do that conditionally, based on whether this test
> actually passes. Hence for the latter platforms the DHW in the stack lock
> really contains random value which occasionally can be 0, and being not
> rewritten (e.g. with markOopDesc::unused_mark(), aka 3) will indicate
> recursive stack lock for ::fast_unlock(). Not sure about those stack
> walkers mentioned in the comment in ::quick_enter(). So, right, it’s
> possible to fix the problem in the platform-specific code only. Leaving up
> to you to decide whether it’s proper design :)
>
> Regards,
> Andrey
>
> > On 13 Jun 2018, at 18:17, Daniel D. Daugherty <
> daniel.daugherty at oracle.com> wrote:
> >
> > Not at my fingertips, but I will look thru my archives.
> >
> > Dan
> >
> >
> > On 6/13/18 11:14 AM, Andrew Haley wrote:
> >> Hi Dan,
> >>
> >> Have you got any torture tests for recursive locking?
> >>
> >
>
>
More information about the aarch32-port-dev
mailing list