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

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3179 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:
> > Rather than using that more accurate timestamp *only* in the case where
> > CONFIG_NTP_PPS is enabled, do so unconditionally.
> > 
> >   static inline void pps_get_ts(struct pps_event_time *ts)
> >   {
> > -#ifdef CONFIG_NTP_PPS
> >   	struct system_time_snapshot snap;
> >   
> >   	ktime_get_snapshot_id(CLOCK_REALTIME, &snap);
> >   	ts->ts_real = ktime_to_timespec64(snap.systime);
> > +#ifdef CONFIG_NTP_PPS
> >   	ts->ts_raw = ktime_to_timespec64(snap.monoraw);
> > -#else
> > -	ktime_get_real_ts64(&ts->ts_real);
> >   #endif
> >   }
> 
> Why are you removing ktime_get_real_ts64()?

Because it's inaccurate.

> The commit message says you are using the more accurate timestamp
> unconditionally. What the diff does is delete the !CONFIG_NTP_PPS
> branch. Those are not the same thing, and the second one changes an
> ABI: ts_real reaches userspace through PPS_FETCH on /dev/ppsN and
> through /sys/class/pps/ppsX/assert, documented in
> Documentation/ABI/testing/sysfs-pps and unchanged since 2008. Until
> now it came from ktime_get_real_ts64(), the same clock userspace reads
> with clock_gettime(CLOCK_REALTIME). After your patch it does not.

Hm? After the patch it still reports CLOCK_REALTIME. Just more
accurately. Which is exactly what the commit message was trying to say.

> That needs a good reason and the commit message does not give one.
> Give me that first. Until then the details do not matter.

ktime_get_real_ts64() returns an approximate value of CLOCK_REALTIME
which is limited by the quantisation of the multiplier in the kernel's
core timekeeping. ktime_get_snapshot_id(CLOCK_REALTIME) returns a value
which is *corrected* for that drift.

> Separately: are we sure that calling ktime_get_snapshot_id() does not
> introduce much larger delays than ktime_get_real_ts64()? pps_get_ts()
> runs in hard IRQ, and in pps-gpio it is the first statement of the
> handler. Whatever it costs sits between the edge and the timestamp,
> and that is the one thing PPS has to keep short.

The critical measure here is the time from the edge to the actual
tk_clock_read() or tk_clock_read_snapshot() call which captures the
hardware counter value which is converted into the CLOCK_REALTIME
reading.

ktime_get_snapshot_id() is what's been called in the CONFIG_NTP_PPS
case for a very long time — we're just making it use that same path
unconditionally. The difference between that and ktime_get_real_ts64()
is tiny, but non-zero — it's about 2ns to the snapshot on my x86 test
host.

Even ktime_get_real_ts64() is doing things before the snapshot that
perhaps it needn't (setting ts->tv_sec), and could save us a couple of
cycles. If this is a real concern, we could certainly look at putting
the clock snapshot as early as possible and optimising the common case
of clock_id == CLOCK_REALTIME. 

> I would like to see that measured on something other than an x86 VM
> with a TSC. A small 32-bit ARM board is what I worry about.

Ack. I'll take a look.

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