Page MenuHomeFreeBSD

kern: clarify boot time
ClosedPublic

Authored by imp on May 5 2021, 2:16 AM.

Details

Summary

In FreeBSD, the current time is computed from uptime + boottime. Uptime
is a continuous, smooth function that's monotonically increasing. To
effect changes to the current time, boottime is adjusted so that the
above equation holds true. boottime is therefore not invariant, and
shouldn't be cached against future need.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

imp requested review of this revision.May 5 2021, 2:16 AM
rpokala added inline comments.
share/man/man9/time.9
58

If that's the case, this description is misleading. How about this:

The boottime is computed by the current time minus uptime.
When those two values change independently, the boottime
is re-calculated accordingly.
The
.Xr sysctl 8
.Va kern.boottime
returns this computed value.
sys/kern/kern_tc.c
116
"System boottime, the difference between current time and uptime.");
share/man/man9/time.9
58

Boot time is not computed. It is stored and adjusted. Current time is computed.

sys/kern/kern_tc.c
116

Again, it's stored and adjusted. Current time is computed. This wording can easily be misconstrued to mean that boot time is computed often.

share/man/man9/time.9
58

How about this?

The current time is computed by boottime + uptime.
The uptime increases monotonically.
When the current time is adjusted, the boottime is adjusted
correspondingly, to maintain equivalence.
Therefore, boottime is not invariant and should not be cached.
The
.Xr sysctl 8
.Va kern.boottime
returns the boottime value.
sys/kern/kern_tc.c
116

Again, it's stored and adjusted. Current time is computed. This wording can easily be misconstrued to mean that boot time is computed often.

It appears the wording from your initial comment -- though perhaps not the actual change -- can also be misconstrued that way, because I misconstrued it that way. :-p

Thanks for the feedback... Sorry if I'm being a bit grumpy til now in understanding it fully...

share/man/man9/time.9
58
Boottime is set when the first time is obtained.
The current time is computed by boottime + uptime.
The uptime increases monotonically.
When the current time is set, boottime is adjusted to maintain equivalence.
Therefore, boottime is not invariant and should not be cached.
The
.Xr sysctl 8
.Va kern.boottime
returns the boottime value.

Do you think that might be clearer?

sys/kern/kern_tc.c
116

Hmmm, I take your meaning. Is the following any better?

System boot time. Adjusted when time is set so the current time always is the sum of the up time and the boot time.
share/man/man9/time.9
58

This is clear to me.

sys/kern/kern_tc.c
116

How about,

System boot time: adjusted when current time is adjusted, see time.9

IMO a single line cannot give the topic of how boot epoch (which boot time in fact is) enough explanation without being incomplete or imprecise to possibly confuse the reader. And that boot epoch is what is adjusted when system time is changed or adjusted. That boot time is not immutable must be understood.

Or how about this?

System boot time: mutable, adjusted when current time is adjusted, see time.9

It's enough information to say that it's not immutable, that it's adjusted when the clock is adjusted, and for details read the man page.

I think that's how I'd approach it.

I would go with:

"Estimated System Boot time. When the clock is set, this is recalculated as new_time - uptime"

I think the "Estimated" is important, because neither clock-frequency tweaks, up to half a millisecond per second, nor leap-seconds are accounted for.

Is 'stepped' is a better word ?

"Estimated System Boot time. When the clock is stepped, this is recalculated as new_time - uptime"

Try again based on rpokala's feedback on my first attempt to
clarify things.

In D30116#676007, @phk wrote:

I would go with:

"Estimated System Boot time. When the clock is set, this is recalculated as new_time - uptime"

I think the "Estimated" is important, because neither clock-frequency tweaks, up to half a millisecond per second, nor leap-seconds are accounted for.

How about splitting the difference between @cy and @phk:

Estimated system boot time: adjusted when current time is adjusted; see time(9)
share/man/man9/time.9
58

"when the first time is obtained" ; did you mean "the first time it is obtained"?

Otherwise, I concur with @cy, this is pretty clear.

sys/kern/kern_tc.c
116

s/time.9/time(9)/g, since we generally refer to manpages with the section in parens, even if the file itself uses a .section suffix.

share/man/man9/time.9
52

When the current time is set, adjusted by ntpd, or when a new time is read when
the system resumes,


When the current time is set, adjusted by
.Xr ntpd 8 ,
or updated when the system resumes,
sys/kern/kern_tc.c
108

"acround" is not a cromulent word. :-)

"time" => "the current time"

109

"vaires" => "varies"

The reason I specifically said "set" or "stepped" instead of "adjusted" is that continous adjustments, ie: frequency changes, do not cause recalculation of the estimated boot time.

And fold in suggestions from cy and phk.
Duplicated wording because that's our current practice for documentation,
but also slanted each to the context it's in.

typo found during submission

share/man/man9/time.9
51

If we want to be really nit-picky: We only set it at boot if the hardware has a RTC+driver.

(I think we dropped the "set it from the filesystem", right ?)

52

... read from RTC ... ?

once more into the breach

share/man/man9/time.9
51

We have not.

share/man/man9/time.9
55

Actually: What precisely happens at resume ?

We get a new RTC timestamp, but doing "boottime = new_time - uptime" would be bogus ?

I guess we should be recalculating uptime on resume and leave the boottime intact ?

sys/kern/kern_tc.c
116

I went with a simpler string and confined things to the comment since these strings don't have much visibility.

share/man/man9/time.9
52
This time is initially set when the system boots, either from the RTC, or
from a time estimated from the system's root filesystem.

Except, what does "a time estimated from the system's root filesystem" refer to?

sys/kern/kern_tc.c
116
"Estimated system boottime");

(It's not a sentence, so no period is required.)

share/man/man9/time.9
55

This is what happens. Uptime doesn't tick while we're suspended. so if we go to sleep with 1hr of up time and boot time of Jan 1 at 12:34pm and wake up on Jan 15th at 4:45am, the new boot time is set to Jan 15 at 3:45am. I'd like to document current behavior in this code review.

We can debate whether this is actually the right thing to do. :) ACPI and APM went back and forth on this back in the day. And there was much angst about what to do about timers across this event. It's not clear that this change would be better or worse than the current behavior.

share/man/man9/time.9
52

For UFS, it's set to the time recorded in the superblock (which is time of last unmount).
For everything else, nothing (so if you use ZFS and have not RTC, you'll see 1970 times). I don't know if this was lost in the OpenZFS import, or was never there.
It's supposed to be 'whatever the filesystem set mp->mnt_time to' but only UFS does this trick.

rpokala's period removal and add 'estimated'

imp marked 16 inline comments as done.May 5 2021, 6:14 AM

Marked all comments as done. Time for bed for me, but if I've done so in error, please make a new comment (or if you find something else to maybe clarify).
I'm torn on the UFS super block thing... I'm tired so I'm not clarifying tonight, but will entertain the notion again when I wake.

share/man/man9/time.9
52
This time is initially set when the system boots, either from the RTC, or
from a time estimated from the system's root filesystem.

The "is set initially" => "is initially set", and the comma after "boots", are still pending.

missed tweaks from rpokala.

Thanks for the changes!

LGTM, but wait for @cy and/or @phk before committing.

This revision is now accepted and ready to land.May 5 2021, 6:48 AM
trasz added inline comments.
share/man/man9/time.9
55

There's a PR for this: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236702. Both Linux and OSX count sleep as part of uptime, and I'd argue that's the right thing to do: when sleeping the system is still up, it's just conserving power.

imp marked an inline comment as done.May 5 2021, 5:49 PM
imp added inline comments.
share/man/man9/time.9
55

Yes. Unless stepping uptime causes issues for pending timeout... the += part of the change is easy, but making sure it all works after is harder... And at the moment, suspend and resume are totally broken on my laptop, so I can't test it. But regardless, beyond the scope of this change.

I agree that the resume behaviour should fix uptime rather than boottime. (S/R also broke on my T480 these days).

Otherwise fine with me.

This revision was automatically updated to reflect the committed changes.
imp marked an inline comment as done.