diff --git a/pkg/sentry/kernel/task_block.go b/pkg/sentry/kernel/task_block.go index 37c662d11..e51e84ece 100644 --- a/pkg/sentry/kernel/task_block.go +++ b/pkg/sentry/kernel/task_block.go @@ -41,9 +41,10 @@ func (t *Task) BlockWithTimeout(C chan struct{}, haveTimeout bool, timeout time. return timeout, t.block(C, nil) } - start := t.Kernel().MonotonicClock().Now() + clock := t.Kernel().MonotonicClock() + start := clock.Now() deadline := start.Add(timeout) - err := t.BlockWithDeadline(C, true, deadline) + err := t.BlockWithDeadlineFrom(C, clock, true, deadline) // Timeout, explicitly return a remaining duration of 0. if linuxerr.Equals(linuxerr.ETIMEDOUT, err) { @@ -54,7 +55,7 @@ func (t *Task) BlockWithTimeout(C chan struct{}, haveTimeout bool, timeout time. // return due to a timeout, we may have used up any of the remaining time // since then. We cap the remaining timeout to 0 to make it easier to // directly use the returned duration. - end := t.Kernel().MonotonicClock().Now() + end := clock.Now() remainingTimeout := timeout - end.Sub(start) if remainingTimeout < 0 { remainingTimeout = 0 @@ -80,12 +81,22 @@ func (t *Task) BlockWithTimeoutOn(w waiter.Waitable, mask waiter.EventMask, time // // Preconditions: The caller must be running on the task goroutine. func (t *Task) BlockWithDeadline(C <-chan struct{}, haveDeadline bool, deadline ktime.Time) error { + return t.BlockWithDeadlineFrom(C, t.Kernel().MonotonicClock(), haveDeadline, deadline) +} + +// BlockWithDeadlineFrom is similar to BlockWithDeadline, except it uses the +// passed clock (instead of application monotonic clock). +// +// Most clients should use BlockWithDeadline or BlockWithTimeout instead. +// +// Preconditions: The caller must be running on the task goroutine. +func (t *Task) BlockWithDeadlineFrom(C <-chan struct{}, clock ktime.Clock, haveDeadline bool, deadline ktime.Time) error { if !haveDeadline { return t.block(C, nil) } // Start the timeout timer. - t.blockingTimer.Swap(ktime.Setting{ + t.blockingTimer.SetClock(clock, ktime.Setting{ Enabled: true, Next: deadline, }) @@ -102,26 +113,6 @@ func (t *Task) BlockWithDeadline(C <-chan struct{}, haveDeadline bool, deadline return err } -// BlockWithDeadlineFrom is similar to BlockWithDeadline, except it uses the -// passed clock (instead of application monotonic clock). -// -// Most clients should use BlockWithDeadline or BlockWithTimeout instead. -// -// Preconditions: The caller must be running on the task goroutine. -func (t *Task) BlockWithDeadlineFrom(C <-chan struct{}, clock ktime.Clock, haveDeadline bool, deadline ktime.Time) error { - if !haveDeadline { - return t.block(C, nil) - } - notifier, tchan := ktime.NewChannelNotifier() - timer := ktime.NewTimer(clock, notifier) - timer.Swap(ktime.Setting{ - Enabled: true, - Next: deadline, - }) - defer timer.Destroy() - return t.block(C, tchan) -} - // Block implements context.Context.Block func (t *Task) Block(C <-chan struct{}) error { return t.block(C, nil) diff --git a/pkg/sentry/kernel/time/BUILD b/pkg/sentry/kernel/time/BUILD index 4a9fa0c4b..b74e9643d 100644 --- a/pkg/sentry/kernel/time/BUILD +++ b/pkg/sentry/kernel/time/BUILD @@ -1,14 +1,27 @@ load("//tools:defs.bzl", "go_library") +load("//tools/go_generics:defs.bzl", "go_template_instance") package( default_applicable_licenses = ["//:license"], licenses = ["notice"], ) +go_template_instance( + name = "seqatomic_clock", + out = "seqatomic_clock_unsafe.go", + package = "time", + suffix = "Clock", + template = "//pkg/sync/seqatomic:generic_seqatomic", + types = { + "Value": "Clock", + }, +) + go_library( name = "time", srcs = [ "context.go", + "seqatomic_clock_unsafe.go", "time.go", "util.go", ], @@ -17,6 +30,7 @@ go_library( "//pkg/abi/linux", "//pkg/context", "//pkg/errors/linuxerr", + "//pkg/gohacks", "//pkg/sync", "//pkg/waiter", ], diff --git a/pkg/sentry/kernel/time/time.go b/pkg/sentry/kernel/time/time.go index fa40508de..03fb11ba9 100644 --- a/pkg/sentry/kernel/time/time.go +++ b/pkg/sentry/kernel/time/time.go @@ -421,8 +421,9 @@ func (s Setting) At(now Time) (Setting, uint64) { // // +stateify savable type Timer struct { - // clock is the time source. clock is immutable. - clock Clock + // clock is the time source. clock is protected by mu and clockSeq. + clockSeq sync.SeqCount `state:"nosave"` + clock Clock // listener is notified of expirations. listener is immutable. listener Listener @@ -519,12 +520,18 @@ func (t *Timer) runGoroutine() { // Tick requests that the Timer immediately check for expirations and // re-evaluate when it should next check for expirations. func (t *Timer) Tick() { - now := t.clock.Now() + // Optimistically read t.Clock().Now() before locking t.mu, as t.clock is + // unlikely to change. + unlockedClock := t.Clock() + now := unlockedClock.Now() t.mu.Lock() defer t.mu.Unlock() if t.paused { return } + if t.clock != unlockedClock { + now = t.clock.Now() + } s, exp := t.setting.At(now) t.setting = s if exp > 0 { @@ -576,12 +583,18 @@ func (t *Timer) Resume() { // Preconditions: The Timer must not be paused (since its Setting cannot // be advanced to the current time while it is paused.) func (t *Timer) Get() (Time, Setting) { - now := t.clock.Now() + // Optimistically read t.Clock().Now() before locking t.mu, as t.clock is + // unlikely to change. + unlockedClock := t.Clock() + now := unlockedClock.Now() t.mu.Lock() defer t.mu.Unlock() if t.paused { panic(fmt.Sprintf("Timer.Get called on paused Timer %p", t)) } + if t.clock != unlockedClock { + now = t.clock.Now() + } s, exp := t.setting.At(now) t.setting = s if exp > 0 { @@ -613,12 +626,18 @@ func (t *Timer) Swap(s Setting) (Time, Setting) { // - f cannot call any Timer methods since it is called with the Timer mutex // locked. func (t *Timer) SwapAnd(s Setting, f func()) (Time, Setting) { - now := t.clock.Now() + // Optimistically read t.Clock().Now() before locking t.mu, as t.clock is + // unlikely to change. + unlockedClock := t.Clock() + now := unlockedClock.Now() t.mu.Lock() defer t.mu.Unlock() if t.paused { panic(fmt.Sprintf("Timer.SwapAnd called on paused Timer %p", t)) } + if t.clock != unlockedClock { + now = t.clock.Now() + } oldS, oldExp := t.setting.At(now) if oldExp > 0 { t.listener.NotifyTimer(oldExp, oldS) @@ -639,15 +658,23 @@ func (t *Timer) SwapAnd(s Setting, f func()) (Time, Setting) { return now, oldS } -// Atomically invokes f atomically with respect to expirations of t; that is, t -// cannot generate expirations while f is being called. -// -// Preconditions: f cannot call any Timer methods since it is called with the -// Timer mutex locked. -func (t *Timer) Atomically(f func()) { +// SetClock atomically changes a Timer's Clock and Setting. +func (t *Timer) SetClock(c Clock, s Setting) { + var now Time + if s.Enabled { + now = c.Now() + } t.mu.Lock() defer t.mu.Unlock() - f() + t.setting = s + if oldC := t.clock; oldC != c { + oldC.EventUnregister(&t.entry) + c.EventRegister(&t.entry) + t.clockSeq.BeginWrite() + t.clock = c + t.clockSeq.EndWrite() + } + t.resetKickerLocked(now) } // Preconditions: t.mu must be locked. @@ -666,7 +693,7 @@ func (t *Timer) resetKickerLocked(now Time) { // Clock returns the Clock used by t. func (t *Timer) Clock() Clock { - return t.clock + return SeqAtomicLoadClock(&t.clockSeq, &t.clock) } // ChannelNotifier is a Listener that sends on a channel. diff --git a/pkg/sentry/syscalls/linux/sys_time.go b/pkg/sentry/syscalls/linux/sys_time.go index c59b7bf1a..f76bc6f3f 100644 --- a/pkg/sentry/syscalls/linux/sys_time.go +++ b/pkg/sentry/syscalls/linux/sys_time.go @@ -241,12 +241,7 @@ func ClockNanosleep(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) ( // If blocking is interrupted, the syscall is restarted with the original // arguments. func clockNanosleepUntil(t *kernel.Task, c ktime.Clock, end ktime.Time, rem hostarch.Addr, needRestartBlock bool) error { - var err error - if c == t.Kernel().MonotonicClock() { - err = t.BlockWithDeadline(nil, true, end) - } else { - err = t.BlockWithDeadlineFrom(nil, c, true, end) - } + err := t.BlockWithDeadlineFrom(nil, c, true, end) switch { case linuxerr.Equals(linuxerr.ETIMEDOUT, err):