From c0604fb823d9f3b2e347a9857b11606b223ad8ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20=C3=96sterlund?= Date: Thu, 4 Jul 2024 10:06:09 +0000 Subject: [PATCH] 8334890: Missing unconditional cross modifying fence in nmethod entry barriers Reviewed-by: aboldtch, kbarrett --- .../share/gc/shared/barrierSetNMethod.cpp | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/hotspot/share/gc/shared/barrierSetNMethod.cpp b/src/hotspot/share/gc/shared/barrierSetNMethod.cpp index 79e3f47ed57..f041a2ccce1 100644 --- a/src/hotspot/share/gc/shared/barrierSetNMethod.cpp +++ b/src/hotspot/share/gc/shared/barrierSetNMethod.cpp @@ -178,16 +178,9 @@ int BarrierSetNMethod::nmethod_stub_entry_barrier(address* return_address_ptr) { nmethod* nm = cb->as_nmethod(); BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod(); - // Check for disarmed method here to avoid going into DeoptimizeNMethodBarriersALot code - // too often. nmethod_entry_barrier checks for disarmed status itself, - // but we have no visibility into whether the barrier acted or not. - if (!bs_nm->is_armed(nm)) { - return 0; - } - - assert(!nm->is_osr_method(), "Should not reach here"); // Called upon first entry after being armed bool may_enter = bs_nm->nmethod_entry_barrier(nm); + assert(!nm->is_osr_method() || may_enter, "OSR nmethods should always be entrant after migration"); // In case a concurrent thread disarmed the nmethod, we need to ensure the new instructions // are made visible, by using a cross modify fence. Note that this is synchronous cross modifying @@ -197,11 +190,11 @@ int BarrierSetNMethod::nmethod_stub_entry_barrier(address* return_address_ptr) { // it can be made conditional on the nmethod_patching_type. OrderAccess::cross_modify_fence(); - // Diagnostic option to force deoptimization 1 in 3 times. It is otherwise + // Diagnostic option to force deoptimization 1 in 10 times. It is otherwise // a very rare event. - if (DeoptimizeNMethodBarriersALot) { + if (DeoptimizeNMethodBarriersALot && !nm->is_osr_method()) { static volatile uint32_t counter=0; - if (Atomic::add(&counter, 1u) % 3 == 0) { + if (Atomic::add(&counter, 1u) % 10 == 0) { may_enter = false; } } @@ -214,15 +207,6 @@ int BarrierSetNMethod::nmethod_stub_entry_barrier(address* return_address_ptr) { } bool BarrierSetNMethod::nmethod_osr_entry_barrier(nmethod* nm) { - // This check depends on the invariant that all nmethods that are deoptimized / made not entrant - // are NOT disarmed. - // This invariant is important because a method can be deoptimized after the method have been - // resolved / looked up by OSR by another thread. By not deoptimizing them we guarantee that - // a deoptimized method will always hit the barrier and come to the same conclusion - deoptimize - if (!is_armed(nm)) { - return true; - } - assert(nm->is_osr_method(), "Should not reach here"); log_trace(nmethod, barrier)("Running osr nmethod entry barrier: " PTR_FORMAT, p2i(nm)); bool result = nmethod_entry_barrier(nm);