From df33c170919faca6ea849ee23fb0af6971fddc5b Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 3 Feb 2025 15:22:37 +0000 Subject: [PATCH] runtime: add _Gdeadextra status _Gdeadextra is almost the same as _Gdead but for goroutines attached to extra Ms. The primary difference is that it can be transitioned into a _Gscan status, unlike _Gdead. (Why not just use _Gdead? For safety, mostly. There's exactly one case where we're going to want to transition _Gdead to _Gscan|_Gdead, and it's for extra Ms. It's also a bit weird to call this state dead when it can still have a syscalling P attached to it.) This status is used in a follow-up change that changes entersyscall and exitsyscall. Change-Id: I169a4c8617aa3dc329574b829203f56c86b58169 Reviewed-on: https://go-review.googlesource.com/c/go/+/646197 Reviewed-by: Cherry Mui Reviewed-by: Michael Pratt LUCI-TryBot-Result: Go LUCI --- src/runtime/heapdump.go | 2 +- src/runtime/mgcmark.go | 2 +- src/runtime/mprof.go | 15 ++++++++++++--- src/runtime/preempt.go | 2 +- src/runtime/proc.go | 21 +++++++++++---------- src/runtime/runtime-gdb.py | 8 ++++++-- src/runtime/runtime2.go | 5 +++++ src/runtime/synctest.go | 6 +++--- src/runtime/traceback.go | 12 +++++++++++- src/runtime/tracestatus.go | 2 +- 10 files changed, 52 insertions(+), 23 deletions(-) diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go index d9474034c2..9aaee8f90c 100644 --- a/src/runtime/heapdump.go +++ b/src/runtime/heapdump.go @@ -415,7 +415,7 @@ func dumpgs() { default: print("runtime: unexpected G.status ", hex(status), "\n") throw("dumpgs in STW - bad status") - case _Gdead: + case _Gdead, _Gdeadextra: // ok case _Grunnable, _Gsyscall, diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go index ba3824f00d..dd76973c62 100644 --- a/src/runtime/mgcmark.go +++ b/src/runtime/mgcmark.go @@ -911,7 +911,7 @@ func scanstack(gp *g, gcw *gcWork) int64 { default: print("runtime: gp=", gp, ", goid=", gp.goid, ", gp->atomicstatus=", readgstatus(gp), "\n") throw("mark - bad status") - case _Gdead: + case _Gdead, _Gdeadextra: return 0 case _Grunning: print("runtime: gp=", gp, ", goid=", gp.goid, ", gp->atomicstatus=", readgstatus(gp), "\n") diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go index 794f57cee2..19e90fbb33 100644 --- a/src/runtime/mprof.go +++ b/src/runtime/mprof.go @@ -1454,7 +1454,7 @@ func goroutineProfileWithLabelsConcurrent(p []profilerecord.StackRecord, labels // were collecting the profile. But probably better to return a // truncated profile than to crash the whole process. // - // For instance, needm moves a goroutine out of the _Gdead state and so + // For instance, needm moves a goroutine out of the _Gdeadextra state and so // might be able to change the goroutine count without interacting with // the scheduler. For code like that, the race windows are small and the // combination of features is uncommon, so it's hard to be (and remain) @@ -1480,7 +1480,7 @@ func tryRecordGoroutineProfileWB(gp1 *g) { // in the current goroutine profile: either that it should not be profiled, or // that a snapshot of its call stack and labels are now in the profile. func tryRecordGoroutineProfile(gp1 *g, pcbuf []uintptr, yield func()) { - if readgstatus(gp1) == _Gdead { + if status := readgstatus(gp1); status == _Gdead || status == _Gdeadextra { // Dead goroutines should not appear in the profile. Goroutines that // start while profile collection is active will get goroutineProfiled // set to goroutineProfileSatisfied before transitioning out of _Gdead, @@ -1570,7 +1570,16 @@ func goroutineProfileWithLabelsSync(p []profilerecord.StackRecord, labels []unsa isOK := func(gp1 *g) bool { // Checking isSystemGoroutine here makes GoroutineProfile // consistent with both NumGoroutine and Stack. - return gp1 != gp && readgstatus(gp1) != _Gdead && !isSystemGoroutine(gp1, false) + if gp1 == gp { + return false + } + if status := readgstatus(gp1); status == _Gdead || status == _Gdeadextra { + return false + } + if isSystemGoroutine(gp1, false) { + return false + } + return true } pcbuf := makeProfStack() // see saveg() for explanation diff --git a/src/runtime/preempt.go b/src/runtime/preempt.go index 5367f66213..1044a6d720 100644 --- a/src/runtime/preempt.go +++ b/src/runtime/preempt.go @@ -134,7 +134,7 @@ func suspendG(gp *g) suspendGState { dumpgstatus(gp) throw("invalid g status") - case _Gdead: + case _Gdead, _Gdeadextra: // Nothing to suspend. // // preemptStop may need to be cleared, but diff --git a/src/runtime/proc.go b/src/runtime/proc.go index fadd9a5963..36949fb2cf 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1223,7 +1223,8 @@ func casfrom_Gscanstatus(gp *g, oldval, newval uint32) { _Gscanrunning, _Gscansyscall, _Gscanleaked, - _Gscanpreempted: + _Gscanpreempted, + _Gscandeadextra: if newval == oldval&^_Gscan { success = gp.atomicstatus.CompareAndSwap(oldval, newval) } @@ -1244,7 +1245,8 @@ func castogscanstatus(gp *g, oldval, newval uint32) bool { _Grunning, _Gwaiting, _Gleaked, - _Gsyscall: + _Gsyscall, + _Gdeadextra: if newval == oldval|_Gscan { r := gp.atomicstatus.CompareAndSwap(oldval, newval) if r { @@ -2459,7 +2461,7 @@ func needm(signal bool) { } // mp.curg is now a real goroutine. - casgstatus(mp.curg, _Gdead, _Gsyscall) + casgstatus(mp.curg, _Gdeadextra, _Gsyscall) sched.ngsys.Add(-1) sched.nGsyscallNoP.Add(1) @@ -2515,11 +2517,10 @@ func oneNewExtraM() { gp.syscallpc = gp.sched.pc gp.syscallsp = gp.sched.sp gp.stktopsp = gp.sched.sp - // malg returns status as _Gidle. Change to _Gdead before - // adding to allg where GC can see it. We use _Gdead to hide - // this from tracebacks and stack scans since it isn't a - // "real" goroutine until needm grabs it. - casgstatus(gp, _Gidle, _Gdead) + // malg returns status as _Gidle. Change to _Gdeadextra before + // adding to allg where GC can see it. _Gdeadextra hides this + // from traceback and stack scans. + casgstatus(gp, _Gidle, _Gdeadextra) gp.m = mp mp.curg = gp mp.isextra = true @@ -2593,8 +2594,8 @@ func dropm() { trace = traceAcquire() } - // Return mp.curg to dead state. - casgstatus(mp.curg, _Gsyscall, _Gdead) + // Return mp.curg to _Gdeadextra state. + casgstatus(mp.curg, _Gsyscall, _Gdeadextra) mp.curg.preemptStop = false sched.ngsys.Add(1) sched.nGsyscallNoP.Add(-1) diff --git a/src/runtime/runtime-gdb.py b/src/runtime/runtime-gdb.py index 345a59605e..9d2446a1eb 100644 --- a/src/runtime/runtime-gdb.py +++ b/src/runtime/runtime-gdb.py @@ -46,11 +46,13 @@ G_MORIBUND_UNUSED = read_runtime_const("'runtime._Gmoribund_unused'", 5) G_DEAD = read_runtime_const("'runtime._Gdead'", 6) G_ENQUEUE_UNUSED = read_runtime_const("'runtime._Genqueue_unused'", 7) G_COPYSTACK = read_runtime_const("'runtime._Gcopystack'", 8) +G_DEADEXTRA = read_runtime_const("'runtime._Gdeadextra'", 10) G_SCAN = read_runtime_const("'runtime._Gscan'", 0x1000) G_SCANRUNNABLE = G_SCAN+G_RUNNABLE G_SCANRUNNING = G_SCAN+G_RUNNING G_SCANSYSCALL = G_SCAN+G_SYSCALL G_SCANWAITING = G_SCAN+G_WAITING +G_SCANEXTRA = G_SCAN+G_DEADEXTRA sts = { G_IDLE: 'idle', @@ -62,11 +64,13 @@ sts = { G_DEAD: 'dead', G_ENQUEUE_UNUSED: 'enqueue', G_COPYSTACK: 'copystack', + G_DEADEXTRA: 'extra', G_SCAN: 'scan', G_SCANRUNNABLE: 'runnable+s', G_SCANRUNNING: 'running+s', G_SCANSYSCALL: 'syscall+s', G_SCANWAITING: 'waiting+s', + G_SCANEXTRA: 'extra+s', } @@ -524,7 +528,7 @@ class GoroutinesCmd(gdb.Command): # args = gdb.string_to_argv(arg) vp = gdb.lookup_type('void').pointer() for ptr in SliceValue(gdb.parse_and_eval("'runtime.allgs'")): - if ptr['atomicstatus']['value'] == G_DEAD: + if ptr['atomicstatus']['value'] in [G_DEAD, G_DEADEXTRA]: continue s = ' ' if ptr['m']: @@ -549,7 +553,7 @@ def find_goroutine(goid): """ vp = gdb.lookup_type('void').pointer() for ptr in SliceValue(gdb.parse_and_eval("'runtime.allgs'")): - if ptr['atomicstatus']['value'] == G_DEAD: + if ptr['atomicstatus']['value'] in [G_DEAD, G_DEADEXTRA]: continue if ptr['goid'] == goid: break diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 6016c6fde0..eaf24fe908 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -90,6 +90,10 @@ const ( // _Gleaked represents a leaked goroutine caught by the GC. _Gleaked // 10 + // _Gdeadextra is a _Gdead goroutine that's attached to an extra M + // used for cgo callbacks. + _Gdeadextra // 11 + // _Gscan combined with one of the above states other than // _Grunning indicates that GC is scanning the stack. The // goroutine is not executing user code and the stack is owned @@ -108,6 +112,7 @@ const ( _Gscanwaiting = _Gscan + _Gwaiting // 0x1004 _Gscanpreempted = _Gscan + _Gpreempted // 0x1009 _Gscanleaked = _Gscan + _Gleaked // 0x100a + _Gscandeadextra = _Gscan + _Gdeadextra // 0x100b ) const ( diff --git a/src/runtime/synctest.go b/src/runtime/synctest.go index 529f69fd93..8fdf437acb 100644 --- a/src/runtime/synctest.go +++ b/src/runtime/synctest.go @@ -52,7 +52,7 @@ func (bubble *synctestBubble) changegstatus(gp *g, oldval, newval uint32) { totalDelta := 0 wasRunning := true switch oldval { - case _Gdead: + case _Gdead, _Gdeadextra: wasRunning = false totalDelta++ case _Gwaiting: @@ -62,7 +62,7 @@ func (bubble *synctestBubble) changegstatus(gp *g, oldval, newval uint32) { } isRunning := true switch newval { - case _Gdead: + case _Gdead, _Gdeadextra: isRunning = false totalDelta-- if gp == bubble.main { @@ -86,7 +86,7 @@ func (bubble *synctestBubble) changegstatus(gp *g, oldval, newval uint32) { bubble.running++ } else { bubble.running-- - if raceenabled && newval != _Gdead { + if raceenabled && newval != _Gdead && newval != _Gdeadextra { // Record that this goroutine parking happens before // any subsequent Wait. racereleasemergeg(gp, bubble.raceaddr()) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 8882c306ed..de62762ba7 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -1208,6 +1208,7 @@ var gStatusStrings = [...]string{ _Gcopystack: "copystack", _Gleaked: "leaked", _Gpreempted: "preempted", + _Gdeadextra: "waiting for cgo callback", } func goroutineheader(gp *g) { @@ -1295,7 +1296,16 @@ func tracebacksomeothers(me *g, showf func(*g) bool) { // against concurrent creation of new Gs, but even with allglock we may // miss Gs created after this loop. forEachGRace(func(gp *g) { - if gp == me || gp == curgp || readgstatus(gp) == _Gdead || !showf(gp) || (isSystemGoroutine(gp, false) && level < 2) { + if gp == me || gp == curgp { + return + } + if status := readgstatus(gp); status == _Gdead || status == _Gdeadextra { + return + } + if !showf(gp) { + return + } + if isSystemGoroutine(gp, false) && level < 2 { return } print("\n") diff --git a/src/runtime/tracestatus.go b/src/runtime/tracestatus.go index 8b5eafd170..f09bdfd355 100644 --- a/src/runtime/tracestatus.go +++ b/src/runtime/tracestatus.go @@ -134,7 +134,7 @@ func goStatusToTraceGoStatus(status uint32, wr waitReason) tracev2.GoStatus { if status == _Gwaiting && wr.isWaitingForSuspendG() { tgs = tracev2.GoRunning } - case _Gdead: + case _Gdead, _Gdeadextra: throw("tried to trace dead goroutine") default: throw("tried to trace goroutine with invalid or unsupported status") -- 2.52.0