From 24070cf747d8b3f4487f1ca7247dcdf0ac266e8c Mon Sep 17 00:00:00 2001 From: Andy Pan Date: Fri, 15 Dec 2023 17:19:08 +0800 Subject: [PATCH] runtime: fix the potential race of idle P's during injectglist Fixes #63555 Change-Id: I26e7baf58fcb78e66e0feed5725e371e25d657cc Reviewed-on: https://go-review.googlesource.com/c/go/+/550175 Reviewed-by: Michael Pratt LUCI-TryBot-Result: Go LUCI Reviewed-by: Carlos Amedee --- src/runtime/proc.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index d4919e56fd6..cbd30228022 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -3816,8 +3816,10 @@ func injectglist(glist *gList) { } npidle := int(sched.npidle.Load()) - var globq gQueue - var n int + var ( + globq gQueue + n int + ) for n = 0; n < npidle && !q.empty(); n++ { g := q.pop() globq.pushBack(g) @@ -3833,6 +3835,21 @@ func injectglist(glist *gList) { if !q.empty() { runqputbatch(pp, &q, qsize) } + + // Some P's might have become idle after we loaded `sched.npidle` + // but before any goroutines were added to the queue, which could + // lead to idle P's when there is work available in the global queue. + // That could potentially last until other goroutines become ready + // to run. That said, we need to find a way to hedge + // + // Calling wakep() here is the best bet, it will do nothing in the + // common case (no racing on `sched.npidle`), while it could wake one + // more P to execute G's, which might end up with >1 P's: the first one + // wakes another P and so forth until there is no more work, but this + // ought to be an extremely rare case. + // + // Also see "Worker thread parking/unparking" comment at the top of the file for details. + wakep() } // One round of scheduler: find a runnable goroutine and execute it.