runtime: avoid double-scanning of stacks

Currently there's a race between stopg scanning another G's stack and
the G reaching a preemption point and scanning its own stack. When
this race occurs, the G's stack is scanned twice. Currently this is
okay, so this race is benign.

However, we will shortly be adding stack barriers during the first
stack scan, so scanning will no longer be idempotent. To prepare for
this, this change ensures that each stack is scanned only once during
each GC phase by checking the flag that indicates that the stack has
been scanned in this phase before scanning the stack.

Change-Id: Id9f4d5e2e5b839bc3f200ec1723a4a12dd677ab4
Reviewed-on: https://go-review.googlesource.com/10458
Reviewed-by: Rick Hudson <rlh@golang.org>

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 6bc2d73d557c0719ca475c1456e1fc17ea1f24e7..c4c922bda8d65ea4f876c943a43eb3a51821b5d0 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -258,6 +258,9 @@ func gcAssistAlloc(size uintptr, allowAssist bool) {
 // work is done here.
 //go:nowritebarrier
 func gcphasework(gp *g) {
+       if gp.gcworkdone {
+               return
+       }
        switch gcphase {
        default:
                throw("gcphasework in bad gcphase")

diff --git a/src/runtime/stack1.go b/src/runtime/stack1.go
index f77e87cdf9c73bed13b4263f69c68c643d4785e2..5c2388d0e6f85d449aa2b73aa5d8c462bad1a6bf 100644 (file)
--- a/src/runtime/stack1.go
+++ b/src/runtime/stack1.go
@@ -743,8 +743,11 @@ func newstack() {
                }
                if gp.preemptscan {
                        for !castogscanstatus(gp, _Gwaiting, _Gscanwaiting) {
-                               // Likely to be racing with the GC as it sees a _Gwaiting and does the stack scan.
-                               // If so this stack will be scanned twice which does not change correctness.
+                               // Likely to be racing with the GC as
+                               // it sees a _Gwaiting and does the
+                               // stack scan. If so, gcworkdone will
+                               // be set and gcphasework will simply
+                               // return.
                        }
                        gcphasework(gp)
                        casfrom_Gscanstatus(gp, _Gscanwaiting, _Gwaiting)