diff options
author | Ian Lance Taylor <iant@golang.org> | 2019-11-05 16:05:09 -0800 |
---|---|---|
committer | Ian Lance Taylor <iant@golang.org> | 2019-11-06 15:46:26 +0000 |
commit | b50fcc88e93eb41a64ff80d74aae36c531c5fe60 (patch) | |
tree | 2f1d3499a08b2ef30c5a66d8782cb38b0c109780 /src/runtime/time.go | |
parent | cf3be9bbca2f90d87283ae69322616b43702a2f7 (diff) | |
download | go-b50fcc88e93eb41a64ff80d74aae36c531c5fe60.tar.gz go-b50fcc88e93eb41a64ff80d74aae36c531c5fe60.zip |
runtime: don't hold scheduler lock when calling timeSleepUntil
Otherwise, we can get into a deadlock: sysmon takes the scheduler lock
and calls timeSleepUntil which takes each P's timer lock. Simultaneously,
some P calls runtimer (holding the P's own timer lock) which wakes up
the scavenger, calling goready, calling wakep, calling startm, getting
the scheduler lock. Now the sysmon thread is holding the scheduler lock
and trying to get a P's timer lock, while some other thread running on
that P is holding the P's timer lock and trying to get the scheduler lock.
So change sysmon to call timeSleepUntil without holding the scheduler
lock, and change timeSleepUntil to use allpLock, which is only held for
limited periods of time and should never compete with timer locks.
This hopefully
Fixes #35375
At least it should fix the linux-arm64-packet builder problems,
which occurred more reliably as that system has GOMAXPROCS == 96,
giving a lot more scope for this deadlock.
Change-Id: I7a7917daf7a4882e0b27ca416e4f6300cfaaa774
Reviewed-on: https://go-review.googlesource.com/c/go/+/205558
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Diffstat (limited to 'src/runtime/time.go')
-rw-r--r-- | src/runtime/time.go | 11 |
1 files changed, 11 insertions, 0 deletions
diff --git a/src/runtime/time.go b/src/runtime/time.go index ad5eaf7c48..39df413ad9 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -1234,6 +1234,8 @@ func timejumpLocked() *g { return tb.gp } +// timeSleepUntil returns the time when the next timer should fire. +// This is only called by sysmon. func timeSleepUntil() int64 { if oldTimers { return timeSleepUntilOld() @@ -1241,7 +1243,15 @@ func timeSleepUntil() int64 { next := int64(maxWhen) + // Prevent allp slice changes. This is like retake. + lock(&allpLock) for _, pp := range allp { + if pp == nil { + // This can happen if procresize has grown + // allp but not yet created new Ps. + continue + } + lock(&pp.timersLock) c := atomic.Load(&pp.adjustTimers) for _, t := range pp.timers { @@ -1276,6 +1286,7 @@ func timeSleepUntil() int64 { } unlock(&pp.timersLock) } + unlock(&allpLock) return next } |