From b89033399894776d677a4cecc82e2ac888fd7906 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Mon, 23 Nov 2015 11:37:12 -0500 Subject: [PATCH] runtime: clean up gcMarkDone This improves the documentation comment on gcMarkDone, replaces a recursive call with a simple goto, and disables preemption before stopping the world in accordance with the documentation comment on stopTheWorldWithSema. Updates #13363, but, sadly, doesn't fix it. Change-Id: I6cb2a5836b35685bf82f7b1ce7e48a7625906656 Reviewed-on: https://go-review.googlesource.com/17149 Reviewed-by: Rick Hudson Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot --- src/runtime/mgc.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index e04c1a84593..5710cd4bd7f 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1027,7 +1027,15 @@ func gcStart(mode gcMode, forceTrigger bool) { // active work buffers in assists and background workers; however, // work may still be cached in per-P work buffers. In mark 2, per-P // caches are disabled. +// +// The calling context must be preemptible. +// +// Note that it is explicitly okay to have write barriers in this +// function because completion of concurrent mark is best-effort +// anyway. Any work created by write barriers here will be cleaned up +// by mark termination. func gcMarkDone() { +top: semacquire(&work.markDoneSema, false) // Re-check transition condition under transition lock. @@ -1090,15 +1098,17 @@ func gcMarkDone() { incnwait := atomic.Xadd(&work.nwait, +1) if incnwait == work.nproc && !gcMarkWorkAvailable(nil) { - // This recursion is safe because the call - // can't take this same "if" branch. - gcMarkDone() + // This loop will make progress because + // gcBlackenPromptly is now true, so it won't + // take this same "if" branch. + goto top } } else { // Transition to mark termination. now := nanotime() work.tMarkTerm = now work.pauseStart = now + getg().m.preemptoff = "gcing" systemstack(stopTheWorldWithSema) // The gcphase is _GCmark, it will transition to _GCmarktermination // below. The important thing is that the wb remains active until