Re: [RFC PATCH v3 03/10] timekeeping: Account for monotonicity adjustment in ntp_error

[John Stultz]

On Wed, May 20, 2026 at 6:52 AM David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote:
>
> From: David Woodhouse <dwmw@xxxxxxxxxxxx>
>
> timekeeping_apply_adjustment() modifies xtime_nsec to maintain vDSO
> monotonicity when mult changes:
>
>     xtime_nsec -= offset
>
> This ensures that the time reported to userspace does not jump when the
> multiplier is adjusted from one tick to the next. However, the ntp_error
> accumulator which tracks the difference between intended and actual
> clock position was not being updated updated to reflect this additional
> discrepancy.
>
> An earlier attempt at this compensation existed as:
>
>     ntp_error -= (interval - offset) << ntp_error_shift
>
> but was removed in commit c2cda2a5bda9 ("timekeeping/ntp: Don't align
> NTP frequency adjustments to ticks") because it was a major source of
> NTP error. That's because (interval - offset) was wrong: the subtraction
> of "interval" prematurely accounted for the changed xtime_interval of
> the next tick, which would be correctly accounted in the next
> accumulation anyway — a double subtraction.
>
> What is actually needed is just the "offset" part: ntp_error must be
> told that xtime_nsec moved by "offset" without a corresponding change
> in the intended position. For the normal ±1 mult dithering this is
> negligible (the adjustments cancel over time), but for larger mult
> changes — such as when an external reference clock sets a new
> frequency — the one-time uncompensated offset is significant.
>
> Fix by adjusting ntp_error by the correct amount:
>
>     ntp_error += offset << ntp_error_shift
>
> This keeps ntp_error consistent with the actual xtime_nsec position
> after the adjustment.
>
> Fixes: c2cda2a5bda9 ("timekeeping/ntp: Don't align NTP frequency adjustments to ticks")
> Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> Assisted-by: Kiro:claude-opus-4.6-1m
> ---
>  kernel/time/timekeeping.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index b84b05f9d460..95973e45d456 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -2317,6 +2317,7 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
>         tk->tkr_mono.mult += mult_adj;
>         tk->xtime_interval += interval;
>         tk->tkr_mono.xtime_nsec -= offset;
> +       tk->ntp_error += offset << tk->ntp_error_shift;
>  }

Having the details in the comment above this block would have still
been nice, but..

Acked-by: John Stultz <jstultz@xxxxxxxxxx>