Handle pending CPU clock tick in incRunningTasks().

PiperOrigin-RevId: 481060107

pkg/sentry/kernel/kernel.go

@@ -213,6 +213,9 @@ type Kernel struct {
 	// cpuClock is mutable, and is accessed using atomic memory operations.
 	cpuClock atomicbitops.Uint64
 
+	// cpuClockTickTimer drives increments of cpuClock.
+	cpuClockTickTimer *time.Timer `state:"nosave"`
+
 	// cpuClockMu is used to make increments of cpuClock, and updates of timers
 	// based on cpuClock, atomic.
 	cpuClockMu cpuClockMutex `state:"nosave"`
@@ -1130,6 +1133,7 @@ func (k *Kernel) Start() error {
 	}
 
 	k.started = true
+	k.cpuClockTickTimer = time.NewTimer(linux.ClockTick)
 	k.runningTasksMu.Lock()
 	k.cpuClockTickerRunning = true
 	k.runningTasksMu.Unlock()
@@ -1250,11 +1254,51 @@ func (k *Kernel) incRunningTasks() {
 			return
 		}
 
-		// Transition from 0 -> 1. Synchronize with other transitions and timer.
+		// Transition from 0 -> 1.
 		k.runningTasksMu.Lock()
-		if k.runningTasks.Add(1) == 1 {
+		if k.runningTasks.Load() != 0 {
+			// Raced with another transition and lost.
+			k.runningTasks.Add(1)
+			k.runningTasksMu.Unlock()
+			return
+		}
+		if !k.cpuClockTickerRunning {
+			select {
+			case tickTime := <-k.cpuClockTickTimer.C:
+				// Rearm the timer since we consumed the wakeup. Estimate how much time
+				// remains on the current tick so that periodic workloads interact with
+				// the (periodic) CPU clock ticker in the same way that they would
+				// without the optimization of putting the ticker to sleep.
+				missedNS := time.Since(tickTime).Nanoseconds()
+				missedTicks := missedNS / linux.ClockTick.Nanoseconds()
+				thisTickNS := missedNS - missedTicks*linux.ClockTick.Nanoseconds()
+				k.cpuClockTickTimer.Reset(time.Duration(linux.ClockTick.Nanoseconds() - thisTickNS))
+				// Increment k.cpuClock on the CPU clock ticker goroutine's behalf.
+				// (Whole missed ticks don't matter, and adding them to k.cpuClock will
+				// just confuse the watchdog.) At the time the tick occurred, all task
+				// goroutines were asleep, so there's nothing else to do. This ensures
+				// that our caller (Task.accountTaskGoroutineLeave()) records an
+				// updated k.cpuClock in Task.gosched.Timestamp, so that it's correctly
+				// accounted as having resumed execution in the sentry during this tick
+				// instead of at the end of the previous one.
+				k.cpuClock.Add(1)
+			default:
+			}
+			// We are transitioning from idle to active. Set k.cpuClockTickerRunning
+			// = true here so that if we transition to idle and then active again
+			// before the CPU clock ticker goroutine has a chance to run, the first
+			// call to k.incRunningTasks() at the end of that cycle does not try to
+			// steal k.cpuClockTickTimer.C again, as this would allow workloads that
+			// rapidly cycle between idle and active to starve the CPU clock ticker
+			// of chances to observe task goroutines in a running state and account
+			// their CPU usage.
+			k.cpuClockTickerRunning = true
 			k.runningTasksCond.Signal()
 		}
+		// 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.
+		k.runningTasks.Store(1)
 		k.runningTasksMu.Unlock()
 		return
 	}

pkg/sentry/kernel/task_sched.go

@@ -336,37 +336,30 @@ func (tg *ThreadGroup) CPUClock() ktime.Clock {
 }
 
 func (k *Kernel) runCPUClockTicker() {
-	tickTimer := time.NewTimer(linux.ClockTick)
 	rng := rand.New(rand.NewSource(rand.Int63()))
 	var tgs []*ThreadGroup
 
 	for {
-		// Wait for the next CPU clock tick.
-		wokenEarly := false
-		select {
-		case <-tickTimer.C:
-			tickTimer.Reset(linux.ClockTick)
-		case <-k.cpuClockTickerWakeCh:
-			// Wake up to check if we need to stop with cpuClockTickerRunning =
-			// false, but then continue waiting for the next CPU clock tick.
-			wokenEarly = true
-		}
-
 		// Stop the CPU clock while nothing is running.
 		if k.runningTasks.Load() == 0 {
 			k.runningTasksMu.Lock()
 			if k.runningTasks.Load() == 0 {
 				k.cpuClockTickerRunning = false
 				k.cpuClockTickerStopCond.Broadcast()
-				for k.runningTasks.Load() == 0 {
-					k.runningTasksCond.Wait()
-				}
-				k.cpuClockTickerRunning = true
+				k.runningTasksCond.Wait()
+				// k.cpuClockTickerRunning was set to true by our waker
+				// (Kernel.incRunningTasks()). For reasons described there, we must
+				// process at least one CPU clock tick between calls to
+				// k.runningTasksCond.Wait().
 			}
 			k.runningTasksMu.Unlock()
 		}
 
-		if wokenEarly {
+		// Wait for the next CPU clock tick.
+		select {
+		case <-k.cpuClockTickTimer.C:
+			k.cpuClockTickTimer.Reset(linux.ClockTick)
+		case <-k.cpuClockTickerWakeCh:
 			continue
 		}