Re: [PATCH] tick/nohz: Avoid unused timekeeping_max_deferment() calls

[Frederic Weisbecker <frederic@kernel.org>]

Le Thu, Sep 17, 2026 at 09:39:29AM -0700, Usama Arif a écrit :
> tick_nohz_next_event() limits a CPU's sleep interval to the maximum
> deferment supported by the current clocksource when that CPU owns the
> do_timer() duty. If the duty is unassigned, the limit also applies when
> the CPU's TS_FLAG_DO_TIMER_LAST flag is set.
> 
> After the early timer checks, the function currently reads the maximum
> deferment unconditionally. It then replaces the result with KTIME_MAX
> unless one of the two conditions above applies.
> 
> timekeeping_max_deferment() performs a seqcount-protected read of the
> shared timekeeper and follows its clocksource pointer. Check the do_timer
> state first and avoid this work when the result would be discarded. This
> leaves the resulting expiry unchanged and reduces accesses to timekeeper
> data that is modified regularly.
> 
> On x86-64 this removes 18-20 dynamically executed instructions, including
> the call, from the common non-owner path when the seqcount does not retry.
> 
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  kernel/time/tick-sched.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index c8f2c4a503b08..e7c7c28311156 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -872,18 +872,25 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
>  	 * If this CPU is the one which had the do_timer() duty last, we limit
>  	 * the sleep time to the timekeeping 'max_deferment' value.
>  	 * Otherwise we can sleep as long as we want.
> +	 *
> +	 * Only read the max deferment in the former case: it is a seqcount
> +	 * read of the globally shared timekeeper, and on a large machine
> +	 * almost every caller is not the do_timer() CPU and would throw the
> +	 * value away.

I don't think that needs a comment.

>  	 */
> -	delta = timekeeping_max_deferment();
>  	tick_cpu = READ_ONCE(tick_do_timer_cpu);
>  	if (tick_cpu != cpu &&
> -	    (tick_cpu != TICK_DO_TIMER_NONE || !tick_sched_flag_test(ts, TS_FLAG_DO_TIMER_LAST)))
> -		delta = KTIME_MAX;
> -
> -	/* Calculate the next expiry time */
> -	if (delta < (KTIME_MAX - basemono))
> -		expires = basemono + delta;
> -	else
> +	    (tick_cpu != TICK_DO_TIMER_NONE || !tick_sched_flag_test(ts, TS_FLAG_DO_TIMER_LAST))) {
>  		expires = KTIME_MAX;
> +	} else {
> +		delta = timekeeping_max_deferment();
> +
> +		/* Calculate the next expiry time */
> +		if (delta < (KTIME_MAX - basemono))
> +			expires = basemono + delta;
> +		else
> +			expires = KTIME_MAX;
> +	}

If anything, it also makes the code more readable. You can even remove the delta
variable because even in its former use in the function, it's an unecessary
middle step.

Thanks.

>  
>  	ts->timer_expires = min_t(u64, expires, next_tick);
>  
> -- 
> 2.53.0-Meta
> 

-- 
Frederic Weisbecker
SUSE Labs