Re: [PATCH 1/5] timekeeping: Allow tick_length changes to apply mid-tick

[John Stultz]

On Thu, Oct 22, 2026 at 6:39 AM David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote:
> On Wed, 2026-10-21 at 12:01 -0700, John Stultz wrote:
> > Might more obviously visually match the logic above where mult is
> > calculated (or alternatively you could alter the mult calcualtion to
> > match what you have here)?
> >
>
> In fact I already do the latter in patch 5 (as I need 'dividend' again
> to calculate the ntp_err_frac divergence rate), so it ends up looking
> like this, and does match:
>
> →               dividend = (tk->ntp_tick + skew) >> tk->ntp_error_shift;
> →               mult = div64_u64(dividend, tk->cycle_interval);
>                 /*
>                  * Stash the fractional part of the per-cycle ideal mult that
>                  * the integer @mult discards, scaled by 2^32, in clock-shifted
>                  * ns per cycle.
>                  */
>                 dividend -= (u64)mult * tk->cycle_interval;
>                 tk->ntp_err_frac = div64_u64(dividend << 32, tk->cycle_interval);
>
>                 /*
>                  * Rate adjustments are conceptually applied mid-tick in order
>                  * to avoid accumulating large deltas in ntp_error which can
>                  * take days to drain. Offset the ntp_error that will be
>                  * introduced by timekeeping_apply_adjustment(), accordingly.
>                  */
>                 if (tk->ntp_tick != old_ntp_tick) {
> →                       u64 old_dividend = (old_ntp_tick + skew) >> tk->ntp_error_shift;
> →                       s64 old_base_mult = div64_u64(old_dividend, tk->cycle_interval);

Ah, apologies, I'm still having to review this in drips and drabs as
time allows before my magpie brain gets distracted by other shiny
issues.

>                         s64 mult_delta = (s64)mult - old_base_mult;
>
>                         tk->ntp_error -= ((s64)offset * mult_delta)
>                                          << tk->ntp_error_shift;
>                 }
>
>
> I can happily pull the cosmetic part up to this patch, leaving those
> first two highlighted lines exactly as they are for symmetry, before
> patch 5 adds the extra bit in the middle.

Sounds good.

>
>
> > > +                       s64 mult_delta = (s64)mult - old_base_mult;
> >
> > It might be useful to add a comment clarifying exactly what the
> > mult_delta here represents vs the (mult - tk->tkr_mono.mult) value
> > later passed to timekeeping_apply_adjustment?
>
> Rather than "add a comment", I think that should actually be "rewrite
> the existing four-line comment cited above that clearly isn't doing its
> job".  How about this...
>
>         /*
>          * In timekeeping_apply_adjustment(), the mult_adj value is
>          * retrospectively applied as if it had come into effect at
>          * the *start* of the current tick.  Any discrepancy due to
>          * the fact that time has *actually* been counted at the old
>          * mult rate between the start of the tick and now ('offset'
>          * cycles into that tick) is dumped into ntp_error.
>          *
>          * When tick_length is changed by a large amount, that can
>          * lead to tens of microseconds of ntp_error being accrued.
>          *
>          * Fix this by conceptually applying the tick_length change
>          * now (at 'offset'), rather than the start of the tick.
>          *
>          * That means calculating what 'mult' would have been with
>          * the old ntp_tick length (but the *current* value of skew),
>          * and pre-adjusting ntp_error to compensate.
>          */
>
> It's more than four lines now, but if we want it fully stated in the
> code rather than just the commit message, I don't think it could be
> much smaller.

Looks good to me.

Thanks!
-john