Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[John Stultz]

On Wed, May 13, 2026 at 2:02 PM David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote:
>
> From: David Woodhouse <dwmw@xxxxxxxxxxxx>
>
> The ntp_error accumulator tracks the difference between intended and
> actual clock advance. Each tick it adds ntp_tick (the intended advance)
> and subtracts what the clock actually advanced.
>
> The subtraction was (xtime_interval + xtime_remainder), but only
> xtime_interval is actually added to xtime_nsec each tick.
> xtime_remainder was a boot-time constant representing the rounding error
> from converting the tick period to an integer number of counter cycles.
> It was never added to xtime_nsec, so subtracting it from ntp_error
> created a phantom credit that biased the dithering ratio.
>
> The effect is a systematic drift whose magnitude depends on the value of
> xtime_remainder and the NTP frequency correction. NTP masks this by
> continuously adjusting the frequency to compensate, but with a fixed
> frequency (or an external reference clock like vmclock), the drift is
> exposed.
>
> Also remove xtime_remainder from the mult computation in
> timekeeping_adjust(), which used it to offset the division for the same
> (incorrect) reason.
>
> Fixes: a386b5af8edd ("time: Compensate for rounding on odd-frequency clocksources")

Hey, thanks for sending this out!

Looks like it would be more accurate to say it is a revert then a fix?

Hrm. So this is maybe too far back for my goldfish brain to really remember.

My hazy sense of it was that for coarser-grained clocksources, there
will be an inherent granularity error for what we want to use as an
accumulation interval, and the hardware allows.

So if we want a 1ms ntp interval, taking the hardware freq and finding
cycles/ms, might be something like 10.4 (imaginary example here), so
that rounds 10 cycles per interval which is really gives us a 962us
interval.

Now, the 10.4 cycles/ms is still accurate, and the resulting
xtime_interval is still correct, but there is a resulting 38us
difference between the xtime_interval and the ntp_tick due to this
granularity error in the calculation., When we start accumulating
ntp_error, it will grow at that 38us/ms rate, causing the internal
logic to try to steer the clock faster to compensate.

Thus, instead of assuming the ntp-interval is actually 1ms, we want it
to match the actual accumulation interval, so we subtract that error
off of the ntp-interval.  That "phantom credit" is effectively
intended to address this inherent error (which is why it isn't applied
to the other side - since it's already there as part of the
calculation).

Now, again, it's been far too long, and I may be misremembering
details, and the correction may very well be unnecessary or otherwise
erronious! But I think it deserves a bit more evidence/rationale in
the commit message as to why.

thanks
-john