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:
@@ -1831,6 +1847,8 @@ void timekeeping_resume(void) /* Re-base the last cycle value */ tks->tkr_mono.cycle_last = cycle_now; tks->tkr_raw.cycle_last = cycle_now;
/* Reset the offset for the coarse time getters */tks->coarse_nsec = 0; tks->ntp_error = 0; timekeeping_suspended = 0;So using the clocksource-switch test in kselftest, I can pretty easily hit inconsistencies with this.
The reason is since we use the coarse_nsec as the nanosecond portion of the coarse clockids, I don't think we ever want to set it to zero, as whenever we do so, we lose the previous contents and cause the coarse time to jump back.
Bah. Obviously. What was I thinking?
It seems more likely that we'd want to do something similar to tk_update_coarse_nsecs() filling it in with the shifted down tk->tkr_mono.xtime_nsec.
Indeed. The earlier approach of handing the offset to timekeeping_update_from_shadow() was exactly doing that. I dropped that because of the uglyness vs. the TAI update case in adjtimex().
+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?
Thanks,
tglx