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

[Rodolfo Giometti <giometti@enneenne.com>]

On 02/10/2026 11:14, David Woodhouse wrote:
> 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@amazon.co.uk>
>>>
>>> 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.
>

Yes, please do that run. This patch changes what PPS_FETCH returns to
userspace, so it has to wait for the chrony and ntpd people anyway; if
1-4 alone are good enough at 1PPS I'd rather not tie the tickless
enablement to it.

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming