On Sat, Mar 15, 2025 at 12:23 PM Thomas Gleixner tglx@linutronix.de wrote:
On Fri, Mar 14 2025 at 17:37, John Stultz wrote:
Here because we did not accumulate anything, we have a window where the _COARSE clockids that don't utilize the mult*offset value, can see an inconsistency.
So to fix this, rework the timekeeping_advance() logic a bit so that when we are called from do_adjtimex() and the offset is smaller then cycle_interval, that we call timekeeping_forward(), to first accumulate the sub-interval time into xtime_nsec. Then with no unaccumulated cycles in offset, we can do the mult adjustment without worry of the subtraction having an impact.
It's a smart solution. I briefly pondered something similar, but I'm not really fond of the fact, that it causes a clock_was_set() event for no good reason.
clock_was_set() means that there is a time jump. But that's absolutely not the case with do_adjtimex() changing the frequency for quick adjustments. That does not affect continuity at all.
Oh, good point. I wasn't thinking clearly about the semantics, and was being a little paranoid there (as most calls to timekeeping_forward_now() have clock_was_set() calls that follow). I suspect I can do away with that bit and avoid it. I'll respin later this week.
That event causes avoidable overhead in the kernel, but it's also exposed to user space via timerfd TFD_TIMER_CANCEL_ON_SET.
I have no really strong opinion about that, but the route through clock_was_set() triggers my semantical mismatch sensors :)
Yeah, it's a fair point, thanks for raising it!
NOTE: This was implemented as a potential alternative to Thomas' approach here: https://lore.kernel.org/lkml/87cyej5rid.ffs@tglx/
And similarly, it needs some additional review and testing, as it was developed while packing for conference travel.
We can debate that next week over your favourite beverage :)
Looking forward to it :) -john