Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP

[John Stultz]

On Sun, Jun 21, 2026 at 3:00 PM David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote:
>
> From: David Woodhouse <dwmw@xxxxxxxxxxxx>
>
> cycle_interval is an integer number of counter cycles per NTP interval,
> so the real time it represents differs from the nominal
> NTP_INTERVAL_LENGTH by up to half a counter period. For coarse
> clocksources this is significant: the 3.579545 MHz ACPI PM timer at
> HZ=1000 rounds 3579.545 cycles up to 3580, making each tick 1.000127 ms
> (+127 PPM).
>
> Commit a386b5af8edd ("time: Compensate for rounding on odd-frequency
> clocksources") introduced xtime_remainder to compensate for exactly
> this, citing the same 127 PPM ACPI PM example. The compensation is
> correct and necessary, but it was applied inside the timekeeping
> accumulation in timekeeping.c: subtracted in the mult computation in
> timekeeping_adjust() and folded into the ntp_error update in
> logarithmic_accumulation(). That keeps the base rate correct and leaves
> NTP its full symmetric +/-MAXFREQ range rather than +373/-627 PPM, but
> the NTP code in ntp.c never sees it: tick_length is computed without the
> correction, so ntp.c's notion of how long a tick is disagrees with the
> rate timekeeping actually produces.
>
> Make the offset an explicit part of the NTP tick_length instead. Add
> ntp_data::cs_tick_adj, a fixed per-second addend that
> ntp_update_frequency() includes alongside ntp_tick_adj and time_freq.
> tk_setup_internals() computes it from the difference between the real
> cycle_interval duration and the nominal interval, stores it in the
> timekeeper, and hands it to NTP through a new argument to ntp_clear() --
> which already recomputes the frequency and is invoked after every
> clocksource (re)configuration. timekeeping_init() now uses TK_UPDATE_ALL
> for this; clearing NTP there is otherwise redundant since ntp_init() has
> just initialised it.
>
> ntp.c now computes the true tick rate, giving a single source of truth.
> Like ntp_tick_adj, cs_tick_adj stays internal to the kernel: userspace
> still sees the nominal 1.000000 ms tick via adjtimex and is unaware of
> the addends. timekeeping_adjust() and logarithmic_accumulation() use
> ntp_tick / xtime_interval directly, and xtime_remainder is removed.
>
> The base-rate arithmetic is unchanged: ntp_tick becomes
> xtime_interval << ntp_error_shift, so the mult division yields the same
> base mult and the ntp_error accumulation still nets to zero per tick.
>
> Beyond the cleanup of treating all the tick_length contributions
> (nominal interval, ntp_tick_adj, cs_tick_adj, time_freq) consistently
> as addends in one place, it also prepares for feed-forward discipline:
> a future timekeeping_set_reference() will set tick_length to track an
> absolute external reference such as a vmclock, and that path needs
> ntp.c to own a tick_length that already reflects the clocksource
> quantisation, with no hidden correction applied elsewhere.
>
> Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> Assisted-by: Kiro:claude-opus-4.8


Looks good! Thanks for the laborious iteations here, I appreciate your
work on this!
Acked-by: John Stultz <jstultz@xxxxxxxxxx>