runtime, time: disable preemption in addtimer

The timerpMask optimization updates a mask of Ps (potentially)
containing timers in pidleget / pidleput. For correctness, it depends on
the assumption that new timers can only be added to a P's own heap.

addtimer violates this assumption if it is preempted after computing pp.
That G may then run on a different P, but adding a timer to the original
P's heap.

Avoid this by disabling preemption while pp is in use.

Other uses of doaddtimer should be OK:

* moveTimers: always moves to the current P's heap
* modtimer, cleantimers, addAdjustedTimers, runtimer: does not add net
  new timers to the heap while locked

Fixes #44868

Change-Id: I4a5d080865e854931d0a3a09a51ca36879101d72
Reviewed-on: https://go-review.googlesource.com/c/go/+/300610
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
This commit is contained in:
Michael Pratt 2021-03-10 16:06:47 -05:00
parent f9ed8b3f1e
commit aa26687e45
2 changed files with 21 additions and 0 deletions

View file

@ -263,6 +263,9 @@ func addtimer(t *timer) {
when := t.when when := t.when
// Disable preemption while using pp to avoid changing another P's heap.
mp := acquirem()
pp := getg().m.p.ptr() pp := getg().m.p.ptr()
lock(&pp.timersLock) lock(&pp.timersLock)
cleantimers(pp) cleantimers(pp)
@ -270,6 +273,8 @@ func addtimer(t *timer) {
unlock(&pp.timersLock) unlock(&pp.timersLock)
wakeNetPoller(when) wakeNetPoller(when)
releasem(mp)
} }
// doaddtimer adds t to the current P's heap. // doaddtimer adds t to the current P's heap.

View file

@ -511,6 +511,22 @@ func TestZeroTimerStopPanics(t *testing.T) {
tr.Stop() tr.Stop()
} }
// Test that zero duration timers aren't missed by the scheduler. Regression test for issue 44868.
func TestZeroTimer(t *testing.T) {
if testing.Short() {
t.Skip("-short")
}
for i := 0; i < 1000000; i++ {
s := Now()
ti := NewTimer(0)
<-ti.C
if diff := Since(s); diff > 2*Second {
t.Errorf("Expected time to get value from Timer channel in less than 2 sec, took %v", diff)
}
}
}
// Benchmark timer latency when the thread that creates the timer is busy with // Benchmark timer latency when the thread that creates the timer is busy with
// other work and the timers must be serviced by other threads. // other work and the timers must be serviced by other threads.
// https://golang.org/issue/38860 // https://golang.org/issue/38860