From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A1501371D14 for ; Sun, 21 Jun 2026 22:01:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782079271; cv=none; b=ehL8zW2+pBX0u1b4aA9IV+2lyLlAId+4s0ItBcM3fAokMID61Zc025FAtCwoAU00g516pEyfPHtk6phYXlcTy/L5oEr0E9RpLAsTGKUK2CG1Af0nbWbnejBINsIbZ2eGtL9wqbVhK7D3d5kysg3uo2CHMEdgiu6StvKjM74mdUY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782079271; c=relaxed/simple; bh=kBuLKwki11v4zjKV+jsmhfJIiRhuFnGDRQyP5Sw+auQ=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=hwma5FPMnIRyJIj9ePG5F9/e6tJygAJ8HxFLdnYKQBmsCao7D5ZC8Gx8YGchCz6ODJaYYuCWZpTXni6rpdbA/49/ikUDrHNWItfHVbcD4cc67A+9XEOmCtgtlpOHN9lYoQhoVdPoVmw5hsy5MK8izeIEpqGyVvUas/2O+iXxn2c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=casper.srs.infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=sZCD/kmp; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=casper.srs.infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="sZCD/kmp" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:To: From:Reply-To:Cc:Content-ID:Content-Description; bh=dhQOYOBkHRI+Q1n/1oHkQCJNgu+ArTyhdxORS6Pu3lo=; b=sZCD/kmpa8/l7kW0yASbrPNudH bDn0Ld0tBl33ULJ1IcaCg9rFxzQ3s16VfFKAHgEQgvqkaZJtocep11b+Idk/br8+wjDc5iFKnGzZB oTDt/5svRAqumaxXechXP17O6xhi24vxHugxKpB5D8WMBdr1Z+g0mWm5xhtcnqoHqLSLZKg4AOgUx xeyIkHpLMgmtJFHedk9NniOIvHlLnK1+UjldldPtwZqGz1wFryrI0OZmbxoOWxKG/cJzE35ifpI2M 9e3kdbAKiPtuEX9ytQ3xafZcu63mrYDMNHJE787wtay8bZFeivkHJ2BgsQR/gQbBHvfHUX8xmLIJ7 s53zpWeg==; Received: from [2001:8b0:10b:1::425] (helo=i7.infradead.org) by casper.infradead.org with esmtpsa (Exim 4.99.1 #2 (Red Hat Linux)) id 1wbQDl-0000000349o-0hmg; Sun, 21 Jun 2026 22:00:54 +0000 Received: from dwoodhou by i7.infradead.org with local (Exim 4.99.2 #2 (Red Hat Linux)) id 1wbQDk-00000004K7s-3Loa; Sun, 21 Jun 2026 23:00:52 +0100 From: David Woodhouse To: Richard Cochran , Wen Gu , David Woodhouse , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , John Stultz , Thomas Gleixner , Stephen Boyd , Anna-Maria Behnsen , Frederic Weisbecker , Shuah Khan , Peter Zijlstra , =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= , Arnd Bergmann , Miroslav Lichvar , Julien Ridoux , Ryan Luu , linux-kernel@vger.kernel.org Subject: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP Date: Sun, 21 Jun 2026 22:53:56 +0100 Message-ID: <20260621220051.1030462-4-dwmw2@infradead.org> X-Mailer: git-send-email 2.54.0 In-Reply-To: <20260621220051.1030462-1-dwmw2@infradead.org> References: <20260621220051.1030462-1-dwmw2@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: David Woodhouse X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html From: David Woodhouse cycle_interval is an integer number of counter cycles per NTP interval, so the real time it represents differs from the nominal NTP_INTERVAL_LENGTH by up to half a counter period. For coarse clocksources this is significant: the 3.579545 MHz ACPI PM timer at HZ=1000 rounds 3579.545 cycles up to 3580, making each tick 1.000127 ms (+127 PPM). Commit a386b5af8edd ("time: Compensate for rounding on odd-frequency clocksources") introduced xtime_remainder to compensate for exactly this, citing the same 127 PPM ACPI PM example. The compensation is correct and necessary, but it was applied inside the timekeeping accumulation in timekeeping.c: subtracted in the mult computation in timekeeping_adjust() and folded into the ntp_error update in logarithmic_accumulation(). That keeps the base rate correct and leaves NTP its full symmetric +/-MAXFREQ range rather than +373/-627 PPM, but the NTP code in ntp.c never sees it: tick_length is computed without the correction, so ntp.c's notion of how long a tick is disagrees with the rate timekeeping actually produces. Make the offset an explicit part of the NTP tick_length instead. Add ntp_data::cs_tick_adj, a fixed per-second addend that ntp_update_frequency() includes alongside ntp_tick_adj and time_freq. tk_setup_internals() computes it from the difference between the real cycle_interval duration and the nominal interval, stores it in the timekeeper, and hands it to NTP through a new argument to ntp_clear() -- which already recomputes the frequency and is invoked after every clocksource (re)configuration. timekeeping_init() now uses TK_UPDATE_ALL for this; clearing NTP there is otherwise redundant since ntp_init() has just initialised it. ntp.c now computes the true tick rate, giving a single source of truth. Like ntp_tick_adj, cs_tick_adj stays internal to the kernel: userspace still sees the nominal 1.000000 ms tick via adjtimex and is unaware of the addends. timekeeping_adjust() and logarithmic_accumulation() use ntp_tick / xtime_interval directly, and xtime_remainder is removed. The base-rate arithmetic is unchanged: ntp_tick becomes xtime_interval << ntp_error_shift, so the mult division yields the same base mult and the ntp_error accumulation still nets to zero per tick. Beyond the cleanup of treating all the tick_length contributions (nominal interval, ntp_tick_adj, cs_tick_adj, time_freq) consistently as addends in one place, it also prepares for feed-forward discipline: a future timekeeping_set_reference() will set tick_length to track an absolute external reference such as a vmclock, and that path needs ntp.c to own a tick_length that already reflects the clocksource quantisation, with no hidden correction applied elsewhere. Signed-off-by: David Woodhouse Assisted-by: Kiro:claude-opus-4.8 --- include/linux/timekeeper_internal.h | 8 ++-- kernel/time/ntp.c | 27 ++++++++++-- kernel/time/ntp_internal.h | 2 +- kernel/time/timekeeping.c | 66 +++++++++++++++++++++-------- 4 files changed, 78 insertions(+), 25 deletions(-) diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h index e36d11e33e0c..ec81587a1400 100644 --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -84,8 +84,6 @@ struct tk_read_base { * @cycle_interval: Number of clock cycles in one NTP interval * @xtime_interval: Number of clock shifted nano seconds in one NTP * interval. - * @xtime_remainder: Shifted nano seconds left over when rounding - * @cycle_interval * @raw_interval: Shifted raw nano seconds accumulated per NTP interval. * @next_leap_ktime: CLOCK_MONOTONIC time value of a pending leap-second * @ntp_tick: The ntp_tick_length() value currently being @@ -99,6 +97,10 @@ struct tk_read_base { * @ntp_error_shift: Shift conversion between clock shifted nano seconds and * ntp shifted nano seconds. * @ntp_err_mult: Multiplication factor for scaled math conversion + * @cs_tick_adj: Per-second adjustment handed to NTP via ntp_clear() + * accounting for the difference between the nominal + * NTP interval and the real time taken by the + * clocksource's integer @cycle_interval (upscaled). * @skip_second_overflow: Flag used to avoid updating NTP twice with same second * @tai_offset: The current UTC to TAI offset in seconds * @@ -178,7 +180,6 @@ struct timekeeper { u64 cycle_interval; u64 xtime_interval; - s64 xtime_remainder; u64 raw_interval; ktime_t next_leap_ktime; @@ -186,6 +187,7 @@ struct timekeeper { s64 ntp_error; u32 ntp_error_shift; u32 ntp_err_mult; + s64 cs_tick_adj; u32 skip_second_overflow; s32 tai_offset; }; diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index 97fa99b96dd0..3fad82c47c4c 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -39,6 +39,10 @@ * @time_reftime: Time at last adjustment in seconds * @time_adjust: Adjustment value * @ntp_tick_adj: Constant boot-param configurable NTP tick adjustment (upscaled) + * @cs_tick_adj: Fixed per-second adjustment compensating for the difference + * between the nominal NTP interval and the real time taken + * by the clocksource's integer @cycle_interval (upscaled). + * Set by the timekeeping core via ntp_clear(). * @ntp_next_leap_sec: Second value of the next pending leapsecond, or TIME64_MAX if no leap * * @pps_valid: PPS signal watchdog counter @@ -70,6 +74,7 @@ struct ntp_data { time64_t time_reftime; long time_adjust; s64 ntp_tick_adj; + s64 cs_tick_adj; time64_t ntp_next_leap_sec; #ifdef CONFIG_NTP_PPS int pps_valid; @@ -255,6 +260,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata) second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ) << NTP_SCALE_SHIFT; second_length += ntpdata->ntp_tick_adj; + second_length += ntpdata->cs_tick_adj; second_length += ntpdata->time_freq; new_base = div_u64(second_length, NTP_INTERVAL_FREQ); @@ -350,11 +356,26 @@ static void __ntp_clear(struct ntp_data *ntpdata) } /** - * ntp_clear - Clears the NTP state variables - * @tkid: Timekeeper ID to be able to select proper ntp data array member + * ntp_clear - Clear NTP state and set the clocksource quantisation adjustment + * @tkid: Timekeeper ID + * @cs_tick_adj: Per-second adjustment in ns << NTP_SCALE_SHIFT + * + * The timekeeping core uses an integer number of cycles (@cycle_interval) + * per NTP interval, so the real time that interval represents differs from + * the nominal NTP_INTERVAL_LENGTH by up to half a counter period. Folding + * this fixed offset into @cs_tick_adj makes it an explicit part of the NTP + * tick_length computation in ntp.c, instead of being applied during + * timekeeping accumulation where the NTP code never saw it. Like + * @ntp_tick_adj it stays internal to the kernel; userspace still sees the + * nominal tick via adjtimex. NTP retains its full symmetric ±MAXFREQ range + * around the corrected base rate. + * + * Called whenever the clocksource is (re)configured, which is also when the + * rest of the NTP state must be cleared, so the two are done together. */ -void ntp_clear(unsigned int tkid) +void ntp_clear(unsigned int tkid, s64 cs_tick_adj) { + tk_ntp_data[tkid].cs_tick_adj = cs_tick_adj; __ntp_clear(&tk_ntp_data[tkid]); } diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h index 7084d839c207..598e5dd2fc5b 100644 --- a/kernel/time/ntp_internal.h +++ b/kernel/time/ntp_internal.h @@ -3,7 +3,7 @@ #define _LINUX_NTP_INTERNAL_H extern void ntp_init(void); -extern void ntp_clear(unsigned int tkid); +extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj); /* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */ extern u64 ntp_tick_length(unsigned int tkid); extern ktime_t ntp_get_next_leap(unsigned int tkid); diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index d847bba0481b..53961a1fcf47 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -339,7 +339,6 @@ static inline void clocksource_enable_inline_read(void) { } static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) { u64 interval; - u64 tmp, ntpinterval; struct clocksource *old_clock; ++tk->cs_was_changed_seq; @@ -353,20 +352,16 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last; /* Do the ns -> cycle conversion first, using original mult */ - tmp = NTP_INTERVAL_LENGTH; - tmp <<= clock->shift; - ntpinterval = tmp; - tmp += clock->mult/2; - do_div(tmp, clock->mult); - if (tmp == 0) - tmp = 1; - - interval = (u64) tmp; + interval = NTP_INTERVAL_LENGTH << clock->shift; + interval += clock->mult/2; + do_div(interval, clock->mult); + if (interval == 0) + interval = 1; + tk->cycle_interval = interval; /* Go back from cycles -> shifted ns */ tk->xtime_interval = interval * clock->mult; - tk->xtime_remainder = ntpinterval - tk->xtime_interval; tk->raw_interval = interval * clock->mult; /* if changing clocks, convert xtime_nsec shift units */ @@ -386,7 +381,38 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) tk->ntp_error = 0; tk->ntp_error_shift = NTP_SCALE_SHIFT - clock->shift; - tk->ntp_tick = ntpinterval << tk->ntp_error_shift; + + /* + * ntp_tick is the tick length that NTP disciplines (its ±500 PPM + * scales only this part), in NTP-shifted ns: the real interval of + * a whole number of counter cycles. Because cycle_interval is + * rounded to an integer number of cycles, this ntp_tick differs + * from the true intended 1/HZ tick length by up to half a cycle + * period. + */ + tk->ntp_tick = (u64)tk->xtime_interval << tk->ntp_error_shift; + + /* + * cs_tick_adj is the constant difference between the disciplined + * ntp_tick above and the true 1/HZ tick, expressed per-second to + * match the ntp_update_frequency() addends and handed to NTP via + * ntp_clear() to be explicitly included in its tick_length. + * + * Worked example: HZ=1000, ACPI PM timer at 3.579545 MHz, which + * has 3579.545 cycles in 1ms, rounded to cycle_interval = 3580. + * + * So ntp_tick is actually 1.000127ms, as that is the amount of + * time that 3580 cycles will take at the nominal frequency. This + * is the part that NTP disciplines, causing each 3580 counts to + * advance the clock by up to NTP's ±500PPM of that amount. + * + * The "extra" 127ns/tick is what's stored in cs_tick_adj and + * applied as a constant correction by ntp_update_frequency() so + * that NTP *believes* it's disciplining a 1ms tick. + */ + tk->cs_tick_adj = (s64)tk->ntp_tick - + ((s64)NTP_INTERVAL_LENGTH << NTP_SCALE_SHIFT); + tk->cs_tick_adj *= NTP_INTERVAL_FREQ; /* * The timekeeper keeps its own mult values for the currently @@ -803,7 +829,7 @@ static void timekeeping_update_from_shadow(struct tk_data *tkd, unsigned int act if (action & TK_CLEAR_NTP) { tk->ntp_error = 0; - ntp_clear(tk->id); + ntp_clear(tk->id, tk->cs_tick_adj); } tk_update_leap_state(tk); @@ -2075,7 +2101,12 @@ void __init timekeeping_init(void) tk_set_wall_to_mono(tks, wall_to_mono); - timekeeping_update_from_shadow(&tk_core, TK_CLOCK_WAS_SET); + /* + * Use TK_UPDATE_ALL so the NTP layer picks up the clocksource's + * cs_tick_adj via ntp_clear(). Clearing NTP here is otherwise + * redundant as ntp_init() already initialised it above. + */ + timekeeping_update_from_shadow(&tk_core, TK_UPDATE_ALL); } /* time in seconds when suspend began for persistent clock */ @@ -2424,8 +2455,8 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset) mult = tk->tkr_mono.mult - tk->ntp_err_mult; } else { tk->ntp_tick = ntp_tl; - mult = div64_u64((tk->ntp_tick >> tk->ntp_error_shift) - - tk->xtime_remainder, tk->cycle_interval); + mult = div64_u64(tk->ntp_tick >> tk->ntp_error_shift, + tk->cycle_interval); } /* @@ -2550,8 +2581,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, /* Accumulate error between NTP and clock interval */ tk->ntp_error += tk->ntp_tick << shift; - tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) << - (tk->ntp_error_shift + shift); + tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift); return offset; } -- 2.54.0