runtime: don't do a direct G handoff in semrelease on systemstack

semrelease is safe to call on the system stack (since it just readies
goroutines) except for the fact that it might perform a direct G
handoff and call into the scheduler. If handoff is set to false this is
exceptionally rare, but could happen, and has happened for the trace
reader goroutine which releases a trace.doneSema.

Fixes #73469.

Change-Id: I37ece678bc4721bbb6e5879d74daac762b7d742a
Reviewed-on: https://go-review.googlesource.com/c/go/+/680315
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>

diff --git a/src/runtime/sema.go b/src/runtime/sema.go
index 0f029f604fd537a15146eb71fdad412e11baf7d6..6af49b1b0c42d9dbe21e60e0fb6e07e702423f3d 100644 (file)
--- a/src/runtime/sema.go
+++ b/src/runtime/sema.go
@@ -261,11 +261,13 @@ func semrelease1(addr *uint32, handoff bool, skipframes int) {
                        s.ticket = 1
                }
                readyWithTime(s, 5+skipframes)
-               if s.ticket == 1 && getg().m.locks == 0 {
+               if s.ticket == 1 && getg().m.locks == 0 && getg() != getg().m.g0 {
                        // Direct G handoff
+                       //
                        // readyWithTime has added the waiter G as runnext in the
                        // current P; we now call the scheduler so that we start running
                        // the waiter G immediately.
+                       //
                        // Note that waiter inherits our time slice: this is desirable
                        // to avoid having a highly contended semaphore hog the P
                        // indefinitely. goyield is like Gosched, but it emits a
@@ -275,9 +277,12 @@ func semrelease1(addr *uint32, handoff bool, skipframes int) {
                        // the non-starving case it is possible for a different waiter
                        // to acquire the semaphore while we are yielding/scheduling,
                        // and this would be wasteful. We wait instead to enter starving
-                       // regime, and then we start to do direct handoffs of ticket and
-                       // P.
+                       // regime, and then we start to do direct handoffs of ticket and P.
+                       //
                        // See issue 33747 for discussion.
+                       //
+                       // We don't handoff directly if we're holding locks or on the
+                       // system stack, since it's not safe to enter the scheduler.
                        goyield()
                }
        }