Re: [PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts()

[Rodolfo Giometti]

On Mon, 2026-09-28 at 13:59 +0100, David Woodhouse wrote:
> Theoretically, absent other bugs (qv), ntp_error should rarely be more
> than a few tens of nanoseconds and even that is the extreme case.
[...]
> However, I *have* seen (and fixed) the tick_length changes at chrony
> startup introducing 83µs into ntp_error, which would take *days* to
> drain through the normal ±1 dithering, and would screw up the actual
> frequency settings while it was draining.

Thanks, that gives the order of magnitude I was asking for. Tens of
nanoseconds is below what chrony or ntpd can resolve from a PPS
source, so in the normal case the two lines are indistinguishable.

I think the "Allow tick_length changes to apply mid-tick" fix should
land before (or together with) the patch applying ntp_error to the
snapshots, and the commit message of the latter should say so.

> I wonder if we should switch PPS to using ktime_get_snapshot_id() in an
> *earlier* patch, which wouldn't then include the behavioural change.
> Then the note in the 'Apply extrapolated error' patch can then cover
> PPS and we consider them all together.

Yes, please. That split works well for me:

- the patch switching pps_get_ts() to ktime_get_snapshot_id() is pure
  plumbing: ts_real keeps its current meaning, and its cost is the
  one you measured and I already accepted. I can ack that one;

- the semantic change then lives in "timekeeping: Apply extrapolated
  ntp_error to clock snapshots", covering PPS and the other snapshot
  users together. That is a timekeeping decision, and it is the patch
  where the chrony and ntpd maintainers should be Cc'ed and ack.

As a bonus the two changes can be bisected and reverted independently,
which helps if userspace does notice something.

> The PPS change does stand alone anyway — regardless of the snapshot
> *corrections*, I want PPS using snapshots so that it can report the
> actual *counter* values to userspace, like PTP is going to be able to.

That is new ABI for the PPS subsystem, so please post it as a separate
series, and I would like to see the proposed interface before the
code. Things I would want settled there: how userspace learns which
counter the value refers to (and what happens when the clocksource
changes), and that the existing ioctls and struct pps_ktime stay
unchanged for current users.

Please also keep RFC 2783 in mind: the PPS API is defined there and
LinuxPPS has to stay compliant with it, so the counter values should
come as an extension on top of it that RFC-based users (e.g.
time_pps_fetch() via timepps.h) can simply ignore.

> They shouldn't be compared directly with a clock_gettime() where
> userspace... is preempted and... calls into the vDSO to get the time...
> is preempted again and... eventually does something with that timestamp
> which it considers to be current, or worse paired with whatever happens
> before or after it.

Agreed for a single reading. My concern is systematic rather than
per-sample: chrony and ntpd do compare PPS timestamps with timestamps
taken from the sanitized clock (e.g. NTP packet timestamps), and a
slowly varying offset between the two lines does not average out the
way preemption jitter does. With ntp_error in the tens of nanoseconds
it is irrelevant; I just want the larger cases fixed first and
documented. I'll wait for Miroslav's opinion on the userspace side.

Ciao,

Rodolfo