Re: [PATCH 14/15] sched/cputime: Handle idle irqtime gracefully

[Frederic Weisbecker <frederic@kernel.org>]

Le Tue, Mar 03, 2026 at 04:41:18PM +0530, Shrikanth Hegde a écrit :
> Hi Frederic,
> 
> On 2/6/26 7:52 PM, Frederic Weisbecker wrote:
> > The dyntick-idle cputime accounting always assumes that IRQ time
> > accounting is enabled and consequently stops elapsing the idle time
> > during dyntick-idle IRQs.
> > 
> > This doesn't mix up well with disabled IRQ time accounting because then
> > idle IRQs become a cputime blind-spot. Also this feature is disabled
> > on most configurations and the overhead of pausing dyntick-idle
> > accounting while in idle IRQs could then be avoided.
> > 
> > Fix the situation with conditionally pausing dyntick-idle accounting
> > during idle IRQs only if neither native vtime (which does IRQ time
> > accounting) nor generic IRQ time accounting are enabled.
> > 
> > Also make sure that the accumulated IRQ time is not accidentally
> > substracted from later accounting.
> > 
> > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > ---
> >   kernel/sched/cputime.c | 24 +++++++++++++++++++++---
> >   kernel/sched/sched.h   |  1 +
> >   2 files changed, 22 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> > index d588a4a50e57..92fa2f037b6e 100644
> > --- a/kernel/sched/cputime.c
> > +++ b/kernel/sched/cputime.c
> > @@ -46,7 +46,8 @@ static void irqtime_account_delta(struct irqtime *irqtime, u64 delta,
> >   	u64_stats_update_begin(&irqtime->sync);
> >   	cpustat[idx] += delta;
> >   	irqtime->total += delta;
> > -	irqtime->tick_delta += delta;
> > +	if (!irqtime->idle_dyntick)
> > +		irqtime->tick_delta += delta;
> 
> Wouldn't kcpustat_idle_dyntick achieve the same thing?

Yes indeed.

> 
> >   	u64_stats_update_end(&irqtime->sync);
> >   }
> > @@ -81,6 +82,16 @@ void irqtime_account_irq(struct task_struct *curr, unsigned int offset)
> >   		irqtime_account_delta(irqtime, delta, CPUTIME_SOFTIRQ);
> >   }
> > +static inline void irqtime_dyntick_start(void)
> > +{
> > +	__this_cpu_write(cpu_irqtime.idle_dyntick, true);
> > +}
> > +
> > +static inline void irqtime_dyntick_stop(void)
> > +{
> > +	__this_cpu_write(cpu_irqtime.idle_dyntick, false);
> > +}
> > +
> >   static u64 irqtime_tick_accounted(u64 maxtime)
> >   {
> >   	struct irqtime *irqtime = this_cpu_ptr(&cpu_irqtime);
> > @@ -94,6 +105,9 @@ static u64 irqtime_tick_accounted(u64 maxtime)
> >   #else /* !CONFIG_IRQ_TIME_ACCOUNTING: */
> > +static inline void irqtime_dyntick_start(void) { }
> > +static inline void irqtime_dyntick_stop(void) { }
> > +
> >   static u64 irqtime_tick_accounted(u64 dummy)
> >   {
> >   	return 0;
> > @@ -444,6 +458,7 @@ void kcpustat_dyntick_stop(u64 now)
> >   		WARN_ON_ONCE(!kc->idle_dyntick);
> >   		kcpustat_idle_stop(kc, now);
> >   		kc->idle_dyntick = false;
> > +		irqtime_dyntick_stop();
> >   		vtime_dyntick_stop();
> >   		steal_account_process_time(ULONG_MAX);
> >   	}
> > @@ -455,6 +470,7 @@ void kcpustat_dyntick_start(u64 now)
> >   	if (!vtime_generic_enabled_this_cpu()) {
> >   		vtime_dyntick_start();
> > +		irqtime_dyntick_start();
> >   		kc->idle_dyntick = true;
> >   		kcpustat_idle_start(kc, now);
> >   	}
> > @@ -464,7 +480,8 @@ void kcpustat_irq_enter(u64 now)
> >   {
> >   	struct kernel_cpustat *kc = kcpustat_this_cpu;
> > -	if (!vtime_generic_enabled_this_cpu())
> > +	if (!vtime_generic_enabled_this_cpu() &&
> > +	    (irqtime_enabled() || vtime_accounting_enabled_this_cpu()))
> >   		kcpustat_idle_stop(kc, now);
> >   }
> Scenario: context_tracking is not enabled(since nohz_full or isolcpus not specified) and
> irqtime/native is not enabled. ( config is CONFIG_VIRT_CPU_ACCOUNTING_GEN + IRQ_TIME=n)
> 
> 
> cpu goes into tickless mode. Gets irqs, but kcpustat_irq_enter/exit is nop.
> Then the time it spent in irq is still accounted for idle time, during
> kcpustat_dyntick_stop?

Right! As is the case for IRQs firing in system and user time. Basically this
just consolidate the IRQ time accounting behaviour in CONFIG_VIRT_CPU_ACCOUNTING_GEN=n

> Who is going to account the irq time in this case?

Nothing, it's part of idle time.

We could also decide to account the idle IRQ time as system time. I guess it's a
matter of which semantic we want to give. Though that would be more overhead.

Thanks.

-- 
Frederic Weisbecker
SUSE Labs