Re: [RFC PATCH v3 02/10] timekeeping: Remove xtime_remainder from ntp_error accumulation

[John Stultz]

On Fri, May 22, 2026 at 4:41 AM David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote:
>
> On Wed, 2026-05-20 at 14:33 +0100, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@xxxxxxxxxxxx>
> >
> > The ntp_error accumulator tracks the difference between the time actually
> > reported to consumers in xtime, and the *intended* time. The former is
> > subject to a sawtooth effect due to the quantisation of 'mult', which
> > means that it actually advances by 'xtime_interval' each tick, while
> > the intended clock advances by 'ntp_tick'.
> >
> > By dithering between adjacent integer values of 'mult' which result in
> > an 'xtime_interval' slightly higher/lower than the intended tick length,
> > the advancement of xtime is kept on average to the intended rate.
> >
> > The accounting should therefore adjust ntp_error by adding ntp_tick and
> > subtracting xtime_interval on each tick.
> >
> > Since commit a386b5af8edd ("time: Compensate for rounding on
> > odd-frequency clocksources") the value subtracted has been
> > (xtime_interval + xtime_remainder), which is wrong. 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.
> >
> > The value of xtime_remainder actually does represent the difference
> > between the tick period and xtime_interval, so simply adding it instead
> > of (+ tick length - xtime_remainder) might have made sense... except
> > that it's only calculated once at boot time, so it's inaccurate anyway.
> > So just kill it with fire.
> >
> > Also remove it 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")
> > Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > Assisted-by: Kiro:claude-opus-4.6-1m
>
> Hi John, I note you skipped this one and acked some of the rest of the
> series. Do you have concerns?
>

Sorry, I've been busy with other things and haven't had much time to
sit and think through the details on this one.

You've pointed out well that the error in calculating the mult value
(basically what gets rounded off) does get handled in the ntp_err and
the resulting mult adjustments.

My concern has mostly been for coarse grained clocksources, the error
in the computed cycle_interval may not line up well with the desired
ntp_interval and could be a secondary cause of error.

Now, I think part of our disconnect has been my working mental model
apparently pre-dates 78b98e3c5a66 ("timekeeping/ntp: Determine the
multiplier directly from NTP tick length"), where while the mult/shift
was calculated from the provided frequency, then we calculated the
cycle_interval to best match the ntp_interval in the same way as the
current code. We did the mult adjustment was done in proprortion to
the size of the ntp_error. This meant the quantization error for the
mult value was separate from the quantization error for the calculated
cycle_interval.

But I think after 78b98e3c5a66, since we recalculate the mult from the
ntp_tick/cycle_interval in timekeeping_adjust(), it means the
quantization error in the cycle_interval is reflected in the mult, so
the mult adjustment ("dithering") effectively cancels it out.

So I think that is why I was mistakenly concerned about this change.

Now, I'm still a little guarded as while I know you've done thorough
testing with the fine grained TSC, it's not clear if you've done
similar analysis with other clocksources (ACPI PM I think was the one
that motviated the change you are undoing). So that might strengthen
the argument.  But looking more carefully at current code, I think
you've got a good case.

Thanks again for putting up with my foggy memory and handwringing.  :)

Acked-by: John Stultz <jstultz@xxxxxxxxxx>

thanks
-john