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 <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index e04c1a8459345c7f2f8471e8a7c3aa1d428414bd..5710cd4bd7f09978f97cddb6f3ce124b13707c71 100644 (file)
--- 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