All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rodolfo Giometti <giometti@enneenne.com>
To: David Woodhouse <dwmw2@infradead.org>,
	Richard Cochran <richardcochran@gmail.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	John Stultz <jstultz@google.com>,
	Thomas Gleixner <tglx@kernel.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Miroslav Lichvar <mlichvar@redhat.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Alexander Gordeev <agordeev@linux.ibm.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Subject: Re: [PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS
Date: Tue, 1 Sep 2026 17:35:22 +0200	[thread overview]
Message-ID: <f6e90a49-8c68-42e6-bdff-ea9b3ecbaf7b@enneenne.com> (raw)
In-Reply-To: <20260829210041.40649-3-dwmw2@infradead.org>

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.

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.

> 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.

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

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.

Rodolfo

  reply	other threads:[~2026-09-01 15:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-29 20:56 [PATCH v4 0/4] Add ntp_error to clock snapshot, enable NTP_PPS on tickless kernel David Woodhouse
2026-08-29 20:56 ` [PATCH v4 1/4] timekeeping: Apply extrapolated ntp_error to clock snapshots David Woodhouse
2026-08-29 20:57 ` [PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS David Woodhouse
2026-09-01 15:35   ` Rodolfo Giometti [this message]
2026-09-02  0:13     ` David Woodhouse
2026-08-29 20:57 ` [PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts() David Woodhouse
2026-09-01 15:35   ` Rodolfo Giometti
2026-09-01 23:56     ` David Woodhouse
2026-08-29 20:57 ` [PATCH v4 4/4] [DO NOT MERGE] ptp: ptp_vmclock: Add simulated 1PPS support David Woodhouse
2026-09-01 15:35 ` [PATCH v4 0/4] Add ntp_error to clock snapshot, enable NTP_PPS on tickless kernel Rodolfo Giometti
2026-09-01 23:37   ` David Woodhouse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f6e90a49-8c68-42e6-bdff-ea9b3ecbaf7b@enneenne.com \
    --to=giometti@enneenne.com \
    --cc=agordeev@linux.ibm.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=dwmw2@infradead.org \
    --cc=dwmw@amazon.co.uk \
    --cc=edumazet@google.com \
    --cc=jstultz@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlichvar@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=richardcochran@gmail.com \
    --cc=sboyd@kernel.org \
    --cc=tglx@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.