Re: [PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS
From: David Woodhouse <dwmw2@infradead.org>
Date: 2026-09-02 00:14:02
Also in:
lkml
On Tue, 2026-09-01 at 17:35 +0200, Rodolfo Giometti wrote:
On Sat, 2026-08-29 at 21:57 +0100, David Woodhouse wrote:quoted
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.
quoted
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 fieldThat "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.
Attachments
- smime.p7s [application/pkcs7-signature] 6179 bytes