Re: [PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3906 bytes --]

On Tue, 2026-09-01 at 17:35 +0200, Rodolfo Giometti wrote:
> On Sat, 2026-08-29 at 21:57 +0100, David Woodhouse wrote:
> > Whatever the original reasons were, the only *remaining* reason seems to
> > have been that the accuracy of the time captured by pps_get_ts() was poor
> > on tickless kernels due to the kernel's per-tick timekeeping mechanism.
> 
> "Whatever the original reasons were" and "seems to have been" is not
> enough to drop a dependency that has been there for fifteen years. The
> old comment is useless, I agree. But then we have to say what breaks
> and what does not, not guess.

What breaks is this:

The kernel's core timekeeping keeps a 'mult' (multiplier) value which
it adjusts per tick. As it's an integer, it can either go slightly too
fast, or slightly too slow. The kernel dithers between adjacent values,
to achieve the correct overall rate for CLOCK_REALTIME.

The kernel *tracks* the actual error between what it's reporting in
CLOCK_REALTIME, and what it *should* be reporting, in order to choose
whether to use the high or low value for the next tick.

... in a *tickful* kernel, that is. In a tickful kernel, CLOCK_REALTIME
never really gets that far from where it should be, because the 'mult'
rate is adjusted every tick.

In a *tickless* kernel, it can go a *very* long time without adjusting
'mult', and thus the reported CLOCK_REALTIME can get a very long way
ahead of, or behind, what the kernel actually knows the time to be.

Since patch 1 of this series, ktime_get_snapshot_id() returns the
*corrected* time, while ktime_get_real_ts64() returns the sawtoothing
version.

That's why PPS wasn't viable in a NO_HZ_FULL kernel, and now is.

> First a structural point. This is the only patch of the four that
> applies to mainline, and it has no build dependency on the rest. It is
> a three-line Kconfig delete that compiles on its own. That worries me:
> a small "pps:" patch that applies cleanly is exactly what gets picked
> up alone. Then NTP_PPS becomes selectable on tickless kernels without
> 1/4, and we are worse off than today. Reorder it last, or say in the
> commit message that it must not be applied without 1/4.

Ack.

> > A recent change to ktime_get_snapshot_id() which is used by pps_get_ts()
> > has fixed that problem, by applying a correction to the ::systime field
> 
> That "recent change" is 1/4 of this series, and it is in no tree yet.
> Reading this, one assumes the groundwork already landed. Say "the
> previous patch". Same wording in 3/4.

I thought people hated 'the previous patch'. Probably better to let the
timekeeping patch hit tip, then reference it by commit id. As I said,
there's no rush for any of this. Hell, if you don't care, there's no
*need* for any of this. It's a cleanup that seemed worth doing while I
was fixing things in this area.

> About the test. The pulse comes from 4/4, which derives it from the
> same counter the timekeeping reads. No independent reference anywhere.

As noted elsewhere, that's still showing what it needs to show because
it's all about how we calculate CLOCK_REALTIME *from* that counter.
With the PPS changes and *not* the timekeeping fix, we see large skews
during idle. Fixing ktime_get_snapshot_id() in patch 1 brings it back
to where it should be.

> Before I ack this I want to see:
> 
>    - a real source, pps-gpio with a GPS receiver, where pulse and system
>      clock are independent;
>    - NO_HZ_FULL, not only NO_HZ_IDLE;
>    - a run that goes through a long idle period, not just a busy system.
> 
> That is more work than a three-line delete suggests, I know. But those
> three lines unlock a configuration people will run against real
> receivers and then trust.

Sure, happy to put that together. I don't have actual PPS hardware;
I'll have to see what I can come up with.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]