google / gvisor

Application Kernel for Containers
https://gvisor.dev
Apache License 2.0
15.31k stars 1.27k forks source link

Support CLOCK_BOOTTIME #218

Open prattmic opened 5 years ago

prattmic commented 5 years ago

We could likely make CLOCK_BOOTTIME equivalent to CLOCK_MONOTONIC, as CLOCK_MONOTONIC already includes time spent not running during save / restore, which is the closest thing we have to "suspend".

https://github.com/google/gvisor/blob/8f4634997bd97810a85a70b71f000378d9db2e55/pkg/sentry/kernel/timekeeper.go#L119-L129

prattmic commented 5 years ago

systemd wants CLOCK_BOOTTIME, though it has a fallback: https://github.com/systemd/systemd/blob/04d7ca022843913fba5170c40be07acf2ab5902b/src/basic/time-util.c#L82

It's not immediately clear to me what functionality it loses.

Pixep commented 5 years ago

Is the current behavior of CLOCK_MONOTONIC during save/restore even considered the proper one? This actually looks like CLOCK_BOOTTIME to me.

prattmic commented 5 years ago

The Linux kernel doesn't have the concept of save / restore, so there is no defined "correct" behavior across S/R. I can see arguments for both sides. As is, the clock jump makes operations with a timeout likely to fire immediately upon restore. On the other hand, it may cause some anomalous timing statistics for things that happen to be measured across S/R.

Pixep commented 5 years ago

You're right, but aren't the situations where "it may cause some anomalous timing statistics" supposed to be using CLOCK_BOOTTIME or a real-time clock instead? I'm considering supporting you on that issue, hence the questions :). But I suppose we can assume it to be strictly equivalent to the current CLOCK_MONOTONIC for now as you said, and that is a different topic.

Pixep commented 5 years ago

I believe CLOCK_BOOTTIME is intended to only be an alias to the existing CLOCK_MONOTONIC (i.e. no dedicated kernel timer). This would effectively make this a minimal change. Is there any plan to make the two clocks independents instead?