Update the stathz description to reflect reality. profhz is the only
thing we should deprecate.
Sponsored by: Netflix
Details
Details
- Reviewers
jhb kib - Group Reviewers
manpages - Commits
- rG1426907f4dd0: hz.9: update stathz for current usage
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 39980 Build 36869: arc lint + arc unit
Event Timeline
| share/man/man9/hz.9 | ||
|---|---|---|
| 57–58 | You have an either without an or any more. | |
| share/man/man9/hz.9 | ||
|---|---|---|
| 57–58 | While it is true that historically we used the same physical timer to drive statclock and profclock, it might be clearer to the reader to actually describe them as three separate logical clocks and then maybe add a note after the description of statclock and profclock to say that historically these two clocks were driven by the same physical timer. I think it will also fix Jess's comment about the missing or. Phab's UI won't let me select the body of text to do an suggestion since there's already a comment, so I'll just do a suggestion by hand: A second clock running at .Va stathz gathers statistics on the system and its processes. It computes values for ... .Xr top 1 . .Pp Finally, a third clock runs at .Vt profhz to sample user program counter values for profiling purposes. This profiling clock has been replaced by the more functional .Xr hwpmc 4 and may be removed in a future version of .Fx . Maybe it's not even worth mentioning the historical note that statclock and profclock use the same underlying timer, or that profclock doesn't run except when it is enabled by at least one process requesting profiling. If we did, maybe we would add it in a separate '.Sh IMPLEMENTATION NOTES' section. Also, I snuck in a fuller deprecation warning for profclock. :) | |
Comment Actions
Update, per jhb@ review, adopting his wording...
Unsure about the implementation details section, so please comment.
| share/man/man9/hz.9 | ||
|---|---|---|
| 52–54 | hardclock needs to be updated, and I'll put the details there. | |
| share/man/man9/hz.9 | ||
|---|---|---|
| 52–54 | In the hardclock.9 page, I'll say "the sum of cnt over a second will tend to be about hz" or something similar.... It looks like we make our best guesses on ticks, but nothing really cares how exact we are on that for time keeping, within certain limits dictated by overflow and such. | |
| share/man/man9/hz.9 | ||
|---|---|---|
| 52 | I think something along these lines would be more accurate: This routine is called with the varying frequency, enough to handle events fired from it, in time. Highest frequency is hz, and current implementation ensures that the function is called at least once in a second if CPU is idle. | |
| 69 | s/clock/mechanism/ | |
| share/man/man9/hz.9 | ||
|---|---|---|
| 52–54 | I think that's correct, but probably not quite the right way to express it, essentially hardclock is called on an upcoming tick 1/hz boundry when necessary. But anyway this is outside of the scope of hz.9. | |
Comment Actions
skate the middle ground between all the suggested frequency of hardclock
wordings. I believe this tersely, but accurately, reflects the system behavior.
Comment Actions
tempt fate and tweak a word or two here and there and
add a senence saying what these clocks do.
| share/man/man9/hz.9 | ||
|---|---|---|
| 40 | As an aside I brought up removing -pg profiling support last year (https://lists.freebsd.org/pipermail/freebsd-current/2020-January/075105.html) | |
| 55 | Interesting, I wonder if we can eliminate the at least once per second? | |
| share/man/man9/hz.9 | ||
|---|---|---|
| 40 | Yea. I saw that and wasn't sure what to think, so said nothing. | |
| 55 | No. I don't think so.
There may be other reasons as well. hardclock clocks polling devices and queueing the end of epoch processing (If I'm reading the code right, I just took a quick glance). There may be other reasons I'm unaware of that kib@ or mav@ could expound upon. | |
| share/man/man9/hz.9 | ||
|---|---|---|
| 40 |
We haven't turned PROFILE off by default yet. | |
| share/man/man9/hz.9 | ||
|---|---|---|
| 55 | Yes, the main reason is the timecounters. They are set up to return uint32_t value which we allow to wrap in approximately a second. In particular, this is why we have TSC-low timecounter which shifts TSC value right. And this means that we should call tc_windup() once in half-second, actually, not once in a second. We indeed do, see getnextcpuevent(). Also this frequency determines how fast we can reclaim resources when epochs end, which imp noted above. On the other hand, this 'once in half-second' really need to occur once in half-second globally, not on each CPU. Right now we do not utilize this possible optimization. | |
Comment Actions
The other niglet about once a second is so that the kern.cp_time is updated for idle CPUs at least once a second so that top(1) works. Right now when a CPU is idle we don't schedule statclock on it as an optimization and instead do a fixup later when the CPU resumes assuming all the missed ticks were idle ticks. Perhaps we could do this fixup globally in the same way as is done for tc_windup as Konstantin mentions, but I don't think that has been implemented.
Comment Actions
So if there's still issues, lemme know and I'll work with you. Wanted to get this round into the tree.
next up, hardclock.9