From 5b765ce310c594276ea919a9cb455cc894fee999 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 11 Mar 2016 14:08:10 -0500 Subject: [PATCH] runtime: don't clear gcscanvalid in casfrom_Gscanstatus Currently we clear gcscanvalid in both casgstatus and casfrom_Gscanstatus if the new status is _Grunning. This is very important to do in casgstatus. However, this is potentially wrong in casfrom_Gscanstatus because in this case the caller doesn't own gp and hence the write is racy. Unlike the other _Gscan statuses, during _Gscanrunning, the G is still running. This does not indicate that it's transitioning into a running state. The scan simply hasn't happened yet, so it's neither valid nor invalid. Conveniently, this also means clearing gcscanvalid is unnecessary in this case because the G was already in _Grunning, so we can simply remove this code. What will happen instead is that the G will be preempted to scan itself, that scan will set gcscanvalid to true, and then the G will return to _Grunning via casgstatus, clearing gcscanvalid. This fix will become necessary shortly when we start keeping track of the set of G's with dirty stacks, since it will no longer be idempotent to simply set gcscanvalid to false. Change-Id: I688c82e6fbf00d5dbbbff49efa66acb99ee86785 Reviewed-on: https://go-review.googlesource.com/20669 Reviewed-by: Rick Hudson Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot --- src/runtime/proc.go | 3 --- src/runtime/stack.go | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index d5acbee0a7..dcdc7bedb8 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -681,9 +681,6 @@ func casfrom_Gscanstatus(gp *g, oldval, newval uint32) { dumpgstatus(gp) throw("casfrom_Gscanstatus: gp->status is not in scan state") } - if newval == _Grunning { - gp.gcscanvalid = false - } } // This will return false if the gp is not in the expected status and the cas fails. diff --git a/src/runtime/stack.go b/src/runtime/stack.go index dcb1b06dbd..c4b1fb862e 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -1016,6 +1016,7 @@ func newstack() { gp.preemptscan = false gp.preempt = false casfrom_Gscanstatus(gp, _Gscanwaiting, _Gwaiting) + // This clears gcscanvalid. casgstatus(gp, _Gwaiting, _Grunning) gp.stackguard0 = gp.stack.lo + _StackGuard gogo(&gp.sched) // never return -- 2.48.1