From c87495d1280f5b34451d02e2fe424da9efad4716 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Fri, 25 Sep 2026 13:24:26 -0700 Subject: [PATCH] Pack `Task.gostate{,Seq,Time}` into a single 64-bit atomic int. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Task` used to have two fields (`gostate` and `gostateTime`) which are updated very frequently in lockstep (since the latter is the modification time of the former). To do so lock-less-ly, it used a `sync.SeqCount` to try to update them atomically. However, in practice, `gostateTime` (number of nanoseconds since boot) never reaches 64 bits, because sandboxes don't run for multiple centuries. So we can pack the `gostate` enum bits together with the timestamp bits into a single 64-bit field which can be written with native atomic operations, which is already much faster. The only downside is that now, sandboxes can only run for up to `2^61 - 1` nanoseconds, aka around 73 years, whereas before they could run for `2^63 - 1` nanoseconds, aka around 292 years. So we've lost the ability to run for a century. But wait, there's more. All the readers of this field tolerate stale reads. This means we can actually not use an atomic write at all, so long as we can guarantee that all 64 bits either get written or don't (i.e. no partial writes). This is the case for 64-bit architectures, but not 32-bit. So this introduces a new `StoreRelaxed` method on `atomicbitops.Uint64`, which is just a plain `a = b` for 64-bit architectures, but otherwise a regular atomic store for 32-bit architectures where this second optimization doesn't apply. Benchmarks: ``` │ before │ after │ │ sec/op │ sec/op vs base │ laptop/platform=kvm/bench=getpid 410.0n ± 1% 380.0n ± 1% -7.32% (p=0.002 n=6) laptop/platform=kvm/bench=getpidopt 407.5n ± 2% 376.5n ± 1% -7.61% (p=0.002 n=6) laptop/platform=systrap/bench=getpid 2.200µ ± 0% 2.177µ ± 1% -1.05% (p=0.001 n=15) laptop/platform=systrap/bench=getpidopt 709.8n ± 2% 682.6n ± 1% -3.83% (p=0.000 n=14+15) n2-standard-16/platform=kvm/bench=getpid 579.4n ± 1% 537.7n ± 1% -7.19% (p=0.000 n=32) n2-standard-16/platform=kvm/bench=getpidopt 579.3n ± 0% 533.4n ± 0% -7.92% (p=0.000 n=32) n2-standard-16/platform=systrap/bench=getpid 3.669µ ± 1% 3.632µ ± 1% -1.01% (p=0.000 n=32+33) n2-standard-16/platform=systrap/bench=getpidopt 1.211µ ± 1% 1.182µ ± 1% -2.39% (p=0.000 n=32+33) ``` --- pkg/atomicbitops/aligned_32bit_unsafe.go | 10 +++++ pkg/atomicbitops/aligned_64bit.go | 14 ++++++ pkg/sentry/kernel/task.go | 28 ++++++------ pkg/sentry/kernel/task_sched.go | 54 +++++++++++++++--------- 4 files changed, 71 insertions(+), 35 deletions(-) diff --git a/pkg/atomicbitops/aligned_32bit_unsafe.go b/pkg/atomicbitops/aligned_32bit_unsafe.go index a76c6ed30fa..9905a64f0bc 100644 --- a/pkg/atomicbitops/aligned_32bit_unsafe.go +++ b/pkg/atomicbitops/aligned_32bit_unsafe.go @@ -198,6 +198,16 @@ func (u *Uint64) RacyStore(v uint64) { *u.ptr() = v } +// StoreRelaxed is actually the same as `Store` on 32-bit architectures, +// since 64-bit plain stores are not atomic there. +// See comment on the analogous of this function in `aligned_64bit.go`. +// +//go:norace +//go:nosplit +func (u *Uint64) StoreRelaxed(v uint64) { + atomic.StoreUint64(u.ptr(), v) +} + // Add is analogous to atomic.AddUint64. // //go:nosplit diff --git a/pkg/atomicbitops/aligned_64bit.go b/pkg/atomicbitops/aligned_64bit.go index ecb37e6bbca..904ae37fd67 100644 --- a/pkg/atomicbitops/aligned_64bit.go +++ b/pkg/atomicbitops/aligned_64bit.go @@ -174,6 +174,20 @@ func (u *Uint64) RacyStore(v uint64) { u.value = v } +// StoreRelaxed stores `v` with **no** ordering guarantee. +// Useful only where readers tolerate a stale value. +// Race detection is disabled, so this must be used sparingly. +// On 64-bit architectures, readers are guaranteed to see either the old +// or the new value, no "partial writes" cases. +// The 32-bit-architecture variant of this function does a real atomic +// write to guarantee the same no-partial-write property. +// +//go:norace +//go:nosplit +func (u *Uint64) StoreRelaxed(v uint64) { + u.value = v +} + // Add is analogous to atomic.AddUint64. // //go:nosplit diff --git a/pkg/sentry/kernel/task.go b/pkg/sentry/kernel/task.go index 0ca12edfc9a..0da01e81145 100644 --- a/pkg/sentry/kernel/task.go +++ b/pkg/sentry/kernel/task.go @@ -108,22 +108,20 @@ type Task struct { // interruptChan is always notified after restore (see Task.run). interruptChan chan struct{} `state:"nosave"` - // gostateSeq allows Task.TaskGoroutineStateTime() to read gostate and - // gostateTime atomically. - // - // gostateSeq is owned by the task goroutine. - gostateSeq sync.SeqCount `state:"nosave"` - - // gostate is the current scheduling state of the task goroutine. + // gostate combines two fields in one 64-bit integer: + // - First `gostateBits` bits are the `TaskGoroutineState` enum value. + // - Rest of the bits are the value of Kernel.cpuClock when the state was + // last updated or refreshed. + // Packing them this way allows efficient atomic reads and writes, which + // is critical for performance on the syscall hot path. + // + // Despite the use of `atomicbitops.Uint64`, `gostate` is written to + // **with no barrier guarantee** from the task goroutine, for syscall hot + // path performance reasons. This means all readers (other than from the + // task goroutine) **must** tolerate stale reads. // // gostate is owned by the task goroutine. - gostate atomicbitops.Uint32 - - // gostateTime was the value of Kernel.cpuClock when gostate was last - // updated or refreshed. - // - // gostateTime is owned by the task goroutine. - gostateTime atomicbitops.Int64 + gostate atomicbitops.Uint64 // appCPUClock approximates the amount of time the task goroutine has spent // in TaskGoroutineRunningApp. @@ -754,7 +752,7 @@ func (t *Task) afterLoad(gocontext.Context) { ts.populateCache(t) } t.interruptChan = make(chan struct{}, 1) - t.gostate.Store(uint32(TaskGoroutineNonexistent)) + t.setGostate(TaskGoroutineNonexistent) if t.stop != nil { t.stopCount = atomicbitops.FromInt32(1) } diff --git a/pkg/sentry/kernel/task_sched.go b/pkg/sentry/kernel/task_sched.go index 43e830632b5..f1bafaa6116 100644 --- a/pkg/sentry/kernel/task_sched.go +++ b/pkg/sentry/kernel/task_sched.go @@ -64,34 +64,49 @@ const ( TaskGoroutineStopped ) +// gostateBits is the width of the `TaskGoroutineState` bits in `Task`'s +// `gostate` field. The remainder of the bits holds `Kernel.cpuClock` +// nanoseconds. With gostateBits = 3, this is enough for 73 years. +const gostateBits = 3 + // TaskGoroutineState returns the current state of the task goroutine. func (t *Task) TaskGoroutineState() TaskGoroutineState { - return TaskGoroutineState(t.gostate.Load()) + return TaskGoroutineState(t.gostate.Load() & (1<> gostateBits)) +} + +// setGostate sets the task goroutine state, timestamped with the current +// `Kernel.cpuClock`. +// +// Preconditions: The caller must be running on the task goroutine. +func (t *Task) setGostate(state TaskGoroutineState) { + // StoreRelaxed due to this being on the syscall hot path, and all readers + // are expected to tolerate stale reads. + t.gostate.StoreRelaxed(uint64(t.k.cpuClock.Load())<