Re: [PATCH 1/3] vdso/math64: Add and use __iter_div64_u64_rem()

[Thomas Weißschuh]

On Mon, Aug 31, 2026 at 08:55:55PM +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.
> 
> Add __iter_div64_u64_rem(), the u64-divisor counterpart of
> __iter_div_u64_rem(), keeping the asm() barrier that stops the compiler
> turning the loop into a division, and use it for both.
> 
> No functional change.
> 
> Signed-off-by: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
> ---
>  include/vdso/math64.h  | 21 +++++++++++++++++++++
>  kernel/time/vsyscall.c | 17 ++++++-----------
>  2 files changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/include/vdso/math64.h b/include/vdso/math64.h
> index 22ae212f8b28..eb39d3411981 100644
> --- a/include/vdso/math64.h
> +++ b/include/vdso/math64.h
> @@ -21,6 +21,27 @@ __iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)
>  	return ret;
>  }
>  
> +static __always_inline u64
> +__iter_div64_u64_rem(u64 dividend, u64 divisor, u64 *remainder)
> +{
> +	u64 ret = 0;
> +
> +	while (dividend >= divisor) {
> +		/*
> +		 * Prevent the compiler from optimising this loop into a
> +		 * modulo operation.
> +		 */
> +		asm("" : "+rm"(dividend));

This could probably be OPTIMIZER_HIDE_VAR(), but for consistency with 
__iter_div_u64_rem() it should probably stay like it is.

> +
> +		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..165ad9d7f154 100644
> --- a/kernel/time/vsyscall.c
> +++ b/kernel/time/vsyscall.c
> @@ -30,21 +30,21 @@ static inline void update_vdso_time_data(struct vdso_time_data *vdata, struct ti
>  {
>  	struct vdso_clock *vc = vdata->clock_data;
>  	struct vdso_timestamp *vdso_ts;
> -	u64 nsec, sec;
> +	u64 nsec_per_sec, nsec, sec;
>  
>  	fill_clock_configuration(&vc[CS_HRES_COARSE],	&tk->tkr_mono);
>  	fill_clock_configuration(&vc[CS_RAW],		&tk->tkr_raw);
>  
> +	/* One second in the scaled nanoseconds of tkr_mono */
> +	nsec_per_sec = (u64)NSEC_PER_SEC << tk->tkr_mono.shift;

I am not a fan of the additional variables introduced in this patch.
Both the patch itself and the final code are harder to understand with
them in my opinion.
If you want to keep them, they should be introduced in a dedicated patch.

> +
>  	/* CLOCK_MONOTONIC */
>  	vdso_ts		= &vc[CS_HRES_COARSE].basetime[CLOCK_MONOTONIC];
>  	vdso_ts->sec	= tk->xtime_sec + tk->wall_to_monotonic.tv_sec;
>  
>  	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->sec	+= __iter_div64_u64_rem(nsec, nsec_per_sec, &nsec);

This could directly store the result in vdso_ts->nsec if it produces better code.

>  	vdso_ts->nsec	= nsec;
>  
>  	/* Copy MONOTONIC time for BOOTTIME */
> @@ -55,12 +55,7 @@ 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;
 
I would leave this on its own line for the comment about the copy for BOOTTIME
to be easier to understand.

> -
> -	while (nsec >= (((u64)NSEC_PER_SEC) << tk->tkr_mono.shift)) {
> -		nsec -= (((u64)NSEC_PER_SEC) << tk->tkr_mono.shift);
> -		vdso_ts->sec++;
> -	}
> +	vdso_ts->sec	= sec + __iter_div64_u64_rem(nsec, nsec_per_sec, &nsec);
>  	vdso_ts->nsec	= nsec;
>  
>  	/* CLOCK_MONOTONIC_RAW */
> -- 
> 2.43.0
>