]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: make newproc1 not start the goroutine
authorAustin Clements <austin@google.com>
Wed, 15 Apr 2020 17:56:05 +0000 (13:56 -0400)
committerAustin Clements <austin@google.com>
Wed, 29 Apr 2020 21:29:08 +0000 (21:29 +0000)
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 <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
src/runtime/proc.go

index 202c300e41c8cf6c93b064fd465c166677146e88..939c68a94d8d234a7b292e5f09f5d306c318bc8a 100644 (file)
@@ -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