From a8cf2a90bc76efcbbd51da04b4c4388e3f1fea62 Mon Sep 17 00:00:00 2001 From: hulin Date: Fri, 15 Dec 2023 11:38:20 -0500 Subject: [PATCH] Fix PowerPC specific issues -Replace do_{_if_(condition)_{_..._continute;_}_}while_{false}; with for(;;)_{_if_(condition)_{...}_else{break;_}_} for an infinite loop with a break logic to handle waiting concurrent gc finish during continuation mounting. compiler might generate wrong logic for do_while infinite loop. -fix small hole during mounting synchronization with concurrent scanning(for the case concurrent scanning start just before resetting pendingmount flag-before reading the value of state for atomic update) - new assertion check for catching issue earlier. signed-off-by: hulin --- runtime/vm/ContinuationHelpers.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/runtime/vm/ContinuationHelpers.cpp b/runtime/vm/ContinuationHelpers.cpp index 9e9afabd338..69c2200897a 100644 --- a/runtime/vm/ContinuationHelpers.cpp +++ b/runtime/vm/ContinuationHelpers.cpp @@ -152,6 +152,7 @@ synchronizeWithConcurrentGCScan(J9VMThread *currentThread, j9object_t continuati { ContinuationState oldContinuationState = 0; ContinuationState returnContinuationState = 0; + /* Set pending state and carrier id, GC need to distinguish between fully mounted (don't scan) vs pending to be mounted (do scan) cases. */ do { oldContinuationState = *continuationStatePtr; @@ -167,10 +168,11 @@ synchronizeWithConcurrentGCScan(J9VMThread *currentThread, j9object_t continuati * but only up to small finite number of times. */ do { - if (VM_ContinuationHelpers::isConcurrentlyScanned(returnContinuationState)) { + oldContinuationState = *continuationStatePtr; + if (VM_ContinuationHelpers::isConcurrentlyScanned(oldContinuationState)) { omrthread_monitor_enter(currentThread->publicFlagsMutex); /* Wait till potentially concurrent GC scans are complete */ - do { + for(;;) { oldContinuationState = *continuationStatePtr; if (VM_ContinuationHelpers::isConcurrentlyScanned(oldContinuationState)) { PUSH_OBJECT_IN_SPECIAL_FRAME(currentThread, continuationObject); @@ -183,13 +185,13 @@ synchronizeWithConcurrentGCScan(J9VMThread *currentThread, j9object_t continuati continuationObject = POP_OBJECT_IN_SPECIAL_FRAME(currentThread); /* Object might have moved - update its address and the address of the state slot. */ continuationStatePtr = VM_ContinuationHelpers::getContinuationStateAddress(currentThread, continuationObject); - continue; + } else { + break; } - } while (false); + } omrthread_monitor_exit(currentThread->publicFlagsMutex); } /* Remove pending state */ - oldContinuationState = *continuationStatePtr; Assert_VM_true(VM_ContinuationHelpers::isMountedWithCarrierThread(oldContinuationState, currentThread)); Assert_VM_true(VM_ContinuationHelpers::isPendingToBeMounted(oldContinuationState)); ContinuationState newContinuationState = oldContinuationState; @@ -197,6 +199,7 @@ synchronizeWithConcurrentGCScan(J9VMThread *currentThread, j9object_t continuati returnContinuationState = VM_AtomicSupport::lockCompareExchange(continuationStatePtr, oldContinuationState, newContinuationState); } while (oldContinuationState != returnContinuationState); Assert_VM_false(VM_ContinuationHelpers::isPendingToBeMounted(*continuationStatePtr)); + Assert_VM_false(VM_ContinuationHelpers::isConcurrentlyScanned(*continuationStatePtr)); return continuationObject; } @@ -306,7 +309,6 @@ yieldContinuation(J9VMThread *currentThread, BOOLEAN isFinished) */ Assert_VM_true(VM_ContinuationHelpers::isMountedWithCarrierThread(*continuationStatePtr, currentThread)); VM_ContinuationHelpers::resetCarrierID(continuationStatePtr); - VM_AtomicSupport::writeBarrier(); /* Logically postUnmountContinuation(), which add the related continuation Object to the rememberedSet or dirty the Card for concurrent marking for future scanning, should be called * before resetCarrierID(), but the scan might happened before resetCarrierID() if concurrent card clean happens, then the related compensating scan might be * missed due to the continuation still is stated as mounted(we don't scan any mounted continuation, it should be scanned during root scanning via J9VMThread->currentContinuation).