On Thu, Apr 17, 2025 at 11:37 PM Thomas Gleixner tglx@linutronix.de wrote:
On Thu, Apr 17 2025 at 17:46, John Stultz wrote:
On Sat, Apr 5, 2025 at 2:40 PM Thomas Gleixner tglx@linutronix.de wrote:
+static inline void tk_update_coarse_nsecs(struct timekeeper *tk, u64 offset) +{
offset *= tk->tkr_mono.mult;tk->coarse_nsec = (tk->tkr_mono.xtime_nsec + offset) >> tk->tkr_mono.shift;+}
Thinking more on this, I get that you're providing the offset to save the "at the point" time into the coarse value, but I think this ends up complicating things.
Instead it seems like we should just do: tk->coarse_nsec = tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift;
You end up with the same problem again because xtime_nsec can move backwards when the multiplier is updated, no?
That's assuming you update coarse_nsec on every call to do_adjtimex, which I don't think is necessary (or wanted - as it would sort of be a behavior change to the COARSE clockids).
The root issue here has been that there was a mistaken assumption that the small negative adjustment done to the xtime_nsec base to match the mult adjustment would only happen after a larger accumulation, but the timekeeping_advance() call from do_adjtimex() doesn't actually intend to advance the clock (just change the frequency), so those small negative adjustments made via do_adjtimex() between accumulations become visible to the coarse clockids.
Since the coarse clockids are expected to provide ~tick-granular time, if we are keeping separate state, we should only be incrementing/setting that state when we accumulate (with each cycle_interval). We don't need to be making updates to the coarse clock between ticks on every do_adjtime call. So saving off the tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift value after we actually accumulated something should be fine. Any inter-tick frequency adjustments to xtime_nsec can be ignored by the coarse clockid state.
I'll test with your updated patch here, as I suspect it will avoid the problem I'm seeing, but I do think things can be further simplified.
thanks -john