From f78efc0178d51c02beff8a8203910dc0a9c6e953 Mon Sep 17 00:00:00 2001 From: cui fliter Date: Thu, 8 Sep 2022 11:29:09 +0000 Subject: [PATCH] runtime: convert timer.status to atomic type For #53821 Change-Id: I7cb6a16626964d5023b96609d9921bfe4a679d8f GitHub-Last-Rev: ddfce125be4aa565aa4d2081841c649bf2f71459 GitHub-Pull-Request: golang/go#54865 Reviewed-on: https://go-review.googlesource.com/c/go/+/428196 Reviewed-by: Keith Randall Reviewed-by: Jenny Rakoczy Auto-Submit: Jenny Rakoczy Reviewed-by: Keith Randall Run-TryBot: Michael Pratt TryBot-Result: Gopher Robot --- src/runtime/time.go | 87 +++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 43 deletions(-) diff --git a/src/runtime/time.go b/src/runtime/time.go index 945756109a..6cd70b7aed 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -36,7 +36,7 @@ type timer struct { nextwhen int64 // The status field holds one of the values below. - status uint32 + status atomic.Uint32 } // Code outside this file has to be careful in using a timer value. @@ -249,6 +249,7 @@ func goroutineReady(arg any, seq uintptr) { goready(arg.(*g), 0) } +// Note: this changes some unsynchronized operations to synchronized operations // addtimer adds a timer to the current P. // This should only be called with a newly created timer. // That avoids the risk of changing the when field of a timer in some P's heap, @@ -263,10 +264,10 @@ func addtimer(t *timer) { if t.period < 0 { throw("timer period must be non-negative") } - if t.status != timerNoStatus { + if t.status.Load() != timerNoStatus { throw("addtimer called with initialized timer") } - t.status = timerWaiting + t.status.Store(timerWaiting) when := t.when @@ -312,17 +313,17 @@ func doaddtimer(pp *p, t *timer) { // Reports whether the timer was removed before it was run. func deltimer(t *timer) bool { for { - switch s := atomic.Load(&t.status); s { + switch s := t.status.Load(); s { case timerWaiting, timerModifiedLater: // Prevent preemption while the timer is in timerModifying. // This could lead to a self-deadlock. See #38070. mp := acquirem() - if atomic.Cas(&t.status, s, timerModifying) { + if t.status.CompareAndSwap(s, timerModifying) { // Must fetch t.pp before changing status, // as cleantimers in another goroutine // can clear t.pp of a timerDeleted timer. tpp := t.pp.ptr() - if !atomic.Cas(&t.status, timerModifying, timerDeleted) { + if !t.status.CompareAndSwap(timerModifying, timerDeleted) { badTimer() } releasem(mp) @@ -336,11 +337,11 @@ func deltimer(t *timer) bool { // Prevent preemption while the timer is in timerModifying. // This could lead to a self-deadlock. See #38070. mp := acquirem() - if atomic.Cas(&t.status, s, timerModifying) { + if t.status.CompareAndSwap(s, timerModifying) { // Must fetch t.pp before setting status // to timerDeleted. tpp := t.pp.ptr() - if !atomic.Cas(&t.status, timerModifying, timerDeleted) { + if !t.status.CompareAndSwap(timerModifying, timerDeleted) { badTimer() } releasem(mp) @@ -449,12 +450,12 @@ func modtimer(t *timer, when, period int64, f func(any, uintptr), arg any, seq u var mp *m loop: for { - switch status = atomic.Load(&t.status); status { + switch status = t.status.Load(); status { case timerWaiting, timerModifiedEarlier, timerModifiedLater: // Prevent preemption while the timer is in timerModifying. // This could lead to a self-deadlock. See #38070. mp = acquirem() - if atomic.Cas(&t.status, status, timerModifying) { + if t.status.CompareAndSwap(status, timerModifying) { pending = true // timer not yet run break loop } @@ -466,7 +467,7 @@ loop: // Timer was already run and t is no longer in a heap. // Act like addtimer. - if atomic.Cas(&t.status, status, timerModifying) { + if t.status.CompareAndSwap(status, timerModifying) { wasRemoved = true pending = false // timer already run or stopped break loop @@ -476,7 +477,7 @@ loop: // Prevent preemption while the timer is in timerModifying. // This could lead to a self-deadlock. See #38070. mp = acquirem() - if atomic.Cas(&t.status, status, timerModifying) { + if t.status.CompareAndSwap(status, timerModifying) { t.pp.ptr().deletedTimers.Add(-1) pending = false // timer already stopped break loop @@ -506,7 +507,7 @@ loop: lock(&pp.timersLock) doaddtimer(pp, t) unlock(&pp.timersLock) - if !atomic.Cas(&t.status, timerModifying, timerWaiting) { + if !t.status.CompareAndSwap(timerModifying, timerWaiting) { badTimer() } releasem(mp) @@ -531,7 +532,7 @@ loop: } // Set the new status of the timer. - if !atomic.Cas(&t.status, timerModifying, newStatus) { + if !t.status.CompareAndSwap(timerModifying, newStatus) { badTimer() } releasem(mp) @@ -577,18 +578,18 @@ func cleantimers(pp *p) { if t.pp.ptr() != pp { throw("cleantimers: bad p") } - switch s := atomic.Load(&t.status); s { + switch s := t.status.Load(); s { case timerDeleted: - if !atomic.Cas(&t.status, s, timerRemoving) { + if !t.status.CompareAndSwap(s, timerRemoving) { continue } dodeltimer0(pp) - if !atomic.Cas(&t.status, timerRemoving, timerRemoved) { + if !t.status.CompareAndSwap(timerRemoving, timerRemoved) { badTimer() } pp.deletedTimers.Add(-1) case timerModifiedEarlier, timerModifiedLater: - if !atomic.Cas(&t.status, s, timerMoving) { + if !t.status.CompareAndSwap(s, timerMoving) { continue } // Now we can change the when field. @@ -596,7 +597,7 @@ func cleantimers(pp *p) { // Move t to the right position. dodeltimer0(pp) doaddtimer(pp, t) - if !atomic.Cas(&t.status, timerMoving, timerWaiting) { + if !t.status.CompareAndSwap(timerMoving, timerWaiting) { badTimer() } default: @@ -614,30 +615,30 @@ func moveTimers(pp *p, timers []*timer) { for _, t := range timers { loop: for { - switch s := atomic.Load(&t.status); s { + switch s := t.status.Load(); s { case timerWaiting: - if !atomic.Cas(&t.status, s, timerMoving) { + if !t.status.CompareAndSwap(s, timerMoving) { continue } t.pp = 0 doaddtimer(pp, t) - if !atomic.Cas(&t.status, timerMoving, timerWaiting) { + if !t.status.CompareAndSwap(timerMoving, timerWaiting) { badTimer() } break loop case timerModifiedEarlier, timerModifiedLater: - if !atomic.Cas(&t.status, s, timerMoving) { + if !t.status.CompareAndSwap(s, timerMoving) { continue } t.when = t.nextwhen t.pp = 0 doaddtimer(pp, t) - if !atomic.Cas(&t.status, timerMoving, timerWaiting) { + if !t.status.CompareAndSwap(timerMoving, timerWaiting) { badTimer() } break loop case timerDeleted: - if !atomic.Cas(&t.status, s, timerRemoved) { + if !t.status.CompareAndSwap(s, timerRemoved) { continue } t.pp = 0 @@ -688,11 +689,11 @@ func adjusttimers(pp *p, now int64) { if t.pp.ptr() != pp { throw("adjusttimers: bad p") } - switch s := atomic.Load(&t.status); s { + switch s := t.status.Load(); s { case timerDeleted: - if atomic.Cas(&t.status, s, timerRemoving) { + if t.status.CompareAndSwap(s, timerRemoving) { changed := dodeltimer(pp, i) - if !atomic.Cas(&t.status, timerRemoving, timerRemoved) { + if !t.status.CompareAndSwap(timerRemoving, timerRemoved) { badTimer() } pp.deletedTimers.Add(-1) @@ -701,7 +702,7 @@ func adjusttimers(pp *p, now int64) { i = changed - 1 } case timerModifiedEarlier, timerModifiedLater: - if atomic.Cas(&t.status, s, timerMoving) { + if t.status.CompareAndSwap(s, timerMoving) { // Now we can change the when field. t.when = t.nextwhen // Take t off the heap, and hold onto it. @@ -741,7 +742,7 @@ func adjusttimers(pp *p, now int64) { func addAdjustedTimers(pp *p, moved []*timer) { for _, t := range moved { doaddtimer(pp, t) - if !atomic.Cas(&t.status, timerMoving, timerWaiting) { + if !t.status.CompareAndSwap(timerMoving, timerWaiting) { badTimer() } } @@ -776,14 +777,14 @@ func runtimer(pp *p, now int64) int64 { if t.pp.ptr() != pp { throw("runtimer: bad p") } - switch s := atomic.Load(&t.status); s { + switch s := t.status.Load(); s { case timerWaiting: if t.when > now { // Not ready to run. return t.when } - if !atomic.Cas(&t.status, s, timerRunning) { + if !t.status.CompareAndSwap(s, timerRunning) { continue } // Note that runOneTimer may temporarily unlock @@ -792,11 +793,11 @@ func runtimer(pp *p, now int64) int64 { return 0 case timerDeleted: - if !atomic.Cas(&t.status, s, timerRemoving) { + if !t.status.CompareAndSwap(s, timerRemoving) { continue } dodeltimer0(pp) - if !atomic.Cas(&t.status, timerRemoving, timerRemoved) { + if !t.status.CompareAndSwap(timerRemoving, timerRemoved) { badTimer() } pp.deletedTimers.Add(-1) @@ -805,13 +806,13 @@ func runtimer(pp *p, now int64) int64 { } case timerModifiedEarlier, timerModifiedLater: - if !atomic.Cas(&t.status, s, timerMoving) { + if !t.status.CompareAndSwap(s, timerMoving) { continue } t.when = t.nextwhen dodeltimer0(pp) doaddtimer(pp, t) - if !atomic.Cas(&t.status, timerMoving, timerWaiting) { + if !t.status.CompareAndSwap(timerMoving, timerWaiting) { badTimer() } @@ -858,14 +859,14 @@ func runOneTimer(pp *p, t *timer, now int64) { t.when = maxWhen } siftdownTimer(pp.timers, 0) - if !atomic.Cas(&t.status, timerRunning, timerWaiting) { + if !t.status.CompareAndSwap(timerRunning, timerWaiting) { badTimer() } updateTimer0When(pp) } else { // Remove from heap. dodeltimer0(pp) - if !atomic.Cas(&t.status, timerRunning, timerNoStatus) { + if !t.status.CompareAndSwap(timerRunning, timerNoStatus) { badTimer() } } @@ -912,7 +913,7 @@ func clearDeletedTimers(pp *p) { nextTimer: for _, t := range timers { for { - switch s := atomic.Load(&t.status); s { + switch s := t.status.Load(); s { case timerWaiting: if changedHeap { timers[to] = t @@ -921,22 +922,22 @@ nextTimer: to++ continue nextTimer case timerModifiedEarlier, timerModifiedLater: - if atomic.Cas(&t.status, s, timerMoving) { + if t.status.CompareAndSwap(s, timerMoving) { t.when = t.nextwhen timers[to] = t siftupTimer(timers, to) to++ changedHeap = true - if !atomic.Cas(&t.status, timerMoving, timerWaiting) { + if !t.status.CompareAndSwap(timerMoving, timerWaiting) { badTimer() } continue nextTimer } case timerDeleted: - if atomic.Cas(&t.status, s, timerRemoving) { + if t.status.CompareAndSwap(s, timerRemoving) { t.pp = 0 cdel++ - if !atomic.Cas(&t.status, timerRemoving, timerRemoved) { + if !t.status.CompareAndSwap(timerRemoving, timerRemoved) { badTimer() } changedHeap = true