Re: [PATCH v2] sched/cputime: Don't account idle time twice after dyntick-idle

[Frederic Weisbecker <frederic@kernel.org>]

Le Mon, Oct 05, 2026 at 09:40:10PM +0200, Stian Halseth a écrit :
> On idle exit the dyntick-idle accounting accounts the time up to now,
> then the tick is restarted on its old period. The first tick accounts a
> whole TICK_NSEC to whatever runs, although the part of that period
> before the idle exit has just been accounted as idle time, or with
> IRQ_TIME_ACCOUNTING as IRQ time. That is up to a full tick per idle
> exit, and /proc/stat reports more idle time than wall time.
> 
> Record how much of the current tick period dyntick-idle has accounted,
> and leave it out of the tick that ends the period.
> 
> Fixes: cf6444c3e1bb7 ("tick/sched: Unify idle cputime accounting")
> Link: https://lore.kernel.org/all/20261004142724.3896396-1-stian@itx.no/
> Signed-off-by: Stian Halseth <stian@itx.no>

Looks good, just a few nits:

> @@ -468,18 +468,34 @@ static void kcpustat_idle_start(struct kernel_cpustat *kc, u64 now)
>  	write_seqcount_end(&kc->idle_sleeptime_seq);
>  }
>  
> -void kcpustat_dyntick_stop(u64 now)
> +void kcpustat_dyntick_stop(u64 now, u64 tick_start)
>  {
>  	struct kernel_cpustat *kc = kcpustat_this_cpu;
>  
>  	if (!vtime_generic_enabled_this_cpu()) {
>  		WARN_ON_ONCE(!kc->idle_dyntick);
> +		if (kc->idle_tick_period != tick_start) {
> +			kc->idle_tick_period = tick_start;
> +			kc->idle_tick_overlap = 0;
> +		}
> +		tick_start = max(tick_start, kc->idle_dyntick_entry);
> +		if (now > tick_start)

We probably should warn if now < tick_start.

> +			kc->idle_tick_overlap += now - tick_start;
>  		kcpustat_idle_stop(kc, now);
>  		kc->idle_dyntick = false;
>  		vtime_dyntick_stop();
>  	}
>  }
>  
> +/*
> + * The first tick after dyntick-idle covers a period that the dyntick-idle
> + * accounting may already have accounted in part.
> + */
> +static inline u64 kcpustat_tick_overlap(void)
> +{
> +	return __this_cpu_xchg(kernel_cpustat.idle_tick_overlap, 0);

We don't need that to be an atomic xchg. It can be a plain swap.

> +}
> +
>  void kcpustat_dyntick_start(u64 now)
>  {
>  	struct kernel_cpustat *kc = kcpustat_this_cpu;
> @@ -487,6 +503,7 @@ void kcpustat_dyntick_start(u64 now)
>  	if (!vtime_generic_enabled_this_cpu()) {
>  		vtime_dyntick_start();
>  		kc->idle_dyntick = true;
> +		kc->idle_dyntick_entry = now;
>  		kcpustat_idle_start(kc, now);
>  	}
>  }
> @@ -555,6 +572,11 @@ u64 kcpustat_field_iowait(int cpu)
>  }
>  EXPORT_SYMBOL_GPL(kcpustat_field_iowait);
>  #else
> +static inline u64 kcpustat_tick_overlap(void)
> +{
> +	return 0;
> +}
> +
>  static u64 kcpustat_field_dyntick(int cpu, enum cpu_usage_stat idx,
>  				  bool compute_delta, ktime_t now)
>  {
> @@ -695,12 +717,13 @@ void account_process_tick(struct task_struct *p, int user_tick)
>  	if (kcpustat_idle_dyntick())
>  		return;
>  
> +	cputime = TICK_NSEC - kcpustat_tick_overlap();
> +
>  	if (irqtime_enabled()) {
> -		irqtime_account_process_tick(p, user_tick, 1);
> +		irqtime_account_process_tick(p, user_tick, cputime);
>  		return;
>  	}
>  
> -	cputime = TICK_NSEC;
>  	steal = steal_account_process_time(ULONG_MAX);
>  
>  	if (steal >= cputime)
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 6c3fea3867139..1c5cefbb10812 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -763,13 +763,20 @@ static ktime_t tick_forward_now(ktime_t expires, ktime_t now)
>  	return expires + TICK_NSEC;
>  }
>  
> -static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
> +static ktime_t tick_nohz_restart_expires(struct tick_sched *ts, ktime_t now)
>  {
>  	ktime_t expires = ts->last_tick;
>  
>  	if (now >= expires)
>  		expires = tick_forward_now(expires, now);
>  
> +	return expires;

This should always update ts->last_tick so that we don't need to do
the division twice on dyntick stop.

> +}
> +
> +static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
> +{
> +	ktime_t expires = tick_nohz_restart_expires(ts, now);
> +
>  	if (tick_sched_flag_test(ts, TS_FLAG_HIGHRES)) {
>  		hrtimer_start(&ts->sched_timer,	expires, HRTIMER_MODE_ABS_PINNED_HARD);
>  	} else {
> @@ -1329,6 +1336,16 @@ unsigned long tick_nohz_get_idle_calls_cpu(int cpu)
>  	return ts->idle_calls;
>  }
>  
> +static void tick_nohz_dyntick_stop(struct tick_sched *ts, ktime_t now)
> +{
> +	ktime_t tick_start = now;
> +
> +	if (!tick_nohz_full_cpu(smp_processor_id()))

Just don't check tick_nohz_full_cpu(), it will fall into the
vtime_generic_enabled_this_cpu() anyway.

Thanks!


> +		tick_start = tick_nohz_restart_expires(ts, now) - TICK_NSEC;
> +
> +	kcpustat_dyntick_stop(now, tick_start);
> +}


-- 
Frederic Weisbecker
SUSE Labs