Re: [PATCH 5/5] [DO NOT MERGE] timekeeping: Apply extrapolated ntp_error to clock snapshots

[David Woodhouse]

On Fri, 2026-10-02 at 10:30 +0200, Rodolfo Giometti wrote:
> On 01/10/2026 22:21, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > 
> > The time reported in ::systime of a system_time_snapshot is known to be
> > slightly inaccurate because of the way that the reported realtime clock
> > sawtooths around the *intended* time series, limited by the integer mult
> > value used to calculate the inter-tick times, and designed to ensure
> > smoothness and monotonicity for its consumers.
> > 
> > It is particularly inaccurate in a tickless kernel, where ntp_err_mult
> > is not adjusted on each tick, allowing the reported clock to diverge
> > from the intended time for a large number of ticks before re-converging.
> > 
> > This appears to be the reason why CONFIG_NTP_PPS is not enabled on
> > tickless kernels — because at that scale of precision, the realtime
> > snapshot at the time of the pulse bears little relation to the time the
> > kernel *actually* believes it to be, thus introducing random errors into
> > the PPS phase correction.
> 
> Since enabling NTP_PPS on tickless kernels no longer depends on this
> patch, I think this paragraph should go.

Yep. Assuming the NTP_PPS tickless enablement lands under separate
cover, after the main part of this series but before *this* "DO NOT
MERGE" patch, I should just lump PPS in with the other users listed
later for consideration, as you said.

For that assumption to be true, we have to be happy that the ntp_error
reductions in patches 1-4 are sufficient, and that we don't need to
*also* switch pps_get_ts() to ktime_get_snapshot_id() and have this
patch which applies the correction to the snapshot.

Which leads us to your next question...

> > It would be better for callers of get_device_system_crosststamp() and
> > ktime_get_snapshot_id() to receive the *accurate* time, not the
> > sanitized version provided to gettimeofday().
> 
> With 1-4 applied the correction at a PPS edge should be in the tens of
> ns you measured: is it worth having ts_real differ from clock_gettime()
> for that?

Good question; I've been wondering about that. In a sense, I'm fixing
the same problem *three* times. First I eliminate the cases which
*introduce* significant ntp_error (patches 1-2), then I let the system
*eliminate* it when it does happen (patches 3-4) and now this patch
even *deducts* what remains from the snapshots.

I think all three *do* make sense, even together. Especially now my
last-minute Sashiko review pointed out that the 'eliminate' part is
only for the core timekeeper and not the aux clocks (we *could* change
that, at a cost of extra work on the timekeeping_max_deferment() path).

But also, even for the core timekeeper in a tickless kernel, that
ntp_error can still accumulate at *any* time. If it has exceeded the
elimination threshold while the system sleeps, it could still pollute a
snapshot which is taken at wake time, before the correction has a
chance to happen.

So I think we do need it, and my inclination is to hold off on enabling
CONFIG_NTP_PPS for tickless kernels until we do. But I'll defer to your
preference. If you want to merge it sooner on the basis that with a
1PPS signal the system doesn't get to sleep for long *anyway*, I can do
another test run with just patches 1-4.

Attachment: smime.p7s
Description: S/MIME cryptographic signature