From b3863fbbc2fe1dbf516111992854aa9178d01410 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Wed, 15 Apr 2020 13:56:05 -0400 Subject: [PATCH] runtime: make newproc1 not start the goroutine Currently, newproc1 allocates, initializes, and schedules a new goroutine. We're about to change debug call injection in a way that will need to create a new goroutine without immediately scheduling it. To prepare for that, make scheduling the responsibility of newproc1's caller. Currently, there's exactly one caller (newproc), so this simply shifts that responsibility. For #36365. Change-Id: Idacd06b63e738982e840fe995d891bfd377ce23b Reviewed-on: https://go-review.googlesource.com/c/go/+/229298 Run-TryBot: Austin Clements Reviewed-by: Michael Knyszek Reviewed-by: Cherry Zhang --- src/runtime/proc.go | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 202c300e41..939c68a94d 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -3454,23 +3454,44 @@ func malg(stacksize int32) *g { // Create a new g running fn with siz bytes of arguments. // Put it on the queue of g's waiting to run. // The compiler turns a go statement into a call to this. -// Cannot split the stack because it assumes that the arguments -// are available sequentially after &fn; they would not be -// copied if a stack split occurred. +// +// The stack layout of this call is unusual: it assumes that the +// arguments to pass to fn are on the stack sequentially immediately +// after &fn. Hence, they are logically part of newproc's argument +// frame, even though they don't appear in its signature (and can't +// because their types differ between call sites). +// +// This must be nosplit because this stack layout means there are +// untyped arguments in newproc's argument frame. Stack copies won't +// be able to adjust them and stack splits won't be able to copy them. +// //go:nosplit func newproc(siz int32, fn *funcval) { argp := add(unsafe.Pointer(&fn), sys.PtrSize) gp := getg() pc := getcallerpc() systemstack(func() { - newproc1(fn, argp, siz, gp, pc) + newg := newproc1(fn, argp, siz, gp, pc) + + _p_ := getg().m.p.ptr() + runqput(_p_, newg, true) + + if atomic.Load(&sched.npidle) != 0 && atomic.Load(&sched.nmspinning) == 0 && mainStarted { + wakep() + } }) } -// Create a new g running fn with narg bytes of arguments starting -// at argp. callerpc is the address of the go statement that created -// this. The new g is put on the queue of g's waiting to run. -func newproc1(fn *funcval, argp unsafe.Pointer, narg int32, callergp *g, callerpc uintptr) { +// Create a new g in state _Grunnable, starting at fn, with narg bytes +// of arguments starting at argp. callerpc is the address of the go +// statement that created this. The caller is responsible for adding +// the new g to the scheduler. +// +// This must run on the system stack because it's the continuation of +// newproc, which cannot split the stack. +// +//go:systemstack +func newproc1(fn *funcval, argp unsafe.Pointer, narg int32, callergp *g, callerpc uintptr) *g { _g_ := getg() if fn == nil { @@ -3566,12 +3587,9 @@ func newproc1(fn *funcval, argp unsafe.Pointer, narg int32, callergp *g, callerp if trace.enabled { traceGoCreate(newg, newg.startpc) } - runqput(_p_, newg, true) - - if atomic.Load(&sched.npidle) != 0 && atomic.Load(&sched.nmspinning) == 0 && mainStarted { - wakep() - } releasem(_g_.m) + + return newg } // saveAncestors copies previous ancestors of the given caller g and -- 2.50.0