mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/3] vdso: Keep the CLOCK_AUX base at full precision
@ 2026-09-01  2:06 Zhan Xusheng
  2026-09-01  2:06 ` [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem() Zhan Xusheng
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Zhan Xusheng @ 2026-09-01  2:06 UTC (permalink / raw)
  To: tglx, luto, vincenzo.frascino
  Cc: thomas.weissschuh, david.laight.linux, linux-kernel, zhanxusheng

v1: https://lore.kernel.org/all/20260831125557.1490398-1-zhanxusheng@xiaomi.com
v2 went out before David's mail arrived.  It touched only the call sites,
so both of his remarks about the helper applied to it unchanged.

Changes in v3:

 - The quotient is a u32, as David suggested and as the u32-divisor
   version already returns.  vsyscall.o loses 16 bytes of text on x86-64
   and the loop drops from 36 to 28 instructions on 32-bit gcc.
 - The barrier keeps the value in a register instead of offering a memory
   alternative.  David asked whether clang makes a mess of "+rm": it does,
   it takes the memory alternative and spills inside the loop, on 64-bit
   as well.  Both forms still stop the loop becoming a division, checked
   for gcc and clang at both widths.
 - Fixes: tag on 2/3.

Changes in v2, from Thomas' review of 1/3:

 - Drop the nsec_per_sec local and spell the divisor out at each site.
 - Store the remainder straight into the basetime, as the coarse clocks
   already do.  That also makes the copy of the CLOCK_MONOTONIC values for
   CLOCK_BOOTTIME take sec and nsec from the same place rather than one
   from the basetime and one from a local left over from the division.
   Not a codegen win: gcc emits the same instructions either way.
 - Keep "vdso_ts->sec = sec;" on its own line under the copy comment.
 - Picked up the Reviewed-by on 3/3, which is unchanged.

__iter_div_u64_rem() has the same "+rm" barrier, and it sits on the
clock_gettime() path through vdso_set_timespec().  Converting it is worth
64 bytes of vdso64 text and 112 of vdso32 built with clang 18; gcc is
indifferent.  It is not in this series because it also has users in
arch/x86/kvm, timespec64_add_ns() and lib/math/div64.c.  Say the word and
I will send it.

The problem 2/3 fixes: the CLOCK_AUX basetime is shifted down to
nanoseconds and back up, which drops the fractional nanoseconds of
xtime_nsec, so the vDSO floors the base and the cycle delta separately
where ktime_get_aux() floors their sum.  The vDSO reading ends up 0 or
1 ns below the syscall for the same clock, and clock_getres() advertises
1 ns for these clocks.

Checked under QEMU with an auxiliary clock enabled through
/sys/kernel/time/aux_clocks/0/aux_clock_enable, comparing CLOCK_AUX via
the vDSO against the raw syscall at offset 0, at +5.123456789 s, and with
offs_aux driven negative.  The two agree within read latency in all three,
and over 100000 interleaved pairs each the vDSO reading is never ahead of a
syscall reading taken after it.  The 1 ns bias itself is not measurable
that way, a pair of reads costing several hundred nanoseconds.  The
argument for it is the algebra in 2/3, plus a sweep over
(xtime_nsec, delta) which puts the 1 ns case at 60% for shift 24.

Zhan Xusheng (3):
  vdso/math64: Add and use __iter_div64_u64_rem()
  vdso/vsyscall: Keep the CLOCK_AUX base scaled
  vdso/gettimeofday: Assert that the clock id fits the dispatch mask

 include/vdso/math64.h   | 22 ++++++++++++++++++++++
 kernel/time/vsyscall.c  | 28 ++++++++++++----------------
 lib/vdso/gettimeofday.c |  2 ++
 3 files changed, 36 insertions(+), 16 deletions(-)


base-commit: abdf623ddb75b24659018d3952d8f61937306ae5
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem()
  2026-09-01  2:06 [PATCH v3 0/3] vdso: Keep the CLOCK_AUX base at full precision Zhan Xusheng
@ 2026-09-01  2:06 ` Zhan Xusheng
  2026-09-01  8:35   ` Thomas Weißschuh
  2026-09-01  2:06 ` [PATCH v3 2/3] vdso/vsyscall: Keep the CLOCK_AUX base scaled Zhan Xusheng
  2026-09-01  2:06 ` [PATCH v3 3/3] vdso/gettimeofday: Assert that the clock id fits the dispatch mask Zhan Xusheng
  2 siblings, 1 reply; 8+ messages in thread
From: Zhan Xusheng @ 2026-09-01  2:06 UTC (permalink / raw)
  To: tglx, luto, vincenzo.frascino
  Cc: thomas.weissschuh, david.laight.linux, linux-kernel, zhanxusheng

The vDSO basetimes for CLOCK_MONOTONIC and CLOCK_BOOTTIME are kept in the
scaled nanoseconds of tkr_mono, so normalising them means dividing by
NSEC_PER_SEC << shift, which does not fit the u32 divisor of
__iter_div_u64_rem().

update_vdso_time_data() therefore open-codes the iterative division twice.
Turning the loops into a plain modulo is not an option either, as the vDSO
has no 64-bit division helpers on 32-bit.

Add __iter_div64_u64_rem(), the u64-divisor counterpart of
__iter_div_u64_rem(), and use it for both.  The remainder goes straight
into the basetime as the coarse clocks already do, which also makes the
copy of the CLOCK_MONOTONIC values for CLOCK_BOOTTIME take both of them
from the same place.

The quotient is a u32 like the u32-divisor version returns.  A loop that
subtracts only makes sense when it iterates a handful of times, and it
keeps 32-bit from carrying the counter in a register pair: vsyscall.o
loses 16 bytes of text on x86-64 and the loop drops from 36 to 28
instructions on 32-bit gcc.

The barrier keeps the value in a register rather than offering a memory
alternative like __iter_div_u64_rem() does.  Both forms stop the loop
becoming a division, but clang takes the memory alternative and spills
inside the loop, on 64-bit as well.  Converting __iter_div_u64_rem() the
same way is worth 64 bytes of vdso64 text and 112 of vdso32 built with
clang 18, since it sits on the clock_gettime() path through
vdso_set_timespec(); that is left for a separate patch as it also has
users outside the vDSO.

No functional change.

Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 include/vdso/math64.h  | 22 ++++++++++++++++++++++
 kernel/time/vsyscall.c | 18 +++++++-----------
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/include/vdso/math64.h b/include/vdso/math64.h
index 22ae212f8b28..02abdf6e82ed 100644
--- a/include/vdso/math64.h
+++ b/include/vdso/math64.h
@@ -21,6 +21,28 @@ __iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)
 	return ret;
 }
 
+static __always_inline u32
+__iter_div64_u64_rem(u64 dividend, u64 divisor, u64 *remainder)
+{
+	u32 ret = 0;
+
+	while (dividend >= divisor) {
+		/*
+		 * Prevent the compiler from optimising this loop into a
+		 * modulo operation.  Keep the value in a register, as clang
+		 * spills it when offered a memory alternative.
+		 */
+		asm("" : "=r"(dividend) : "0"(dividend));
+
+		dividend -= divisor;
+		ret++;
+	}
+
+	*remainder = dividend;
+
+	return ret;
+}
+
 #if defined(CONFIG_ARCH_SUPPORTS_INT128) && defined(__SIZEOF_INT128__)
 
 #ifndef mul_u64_u32_add_u64_shr
diff --git a/kernel/time/vsyscall.c b/kernel/time/vsyscall.c
index aa59919b8f2c..993258d5f1e7 100644
--- a/kernel/time/vsyscall.c
+++ b/kernel/time/vsyscall.c
@@ -41,14 +41,13 @@ static inline void update_vdso_time_data(struct vdso_time_data *vdata, struct ti
 
 	nsec = tk->tkr_mono.xtime_nsec;
 	nsec += ((u64)tk->wall_to_monotonic.tv_nsec << tk->tkr_mono.shift);
-	while (nsec >= (((u64)NSEC_PER_SEC) << tk->tkr_mono.shift)) {
-		nsec -= (((u64)NSEC_PER_SEC) << tk->tkr_mono.shift);
-		vdso_ts->sec++;
-	}
-	vdso_ts->nsec	= nsec;
+	vdso_ts->sec	+= __iter_div64_u64_rem(nsec,
+				(u64)NSEC_PER_SEC << tk->tkr_mono.shift,
+				&vdso_ts->nsec);
 
 	/* Copy MONOTONIC time for BOOTTIME */
 	sec	= vdso_ts->sec;
+	nsec	= vdso_ts->nsec;
 	/* Add the boot offset */
 	sec	+= tk->monotonic_to_boot.tv_sec;
 	nsec	+= (u64)tk->monotonic_to_boot.tv_nsec << tk->tkr_mono.shift;
@@ -56,12 +55,9 @@ static inline void update_vdso_time_data(struct vdso_time_data *vdata, struct ti
 	/* CLOCK_BOOTTIME */
 	vdso_ts		= &vc[CS_HRES_COARSE].basetime[CLOCK_BOOTTIME];
 	vdso_ts->sec	= sec;
-
-	while (nsec >= (((u64)NSEC_PER_SEC) << tk->tkr_mono.shift)) {
-		nsec -= (((u64)NSEC_PER_SEC) << tk->tkr_mono.shift);
-		vdso_ts->sec++;
-	}
-	vdso_ts->nsec	= nsec;
+	vdso_ts->sec	+= __iter_div64_u64_rem(nsec,
+				(u64)NSEC_PER_SEC << tk->tkr_mono.shift,
+				&vdso_ts->nsec);
 
 	/* CLOCK_MONOTONIC_RAW */
 	vdso_ts		= &vc[CS_RAW].basetime[CLOCK_MONOTONIC_RAW];
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] vdso/vsyscall: Keep the CLOCK_AUX base scaled
  2026-09-01  2:06 [PATCH v3 0/3] vdso: Keep the CLOCK_AUX base at full precision Zhan Xusheng
  2026-09-01  2:06 ` [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem() Zhan Xusheng
@ 2026-09-01  2:06 ` Zhan Xusheng
  2026-09-01  8:42   ` Thomas Weißschuh
  2026-09-01  2:06 ` [PATCH v3 3/3] vdso/gettimeofday: Assert that the clock id fits the dispatch mask Zhan Xusheng
  2 siblings, 1 reply; 8+ messages in thread
From: Zhan Xusheng @ 2026-09-01  2:06 UTC (permalink / raw)
  To: tglx, luto, vincenzo.frascino
  Cc: thomas.weissschuh, david.laight.linux, linux-kernel, zhanxusheng

The vDSO basetime of a clock is stored in the scaled nanoseconds of
tkr_mono, so that the reader can floor the base and the cycle delta
together in vdso_calc_ns().

vdso_time_update_aux() instead shifts the base down to nanoseconds, adds
the offset, and shifts it back up, which zeroes the fractional nanoseconds
of xtime_nsec.  The reader then floors the base and the delta separately:

  ktime_get_aux():  base + ((delta * mult + xtime_nsec) >> shift)
  vdso:             base + (xtime_nsec >> shift)
                         + ((delta * mult) >> shift)

Since floor(a) + floor(b) <= floor(a + b), the vDSO reports 0 or 1 ns
below the syscall for the same clock.  clock_getres() reports 1 ns for
auxiliary clocks, so that is the full advertised granularity.  It is not a
monotonicity problem: across an update the step is
floor(a + d) - floor(a) - floor(d), which is 0 or 1, never negative.

Add the offset in scaled nanoseconds as the other high resolution clocks
do, and normalise with __iter_div64_u64_rem() so that the stored base
stays below one second and the userspace fast-path does not iterate more
in __iter_div_u64_rem().

monotonic_to_aux.tv_nsec is a normalised timespec64 fraction, so it stays
below NSEC_PER_SEC even for a negative offset, and the sum stays below
2 * (NSEC_PER_SEC << shift).  The largest shift clocks_calc_mult_shift()
can pick is 32, which makes that 8.6e18 against a u64 limit of 1.8e19.

Fixes: 380b84e168e5 ("vdso/vsyscall: Update auxiliary clock data in the datapage")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/time/vsyscall.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/time/vsyscall.c b/kernel/time/vsyscall.c
index 993258d5f1e7..697adc44eebf 100644
--- a/kernel/time/vsyscall.c
+++ b/kernel/time/vsyscall.c
@@ -157,11 +157,11 @@ void vdso_time_update_aux(struct timekeeper *tk)
 
 		vdso_ts->sec = tk->xtime_sec + tk->monotonic_to_aux.tv_sec;
 
-		nsec = tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift;
-		nsec += tk->monotonic_to_aux.tv_nsec;
-		vdso_ts->sec += __iter_div_u64_rem(nsec, NSEC_PER_SEC, &nsec);
-		nsec = nsec << tk->tkr_mono.shift;
-		vdso_ts->nsec = nsec;
+		nsec = tk->tkr_mono.xtime_nsec;
+		nsec += (u64)tk->monotonic_to_aux.tv_nsec << tk->tkr_mono.shift;
+		vdso_ts->sec += __iter_div64_u64_rem(nsec,
+					(u64)NSEC_PER_SEC << tk->tkr_mono.shift,
+					&vdso_ts->nsec);
 	}
 
 	__arch_update_vdso_clock(vc);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] vdso/gettimeofday: Assert that the clock id fits the dispatch mask
  2026-09-01  2:06 [PATCH v3 0/3] vdso: Keep the CLOCK_AUX base at full precision Zhan Xusheng
  2026-09-01  2:06 ` [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem() Zhan Xusheng
  2026-09-01  2:06 ` [PATCH v3 2/3] vdso/vsyscall: Keep the CLOCK_AUX base scaled Zhan Xusheng
@ 2026-09-01  2:06 ` Zhan Xusheng
  2 siblings, 0 replies; 8+ messages in thread
From: Zhan Xusheng @ 2026-09-01  2:06 UTC (permalink / raw)
  To: tglx, luto, vincenzo.frascino
  Cc: thomas.weissschuh, david.laight.linux, linux-kernel, zhanxusheng

The clock id dispatch turns the id into a bit in a u32:

  if (!vdso_clockid_valid(clock))
      return false;
  msk = 1U << clock;

vdso_clockid_valid() admits everything up to CLOCK_AUX_LAST, which is 23,
so the shift is in range.  Nothing states the dependency though, and
raising MAX_AUX_CLOCKS past 16 would take CLOCK_AUX_LAST to 32 or beyond
and make the shift undefined.

Assert it at both dispatch sites.  The condition is on a parameter rather
than a constant, so it relies on the compiler deriving the range from the
vdso_clockid_valid() bail-out above it.  gcc 13 and clang 18 both do: x86
vdso64 and vdso32 build clean, and raising MAX_AUX_CLOCKS to 17 fails the
assert as intended.

Suggested-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
No change since v1.

 lib/vdso/gettimeofday.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index f7a591aba59f..ef4dcc614489 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -285,6 +285,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 	 * Convert the clockid to a bitmask and use it to check which
 	 * clocks are handled in the VDSO directly.
 	 */
+	BUILD_BUG_ON(clock >= BITS_PER_TYPE(msk));
 	msk = 1U << clock;
 	if (likely(msk & VDSO_HRES))
 		vc = &vc[CS_HRES_COARSE];
@@ -438,6 +439,7 @@ bool __cvdso_clock_getres_common(const struct vdso_time_data *vd, clockid_t cloc
 	 * Convert the clockid to a bitmask and use it to check which
 	 * clocks are handled in the VDSO directly.
 	 */
+	BUILD_BUG_ON(clock >= BITS_PER_TYPE(msk));
 	msk = 1U << clock;
 	if (msk & (VDSO_HRES | VDSO_RAW)) {
 		/*
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem()
  2026-09-01  2:06 ` [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem() Zhan Xusheng
@ 2026-09-01  8:35   ` Thomas Weißschuh
  2026-09-01 12:48     ` David Laight
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Weißschuh @ 2026-09-01  8:35 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: tglx, luto, vincenzo.frascino, david.laight.linux, linux-kernel,
	zhanxusheng

Hi Zhan,

thanks for the new version. However please slow down a bit with sending new
revisions to give people time to respond.

On Tue, Sep 01, 2026 at 10:06:34AM +0800, Zhan Xusheng wrote:
> The vDSO basetimes for CLOCK_MONOTONIC and CLOCK_BOOTTIME are kept in the
> scaled nanoseconds of tkr_mono, so normalising them means dividing by
> NSEC_PER_SEC << shift, which does not fit the u32 divisor of
> __iter_div_u64_rem().
> 
> update_vdso_time_data() therefore open-codes the iterative division twice.
> Turning the loops into a plain modulo is not an option either, as the vDSO
> has no 64-bit division helpers on 32-bit.

This is not vDSO userspace code, but on the kernel side.
The problem is that divisions are unnecessarily slow.

Also a module alone would not be enough, as we need the division result.

> Add __iter_div64_u64_rem(), the u64-divisor counterpart of
> __iter_div_u64_rem(), and use it for both.  The remainder goes straight
> into the basetime as the coarse clocks already do, which also makes the
> copy of the CLOCK_MONOTONIC values for CLOCK_BOOTTIME take both of them
> from the same place.
> 
> The quotient is a u32 like the u32-divisor version returns.  A loop that
> subtracts only makes sense when it iterates a handful of times, and it
> keeps 32-bit from carrying the counter in a register pair: vsyscall.o
> loses 16 bytes of text on x86-64 and the loop drops from 36 to 28
> instructions on 32-bit gcc.
> 
> The barrier keeps the value in a register rather than offering a memory
> alternative like __iter_div_u64_rem() does.  Both forms stop the loop
> becoming a division, but clang takes the memory alternative and spills
> inside the loop, on 64-bit as well.  Converting __iter_div_u64_rem() the
> same way is worth 64 bytes of vdso64 text and 112 of vdso32 built with
> clang 18, since it sits on the clock_gettime() path through
> vdso_set_timespec(); that is left for a separate patch as it also has
> users outside the vDSO.
> 
> No functional change.
> 
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  include/vdso/math64.h  | 22 ++++++++++++++++++++++
>  kernel/time/vsyscall.c | 18 +++++++-----------
>  2 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/include/vdso/math64.h b/include/vdso/math64.h
> index 22ae212f8b28..02abdf6e82ed 100644
> --- a/include/vdso/math64.h
> +++ b/include/vdso/math64.h
> @@ -21,6 +21,28 @@ __iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)
>  	return ret;
>  }
>  
> +static __always_inline u32
> +__iter_div64_u64_rem(u64 dividend, u64 divisor, u64 *remainder)
> +{
> +	u32 ret = 0;
> +
> +	while (dividend >= divisor) {
> +		/*
> +		 * Prevent the compiler from optimising this loop into a
> +		 * modulo operation.  Keep the value in a register, as clang
> +		 * spills it when offered a memory alternative.
> +		 */
> +		asm("" : "=r"(dividend) : "0"(dividend));

This is now the same as OPTIMIZER_HIDE_VAR(). Let's use the standard macro.
In my opinion we can then also drop the comment about the memory alternative.

If this variant is proven to be the better one, we should also use it in
__iter_div_u64_rem(), in a new first patch.
Any unnecessary divergence will confuse future readers.

> +
> +		dividend -= divisor;
> +		ret++;
> +	}
> +
> +	*remainder = dividend;
> +
> +	return ret;
> +}
> +
>  #if defined(CONFIG_ARCH_SUPPORTS_INT128) && defined(__SIZEOF_INT128__)
>  
>  #ifndef mul_u64_u32_add_u64_shr
> diff --git a/kernel/time/vsyscall.c b/kernel/time/vsyscall.c
> index aa59919b8f2c..993258d5f1e7 100644
> --- a/kernel/time/vsyscall.c
> +++ b/kernel/time/vsyscall.c
> @@ -41,14 +41,13 @@ static inline void update_vdso_time_data(struct vdso_time_data *vdata, struct ti
>  
>  	nsec = tk->tkr_mono.xtime_nsec;
>  	nsec += ((u64)tk->wall_to_monotonic.tv_nsec << tk->tkr_mono.shift);
> -	while (nsec >= (((u64)NSEC_PER_SEC) << tk->tkr_mono.shift)) {
> -		nsec -= (((u64)NSEC_PER_SEC) << tk->tkr_mono.shift);
> -		vdso_ts->sec++;
> -	}
> -	vdso_ts->nsec	= nsec;
> +	vdso_ts->sec	+= __iter_div64_u64_rem(nsec,
> +				(u64)NSEC_PER_SEC << tk->tkr_mono.shift,
> +				&vdso_ts->nsec);

This alignment looks off. You have 100 characters width. Better:

	vdso_ts->sec	+= __iter_div64_u64_rem(nsec,
					        (u64)NSEC_PER_SEC << tk->tkr_mono.shift,
						&vdso_ts->nsec);

>  
>  	/* Copy MONOTONIC time for BOOTTIME */
>  	sec	= vdso_ts->sec;
> +	nsec	= vdso_ts->nsec;
>  	/* Add the boot offset */
>  	sec	+= tk->monotonic_to_boot.tv_sec;
>  	nsec	+= (u64)tk->monotonic_to_boot.tv_nsec << tk->tkr_mono.shift;

(...)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/3] vdso/vsyscall: Keep the CLOCK_AUX base scaled
  2026-09-01  2:06 ` [PATCH v3 2/3] vdso/vsyscall: Keep the CLOCK_AUX base scaled Zhan Xusheng
@ 2026-09-01  8:42   ` Thomas Weißschuh
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Weißschuh @ 2026-09-01  8:42 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: tglx, luto, vincenzo.frascino, david.laight.linux, linux-kernel,
	zhanxusheng

On Tue, Sep 01, 2026 at 10:06:35AM +0800, Zhan Xusheng wrote:
> The vDSO basetime of a clock is stored in the scaled nanoseconds of
> tkr_mono, so that the reader can floor the base and the cycle delta
> together in vdso_calc_ns().
> 
> vdso_time_update_aux() instead shifts the base down to nanoseconds, adds
> the offset, and shifts it back up, which zeroes the fractional nanoseconds
> of xtime_nsec.  The reader then floors the base and the delta separately:
> 
>   ktime_get_aux():  base + ((delta * mult + xtime_nsec) >> shift)
>   vdso:             base + (xtime_nsec >> shift)
>                          + ((delta * mult) >> shift)
> 
> Since floor(a) + floor(b) <= floor(a + b), the vDSO reports 0 or 1 ns
> below the syscall for the same clock.  clock_getres() reports 1 ns for
> auxiliary clocks, so that is the full advertised granularity.  It is not a
> monotonicity problem: across an update the step is
> floor(a + d) - floor(a) - floor(d), which is 0 or 1, never negative.

I am not so sure I understand the "so that is the full advertised granularity".
In my interpretation we are violating the expected granularity.
Not that anybody will be able to observe the issue.
I would just remove this sentence.

> Add the offset in scaled nanoseconds as the other high resolution clocks
> do, and normalise with __iter_div64_u64_rem() so that the stored base
> stays below one second and the userspace fast-path does not iterate more
> in __iter_div_u64_rem().
> 
> monotonic_to_aux.tv_nsec is a normalised timespec64 fraction, so it stays
> below NSEC_PER_SEC even for a negative offset, and the sum stays below
> 2 * (NSEC_PER_SEC << shift).  The largest shift clocks_calc_mult_shift()
> can pick is 32, which makes that 8.6e18 against a u64 limit of 1.8e19.
> 
> Fixes: 380b84e168e5 ("vdso/vsyscall: Update auxiliary clock data in the datapage")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  kernel/time/vsyscall.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/time/vsyscall.c b/kernel/time/vsyscall.c
> index 993258d5f1e7..697adc44eebf 100644
> --- a/kernel/time/vsyscall.c
> +++ b/kernel/time/vsyscall.c
> @@ -157,11 +157,11 @@ void vdso_time_update_aux(struct timekeeper *tk)
>  
>  		vdso_ts->sec = tk->xtime_sec + tk->monotonic_to_aux.tv_sec;
>  
> -		nsec = tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift;
> -		nsec += tk->monotonic_to_aux.tv_nsec;
> -		vdso_ts->sec += __iter_div_u64_rem(nsec, NSEC_PER_SEC, &nsec);
> -		nsec = nsec << tk->tkr_mono.shift;
> -		vdso_ts->nsec = nsec;
> +		nsec = tk->tkr_mono.xtime_nsec;
> +		nsec += (u64)tk->monotonic_to_aux.tv_nsec << tk->tkr_mono.shift;
> +		vdso_ts->sec += __iter_div64_u64_rem(nsec,
> +					(u64)NSEC_PER_SEC << tk->tkr_mono.shift,
> +					&vdso_ts->nsec);

The same alignment issue as the previous patch. With that fixed:

Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

>  	}
>  
>  	__arch_update_vdso_clock(vc);
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem()
  2026-09-01  8:35   ` Thomas Weißschuh
@ 2026-09-01 12:48     ` David Laight
  2026-09-01 13:05       ` Thomas Weißschuh
  0 siblings, 1 reply; 8+ messages in thread
From: David Laight @ 2026-09-01 12:48 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Zhan Xusheng, tglx, luto, vincenzo.frascino, linux-kernel, zhanxusheng

On Tue, 1 Sep 2026 10:35:30 +0200
Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote:

> Hi Zhan,
> 
> thanks for the new version. However please slow down a bit with sending new
> revisions to give people time to respond.
> 
> On Tue, Sep 01, 2026 at 10:06:34AM +0800, Zhan Xusheng wrote:
> > The vDSO basetimes for CLOCK_MONOTONIC and CLOCK_BOOTTIME are kept in the
> > scaled nanoseconds of tkr_mono, so normalising them means dividing by
> > NSEC_PER_SEC << shift, which does not fit the u32 divisor of
> > __iter_div_u64_rem().
> > 
> > update_vdso_time_data() therefore open-codes the iterative division twice.
> > Turning the loops into a plain modulo is not an option either, as the vDSO
> > has no 64-bit division helpers on 32-bit.  
> 
> This is not vDSO userspace code, but on the kernel side.
> The problem is that divisions are unnecessarily slow.

Until you get to (on x86) zen3 or cannon lake when they drop to ~15 clocks.
(I think someone finally allocated a bit of silicon to integer divide.)

> Also a module alone would not be enough, as we need the division result.

Isn't the divisor constant? (or rather a constant shifted left some).
In that case you can do a 'multiply by reciprocal'.
The simple 'multiply an shift right 32' will give slightly low quotient.
But you need to calculate the remainder - so can fixup the overlarge
remainder it can generate.

Of course, even that is only worthwhile if the quotient is more than
(a guess) 4.

David

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem()
  2026-09-01 12:48     ` David Laight
@ 2026-09-01 13:05       ` Thomas Weißschuh
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Weißschuh @ 2026-09-01 13:05 UTC (permalink / raw)
  To: David Laight
  Cc: Zhan Xusheng, tglx, luto, vincenzo.frascino, linux-kernel, zhanxusheng

On Tue, Sep 01, 2026 at 01:48:20PM +0100, David Laight wrote:
> On Tue, 1 Sep 2026 10:35:30 +0200
> Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote:
> > thanks for the new version. However please slow down a bit with sending new
> > revisions to give people time to respond.
> > 
> > On Tue, Sep 01, 2026 at 10:06:34AM +0800, Zhan Xusheng wrote:
> > > The vDSO basetimes for CLOCK_MONOTONIC and CLOCK_BOOTTIME are kept in the
> > > scaled nanoseconds of tkr_mono, so normalising them means dividing by
> > > NSEC_PER_SEC << shift, which does not fit the u32 divisor of
> > > __iter_div_u64_rem().
> > > 
> > > update_vdso_time_data() therefore open-codes the iterative division twice.
> > > Turning the loops into a plain modulo is not an option either, as the vDSO
> > > has no 64-bit division helpers on 32-bit.  
> > 
> > This is not vDSO userspace code, but on the kernel side.
> > The problem is that divisions are unnecessarily slow.
> 
> Until you get to (on x86) zen3 or cannon lake when they drop to ~15 clocks.
> (I think someone finally allocated a bit of silicon to integer divide.)

This code needs to work more or less everywhere, not only x86.

> > Also a module alone would not be enough, as we need the division result.
> 
> Isn't the divisor constant? (or rather a constant shifted left some).
> In that case you can do a 'multiply by reciprocal'.
> The simple 'multiply an shift right 32' will give slightly low quotient.
> But you need to calculate the remainder - so can fixup the overlarge
> remainder it can generate.

> Of course, even that is only worthwhile if the quotient is more than
> (a guess) 4.

It never is. As per patch 2:

monotonic_to_aux.tv_nsec is a normalised timespec64 fraction, so it stays
below NSEC_PER_SEC even for a negative offset, and the sum stays below
2 * (NSEC_PER_SEC << shift).

The same is true for the users in this patch.


Thomas

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-01 13:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-01  2:06 [PATCH v3 0/3] vdso: Keep the CLOCK_AUX base at full precision Zhan Xusheng
2026-09-01  2:06 ` [PATCH v3 1/3] vdso/math64: Add and use __iter_div64_u64_rem() Zhan Xusheng
2026-09-01  8:35   ` Thomas Weißschuh
2026-09-01 12:48     ` David Laight
2026-09-01 13:05       ` Thomas Weißschuh
2026-09-01  2:06 ` [PATCH v3 2/3] vdso/vsyscall: Keep the CLOCK_AUX base scaled Zhan Xusheng
2026-09-01  8:42   ` Thomas Weißschuh
2026-09-01  2:06 ` [PATCH v3 3/3] vdso/gettimeofday: Assert that the clock id fits the dispatch mask Zhan Xusheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®