diff --git a/src/runtime/proc.go b/src/runtime/proc.go index ba02b14995..378d5e32f5 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -3132,24 +3132,33 @@ func checkIdleGCNoP() (*p, *g) { return nil, nil } - // Work is available; we can start an idle GC worker only if - // there is an available P and available worker G. + // Work is available; we can start an idle GC worker only if there is + // an available P and available worker G. // - // We can attempt to acquire these in either order. Workers are - // almost always available (see comment in findRunnableGCWorker - // for the one case there may be none). Since we're slightly - // less likely to find a P, check for that first. + // We can attempt to acquire these in either order, though both have + // synchonization concerns (see below). Workers are almost always + // available (see comment in findRunnableGCWorker for the one case + // there may be none). Since we're slightly less likely to find a P, + // check for that first. + // + // Synchronization: note that we must hold sched.lock until we are + // committed to keeping it. Otherwise we cannot put the unnecessary P + // back in sched.pidle without performing the full set of idle + // transition checks. + // + // If we were to check gcBgMarkWorkerPool first, we must somehow handle + // the assumption in gcControllerState.findRunnableGCWorker that an + // empty gcBgMarkWorkerPool is only possible if gcMarkDone is running. lock(&sched.lock) pp := pidleget() - unlock(&sched.lock) if pp == nil { + unlock(&sched.lock) return nil, nil } - // Now that we own a P, gcBlackenEnabled can't change - // (as it requires STW). + // Now that we own a P, gcBlackenEnabled can't change (as it requires + // STW). if gcBlackenEnabled == 0 { - lock(&sched.lock) pidleput(pp) unlock(&sched.lock) return nil, nil @@ -3157,12 +3166,13 @@ func checkIdleGCNoP() (*p, *g) { node := (*gcBgMarkWorkerNode)(gcBgMarkWorkerPool.pop()) if node == nil { - lock(&sched.lock) pidleput(pp) unlock(&sched.lock) return nil, nil } + unlock(&sched.lock) + return pp, node.gp.ptr() }