From 1b79afe460c329c1db75456c3278600a4b451b41 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 27 Sep 2019 12:31:33 -0400 Subject: [PATCH] runtime: remove old stack scanning code This removes scang and preemptscan, since the stack scanning code now uses suspendG. For #10958, #24543. Change-Id: Ic868bf5d6dcce40662a82cb27bb996cb74d0720e Reviewed-on: https://go-review.googlesource.com/c/go/+/201138 Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/runtime/proc.go | 113 ---------------------------------------- src/runtime/runtime2.go | 1 - src/runtime/stack.go | 29 +---------- 3 files changed, 1 insertion(+), 142 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 3964941b9c..9e40bc8c94 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -802,14 +802,6 @@ func casgstatus(gp *g, oldval, newval uint32) { if oldval == _Gwaiting && gp.atomicstatus == _Grunnable { throw("casgstatus: waiting for Gwaiting but is Grunnable") } - // Help GC if needed. - // if gp.preemptscan && !gp.gcworkdone && (oldval == _Grunning || oldval == _Gsyscall) { - // gp.preemptscan = false - // systemstack(func() { - // gcphasework(gp) - // }) - // } - // But meanwhile just yield. if i == 0 { nextYield = nanotime() + yieldDelay } @@ -867,111 +859,6 @@ func casGFromPreempted(gp *g, old, new uint32) bool { return atomic.Cas(&gp.atomicstatus, _Gpreempted, _Gwaiting) } -// scang blocks until gp's stack has been scanned. -// It might be scanned by scang or it might be scanned by the goroutine itself. -// Either way, the stack scan has completed when scang returns. -func scang(gp *g, gcw *gcWork) { - // Invariant; we (the caller, markroot for a specific goroutine) own gp.gcscandone. - // Nothing is racing with us now, but gcscandone might be set to true left over - // from an earlier round of stack scanning (we scan twice per GC). - // We use gcscandone to record whether the scan has been done during this round. - - gp.gcscandone = false - - // See https://golang.org/cl/21503 for justification of the yield delay. - const yieldDelay = 10 * 1000 - var nextYield int64 - - // Endeavor to get gcscandone set to true, - // either by doing the stack scan ourselves or by coercing gp to scan itself. - // gp.gcscandone can transition from false to true when we're not looking - // (if we asked for preemption), so any time we lock the status using - // castogscanstatus we have to double-check that the scan is still not done. -loop: - for i := 0; !gp.gcscandone; i++ { - switch s := readgstatus(gp); s { - default: - dumpgstatus(gp) - throw("stopg: invalid status") - - case _Gdead: - // No stack. - gp.gcscandone = true - break loop - - case _Gcopystack: - // Stack being switched. Go around again. - - case _Grunnable, _Gsyscall, _Gwaiting: - // Claim goroutine by setting scan bit. - // Racing with execution or readying of gp. - // The scan bit keeps them from running - // the goroutine until we're done. - if castogscanstatus(gp, s, s|_Gscan) { - if !gp.gcscandone { - scanstack(gp, gcw) - gp.gcscandone = true - } - restartg(gp) - break loop - } - - case _Gscanwaiting: - // newstack is doing a scan for us right now. Wait. - - case _Grunning: - // Goroutine running. Try to preempt execution so it can scan itself. - // The preemption handler (in newstack) does the actual scan. - - // Optimization: if there is already a pending preemption request - // (from the previous loop iteration), don't bother with the atomics. - if gp.preemptscan && gp.preempt && gp.stackguard0 == stackPreempt { - break - } - - // Ask for preemption and self scan. - if castogscanstatus(gp, _Grunning, _Gscanrunning) { - if !gp.gcscandone { - gp.preemptscan = true - gp.preempt = true - gp.stackguard0 = stackPreempt - } - casfrom_Gscanstatus(gp, _Gscanrunning, _Grunning) - } - } - - if i == 0 { - nextYield = nanotime() + yieldDelay - } - if nanotime() < nextYield { - procyield(10) - } else { - osyield() - nextYield = nanotime() + yieldDelay/2 - } - } - - gp.preemptscan = false // cancel scan request if no longer needed -} - -// The GC requests that this routine be moved from a scanmumble state to a mumble state. -func restartg(gp *g) { - s := readgstatus(gp) - switch s { - default: - dumpgstatus(gp) - throw("restartg: unexpected status") - - case _Gdead: - // ok - - case _Gscanrunnable, - _Gscanwaiting, - _Gscansyscall: - casfrom_Gscanstatus(gp, s, s&^_Gscan) - } -} - // stopTheWorld stops all P's from executing goroutines, interrupting // all goroutines at GC safe points and records reason as the reason // for the stop. On return, only the current goroutine's P is running. diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 7eac58eb2c..7630888a3d 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -421,7 +421,6 @@ type g struct { preempt bool // preemption signal, duplicates stackguard0 = stackpreempt preemptStop bool // transition to _Gpreempted on preemption; otherwise, just deschedule paniconfault bool // panic (instead of crash) on unexpected fault address - preemptscan bool // preempted g does scan for gc gcscandone bool // g has scanned stack; protected by _Gscan bit in status gcscanvalid bool // false at start of gc cycle, true if G has not run since last scan; TODO: remove? throwsplit bool // must not split stack diff --git a/src/runtime/stack.go b/src/runtime/stack.go index 3b92c89ff0..2c2a88e6e1 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -916,7 +916,7 @@ func round2(x int32) int32 { // Stack growth is multiplicative, for constant amortized cost. // // g->atomicstatus will be Grunning or Gscanrunning upon entry. -// If the GC is trying to stop this g then it will set preemptscan to true. +// If the scheduler is trying to stop this g, then it will set preemptStop. // // This must be nowritebarrierrec because it can be called as part of // stack growth from other nowritebarrierrec functions, but the @@ -1022,34 +1022,7 @@ func newstack() { preemptPark(gp) // never returns } - // Synchronize with scang. - casgstatus(gp, _Grunning, _Gwaiting) - 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, gcworkdone will - // be set and gcphasework will simply - // return. - } - if !gp.gcscandone { - // gcw is safe because we're on the - // system stack. - gcw := &gp.m.p.ptr().gcw - scanstack(gp, gcw) - gp.gcscandone = true - } - 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 - } - // Act like goroutine called runtime.Gosched. - casgstatus(gp, _Gwaiting, _Grunning) gopreempt_m(gp) // never return }