runtime: abstract M preemption check into a function

We check whether an M is preemptible in a surprising number of places.
Put it in one function.

For #10958, #24543.

Change-Id: I305090fdb1ea7f7a55ffe25851c1e35012d0d06c
Reviewed-on: https://go-review.googlesource.com/c/go/+/201439
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/runtime/mgcwork.go b/src/runtime/mgcwork.go
index f2c16d7d8c4dfcceee90d5b59fc1b9c6f075fb4d..927b06c3f9544c9423d1f80822f09406625fd6f3 100644 (file)
--- a/src/runtime/mgcwork.go
+++ b/src/runtime/mgcwork.go
@@ -126,12 +126,12 @@ func (w *gcWork) checkPut(ptr uintptr, ptrs []uintptr) {
        if debugCachedWork {
                alreadyFailed := w.putGen == w.pauseGen
                w.putGen = w.pauseGen
-               if m := getg().m; m.locks > 0 || m.mallocing != 0 || m.preemptoff != "" || m.p.ptr().status != _Prunning {
+               if !canPreemptM(getg().m) {
                        // If we were to spin, the runtime may
-                       // deadlock: the condition above prevents
-                       // preemption (see newstack), which could
-                       // prevent gcMarkDone from finishing the
-                       // ragged barrier and releasing the spin.
+                       // deadlock. Since we can't be preempted, the
+                       // spin could prevent gcMarkDone from
+                       // finishing the ragged barrier, which is what
+                       // releases us from the spin.
                        return
                }
                for atomic.Load(&gcWorkPauseGen) == w.pauseGen {

diff --git a/src/runtime/preempt.go b/src/runtime/preempt.go
index 0565fd636079c508ee435a0fe79162abe76816a2..96eaa3488b80f512a0cc675af3e40ffe7fd8e89a 100644 (file)
--- a/src/runtime/preempt.go
+++ b/src/runtime/preempt.go
@@ -223,3 +223,12 @@ func resumeG(state suspendGState) {
                ready(gp, 0, true)
        }
 }
+
+// canPreemptM reports whether mp is in a state that is safe to preempt.
+//
+// It is nosplit because it has nosplit callers.
+//
+//go:nosplit
+func canPreemptM(mp *m) bool {
+       return mp.locks == 0 && mp.mallocing == 0 && mp.preemptoff == "" && mp.p.ptr().status == _Prunning
+}

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 9a553a5f8803b8803fc9aba401b78c50e7e89829..60a15c1e9cace36d9dccd12156a7fd2cc7107055 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -2703,7 +2703,7 @@ func gosched_m(gp *g) {
 // goschedguarded is a forbidden-states-avoided version of gosched_m
 func goschedguarded_m(gp *g) {
 
-       if gp.m.locks != 0 || gp.m.mallocing != 0 || gp.m.preemptoff != "" || gp.m.p.ptr().status != _Prunning {
+       if !canPreemptM(gp.m) {
                gogo(&gp.sched) // never return
        }
 

diff --git a/src/runtime/stack.go b/src/runtime/stack.go
index 825826cacd098f15974805e25ed0b57aec0dd7d4..ecefce1e32e9cec40eeb1fda6e2683d84e74bdba 100644 (file)
--- a/src/runtime/stack.go
+++ b/src/runtime/stack.go
@@ -975,7 +975,7 @@ func newstack() {
        // it needs a lock held by the goroutine), that small preemption turns
        // into a real deadlock.
        if preempt {
-               if thisg.m.locks != 0 || thisg.m.mallocing != 0 || thisg.m.preemptoff != "" || thisg.m.p.ptr().status != _Prunning {
+               if !canPreemptM(thisg.m) {
                        // Let the goroutine keep running for now.
                        // gp->preempt is set, so it will be preempted next time.
                        gp.stackguard0 = gp.stack.lo + _StackGuard