Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PowerPC specific issues #18637

Merged
merged 1 commit into from
Dec 19, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions runtime/vm/ContinuationHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -183,20 +185,21 @@ 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;
VM_ContinuationHelpers::resetPendingState(&newContinuationState);
returnContinuationState = VM_AtomicSupport::lockCompareExchange(continuationStatePtr, oldContinuationState, newContinuationState);
} while (oldContinuationState != returnContinuationState);
Assert_VM_false(VM_ContinuationHelpers::isPendingToBeMounted(*continuationStatePtr));
Assert_VM_false(VM_ContinuationHelpers::isConcurrentlyScanned(*continuationStatePtr));
Copy link
Contributor

@amicic amicic Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think this assert is ambitious since it's based on newly re-read state. Another scan can start just after this one finished. It could be a rescan within the same cycle (concurrent card cleaning) or in another cycle of same type or overlapping cycle of the complement type.

My bad, no new scan can start while this is fully mounted. So, the assert is ok.


return continuationObject;
}
Expand Down Expand Up @@ -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).
Expand Down