diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index b2a98d5fa0..dee5ea4c60 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -28,6 +28,7 @@ // Task.mu // FSContext.mu // runningTasksMu +// Timekeeper.updateMu // // Locking SignalHandlers.mu in multiple SignalHandlers requires locking // TaskSet.mu exclusively first. Locking Task.mu in multiple Tasks at the same @@ -1607,6 +1608,11 @@ func (k *Kernel) incRunningTasks() { close(k.taskActivityCh) k.taskActivityCh = make(chan struct{}) + // take a timekeeper reference that says alive until we transition + // back from 1 to 0 tasks so vDSO stays updated; this also updates + // the vDSO before returning if it was parked + k.timekeeper.addRef() + // This store must happen after the increment of k.cpuClock above to ensure // that concurrent calls to Task.accountTaskGoroutineLeave() also observe // the updated k.cpuClock. @@ -1621,6 +1627,9 @@ func (k *Kernel) decRunningTasks() { if tasks < 0 { panic(fmt.Sprintf("Invalid running count %d", tasks)) } + if tasks == 0 { + k.timekeeper.release() + } // Nothing to do. The next CPU clock tick will disable the timer if // there is still nothing running. This provides approximately one tick diff --git a/pkg/sentry/kernel/timekeeper.go b/pkg/sentry/kernel/timekeeper.go index 75b2986d1d..fe27e2248d 100644 --- a/pkg/sentry/kernel/timekeeper.go +++ b/pkg/sentry/kernel/timekeeper.go @@ -26,6 +26,21 @@ import ( "gvisor.dev/gvisor/pkg/tcpip" ) +// Possible values of Timekeeper.updaterState +const ( + // manually stopped + updaterStopped int32 = iota + + // timer not armed due to being idle for a whole interval + updaterParked + + // set when timer triggers if no tasks are running; any task running + // or internal timekeeper usage transitions back to updaterActive + updaterIdle + + updaterActive +) + // Timekeeper manages all of the kernel clocks. // // +stateify savable @@ -86,6 +101,26 @@ type Timekeeper struct { // wg is used to indicate that the update goroutine has exited. wg sync.WaitGroup `state:"nosave"` + + // atomic enum with updater* values + // + // the goroutine moves it active->idle->parked (under updateMu) + // GetTime or 0->1 tasks moves to active if not stopped (lockless) + // startUpdater/stopUpdater move in/out of stopped (under updateMu) + updaterState atomicbitops.Int32 `state:"nosave"` + + // (runningTasks > 0 ? 1 : 0) + number_of_tasks_running_GetTime + refs atomicbitops.Int64 `state:"nosave"` + + // guards updates of the vDSO parameter page and of updaterState + // except for raising updaterState to active on usage + updateMu sync.Mutex `state:"nosave"` + + // timer to periodically update the time calibration + timer *time.Timer `state:"nosave"` + + // vDSO parameter page kept updated by the Timekeeper mechanism + params *VDSOParamPage `state:"nosave"` } // NewTimekeeper returns a Timekeeper that is automatically kept up-to-date. @@ -170,6 +205,35 @@ func (t *Timekeeper) SetClocks(c sentrytime.Clocks, params *VDSOParamPage) { } } +// update samples the backing clocks and writes the new parameters to the VDSO +// parameter page. +// +// Preconditions: updateMu must be held +func (t *Timekeeper) update(parked bool) { + // Call Update within a Write block to prevent the VDSO from using the old + // params between Update and Write. + if err := t.params.Write(func() vdsoParams { + monotonicParams, monotonicOk, realtimeParams, realtimeOk := t.clocks.Update(parked) + + var p vdsoParams + if monotonicOk { + p.monotonicReady = 1 + p.monotonicBaseCycles = int64(monotonicParams.BaseCycles) + p.monotonicBaseRef = int64(monotonicParams.BaseRef) + t.monotonicOffset + p.monotonicFrequency = monotonicParams.Frequency + } + if realtimeOk { + p.realtimeReady = 1 + p.realtimeBaseCycles = int64(realtimeParams.BaseCycles) + p.realtimeBaseRef = int64(realtimeParams.BaseRef) + p.realtimeFrequency = realtimeParams.Frequency + } + return p + }); err != nil { + log.Warningf("Unable to update VDSO parameter page: %v", err) + } +} + // startUpdater starts an update goroutine that keeps the clocks updated. // // mu must be held. @@ -179,6 +243,7 @@ func (t *Timekeeper) startUpdater(params *VDSOParamPage) { return } t.stop = make(chan struct{}) + t.params = params // Keep the clocks up to date. // @@ -186,47 +251,97 @@ func (t *Timekeeper) startUpdater(params *VDSOParamPage) { // timer, so it may run at a *slightly* different rate from the // application CLOCK_MONOTONIC. That is fine, as we only need to update // at approximately this rate. - timer := time.NewTicker(sentrytime.ApproxUpdateInterval) + t.timer = time.NewTimer(sentrytime.ApproxUpdateInterval) + t.timer.Stop() + + t.updateMu.Lock() + // store-then-load to synchronize with addRef + t.updaterState.Store(updaterParked) + if t.refs.Load() != 0 { + t.timer.Reset(sentrytime.ApproxUpdateInterval) + t.update(true) + t.updaterState.Store(updaterActive) + } + t.updateMu.Unlock() + t.wg.Add(1) go func() { // S/R-SAFE: stopped during save. defer t.wg.Done() + defer t.timer.Stop() for { - // Start with an update immediately, so the clocks are - // ready ASAP. - - // Call Update within a Write block to prevent the VDSO - // from using the old params between Update and - // Write. - if err := params.Write(func() vdsoParams { - monotonicParams, monotonicOk, realtimeParams, realtimeOk := t.clocks.Update() - - var p vdsoParams - if monotonicOk { - p.monotonicReady = 1 - p.monotonicBaseCycles = int64(monotonicParams.BaseCycles) - p.monotonicBaseRef = int64(monotonicParams.BaseRef) + t.monotonicOffset - p.monotonicFrequency = monotonicParams.Frequency - } - if realtimeOk { - p.realtimeReady = 1 - p.realtimeBaseCycles = int64(realtimeParams.BaseCycles) - p.realtimeBaseRef = int64(realtimeParams.BaseRef) - p.realtimeFrequency = realtimeParams.Frequency - } - return p - }); err != nil { - log.Warningf("Unable to update VDSO parameter page: %v", err) - } - + // wait until next tick or if parked until addRef is called select { - case <-timer.C: + case <-t.timer.C: case <-t.stop: return } + + t.updateMu.Lock() + + switch t.updaterState.Load() { + case updaterStopped: + t.updateMu.Unlock() + return + + case updaterActive: + // store-then-load to synchronize with addRef + t.updaterState.Store(updaterIdle) + if t.refs.Load() != 0 { + t.updaterState.Store(updaterActive) + } + + case updaterIdle: + // no Timekeeper usage for a whole interval: park + // store-then-load to synchronize with addRef + t.updaterState.Store(updaterParked) + if t.refs.Load() != 0 { + t.updaterState.Store(updaterActive) + } else { + t.updateMu.Unlock() + continue + } + } + + t.timer.Reset(sentrytime.ApproxUpdateInterval) + t.update(false) + t.updateMu.Unlock() } }() } +// add a reference to the Timekeeper: as long as there are references, +// it doesn't park. Call release() to release. +func (t *Timekeeper) addRef() { + // store-then-load to synchronize with the goroutine + t.refs.Add(1) + switch t.updaterState.Load() { + case updaterActive: + case updaterIdle: + // this is enough because the goroutine will + // itself recheck refs after parking + t.updaterState.CompareAndSwap(updaterIdle, updaterActive) + case updaterParked: + t.updateMu.Lock() + cur := t.updaterState.Load() + if cur == updaterParked { + t.timer.Reset(sentrytime.ApproxUpdateInterval) + t.update(true) + } + if cur != updaterStopped { + t.updaterState.Store(updaterActive) + } + t.updateMu.Unlock() + case updaterStopped: + } +} + +// release drops a reference taken by addRef; when refs are 0 the timekeeper can park +func (t *Timekeeper) release() { + if t.refs.Add(-1) < 0 { + panic("Timekeeper.release called with no reference held") + } +} + // stopUpdater stops the update goroutine, blocking until it exits. // // mu must be held. @@ -236,9 +351,17 @@ func (t *Timekeeper) stopUpdater() { return } + // lock to ensure it can't be taken out of stopped by non-startUpdater code + t.updateMu.Lock() + t.updaterState.Store(updaterStopped) + t.updateMu.Unlock() + + // this wakes up the goroutine close(t.stop) t.wg.Wait() t.stop = nil + t.timer = nil + t.params = nil } // Destroy destroys the Timekeeper, freeing all associated resources. @@ -250,7 +373,8 @@ func (t *Timekeeper) Destroy() { } // PauseUpdates stops clock parameter updates. This should only be used when -// Tasks are not running and thus cannot access the clock. +// Tasks are not running and thus cannot access the clock and when +// GetTime is not being called internally func (t *Timekeeper) PauseUpdates() { t.mu.Lock() defer t.mu.Unlock() @@ -272,6 +396,12 @@ func (t *Timekeeper) GetTime(c sentrytime.ClockID) (int64, error) { } <-t.restored } + + // update the calibration if needed and keep the timekeeper calibrated + // during the read + t.addRef() + defer t.release() + now, err := t.clocks.GetTime(c) if err == nil && c == sentrytime.Monotonic { now += t.monotonicOffset diff --git a/pkg/sentry/kernel/timekeeper_test.go b/pkg/sentry/kernel/timekeeper_test.go index e27da18eb8..2014067e2a 100644 --- a/pkg/sentry/kernel/timekeeper_test.go +++ b/pkg/sentry/kernel/timekeeper_test.go @@ -33,11 +33,11 @@ type mockClocks struct { } // Update implements sentrytime.Clocks.Update. It does nothing. -func (*mockClocks) Update() (monotonicParams sentrytime.Parameters, monotonicOk bool, realtimeParam sentrytime.Parameters, realtimeOk bool) { +func (*mockClocks) Update(parked bool) (monotonicParams sentrytime.Parameters, monotonicOk bool, realtimeParam sentrytime.Parameters, realtimeOk bool) { return } -// Update implements sentrytime.Clocks.GetTime. +// GetTime implements sentrytime.Clocks.GetTime. func (c *mockClocks) GetTime(id sentrytime.ClockID) (int64, error) { switch id { case sentrytime.Monotonic: diff --git a/pkg/sentry/time/calibrated_clock.go b/pkg/sentry/time/calibrated_clock.go index 47df5d0a19..a73e1b8f40 100644 --- a/pkg/sentry/time/calibrated_clock.go +++ b/pkg/sentry/time/calibrated_clock.go @@ -107,14 +107,15 @@ func (c *CalibratedClock) resetLocked(str string, v ...any) { // may need to be adjusted slightly from these values to compensate for error. // // Preconditions: c.mu must be held for writing. -func (c *CalibratedClock) updateParams(actual Parameters) { - if !c.ready { - // At initial calibration there is nothing to correct. +func (c *CalibratedClock) updateParams(actual Parameters, parked bool) { + if !c.ready || parked { + // when parked nothing has read the time for a whole interval, so + // assume errors have been compensated and recalibrate from scratch c.params = actual - c.ready = true - - c.Infof("ready") - + if !c.ready { + c.ready = true + c.Infof("ready") + } return } @@ -148,9 +149,12 @@ func (c *CalibratedClock) updateParams(actual Parameters) { // the clock is calibrated. Update should be called regularly to prevent the // clock from getting significantly out of sync from the reference clock. // +// parked indicates that this Update resumes a parked (unobserved) period; see +// updateParams. +// // The returned timekeeping parameters are invalidated on the next call to // Update. -func (c *CalibratedClock) Update() (Parameters, bool) { +func (c *CalibratedClock) Update(parked bool) (Parameters, bool) { c.mu.Lock() defer c.mu.Unlock() @@ -188,7 +192,7 @@ func (c *CalibratedClock) Update() (Parameters, bool) { Frequency: (minHz + maxHz) / 2, BaseRef: newest.ref, BaseCycles: newest.after, - }) + }, parked) return c.params, true } @@ -241,9 +245,9 @@ func NewCalibratedClocks() *CalibratedClocks { } // Update implements Clocks.Update. -func (c *CalibratedClocks) Update() (Parameters, bool, Parameters, bool) { - monotonicParams, monotonicOk := c.monotonic.Update() - realtimeParams, realtimeOk := c.realtime.Update() +func (c *CalibratedClocks) Update(parked bool) (Parameters, bool, Parameters, bool) { + monotonicParams, monotonicOk := c.monotonic.Update(parked) + realtimeParams, realtimeOk := c.realtime.Update(parked) return monotonicParams, monotonicOk, realtimeParams, realtimeOk } diff --git a/pkg/sentry/time/calibrated_clock_test.go b/pkg/sentry/time/calibrated_clock_test.go index f0847191ce..9b671ddec6 100644 --- a/pkg/sentry/time/calibrated_clock_test.go +++ b/pkg/sentry/time/calibrated_clock_test.go @@ -43,7 +43,7 @@ func TestConstantFrequency(t *testing.T) { // Update from all samples. for range samples { - c.Update() + c.Update(false) } c.mu.RLock() @@ -109,12 +109,12 @@ func TestErrorCorrection(t *testing.T) { c := newTestCalibratedClock(tc.samples[:], nil) // Initial calibration takes two updates. - _, ok := c.Update() + _, ok := c.Update(false) if ok { t.Fatalf("Update ready too early") } - params, ok := c.Update() + params, ok := c.Update(false) if !ok { t.Fatalf("Update not ready") } @@ -139,7 +139,7 @@ func TestErrorCorrection(t *testing.T) { } // Update again to see the changed reference clock. - params, ok = c.Update() + params, ok = c.Update(false) if !ok { t.Fatalf("Update not ready") } @@ -165,7 +165,7 @@ func TestErrorCorrection(t *testing.T) { initialErr := c.errorNS t.Logf("initial error: %v ns", initialErr) - _, ok = c.Update() + _, ok = c.Update(false) if !ok { t.Fatalf("Update not ready") } @@ -173,7 +173,7 @@ func TestErrorCorrection(t *testing.T) { t.Errorf("errorNS increased, got %v want |%v| <= |%v|", c.errorNS, c.errorNS, initialErr) } - _, ok = c.Update() + _, ok = c.Update(false) if !ok { t.Fatalf("Update not ready") } diff --git a/pkg/sentry/time/clocks.go b/pkg/sentry/time/clocks.go index 837e860944..ec69cd52a7 100644 --- a/pkg/sentry/time/clocks.go +++ b/pkg/sentry/time/clocks.go @@ -21,7 +21,9 @@ type Clocks interface { // reference host clocks, and returning the new timekeeping parameters. // // Update should be called at approximately ApproxUpdateInterval. - Update() (monotonicParams Parameters, monotonicOk bool, realtimeParam Parameters, realtimeOk bool) + // + // parked indicates that the clock was not read for at least ApproxUpdateInterval + Update(parked bool) (monotonicParams Parameters, monotonicOk bool, realtimeParam Parameters, realtimeOk bool) // GetTime returns the current time in nanoseconds for the given clock. //