Re: [PATCH v2 4/4] sched: Fix cgroup irq accounting for CONFIG_IRQ_TIME_ACCOUNTING

[Johannes Weiner <hannes@cmpxchg.org>]

On Tue, Oct 08, 2024 at 02:19:51PM +0800, Yafang Shao wrote:
> @@ -5587,7 +5587,24 @@ void sched_tick(void)
>  	rq_lock(rq, &rf);
>  
>  	curr = rq->curr;
> -	psi_account_irqtime(rq, curr, NULL);
> +
> +#ifdef CONFIG_IRQ_TIME_ACCOUNTING
> +	if (static_branch_likely(&sched_clock_irqtime)) {
> +		u64 now, irq;
> +		s64 delta;
> +
> +		now = cpu_clock(cpu);
> +		irq = irq_time_read(cpu);
> +		delta = (s64)(irq - rq->psi_irq_time);
> +		if (delta > 0) {
> +			rq->psi_irq_time = irq;
> +			psi_account_irqtime(rq, curr, NULL, now, delta);
> +			cgroup_account_cputime(curr, delta);
> +			/* We account both softirq and irq into softirq */
> +			cgroup_account_cputime_field(curr, CPUTIME_SOFTIRQ, delta);
> +		}
> +	}
> +#endif
>  
>  	update_rq_clock(rq);
>  	hw_pressure = arch_scale_hw_pressure(cpu_of(rq));
> @@ -6667,7 +6684,25 @@ static void __sched notrace __schedule(int sched_mode)
>  		++*switch_count;
>  
>  		migrate_disable_switch(rq, prev);
> -		psi_account_irqtime(rq, prev, next);
> +
> +#ifdef CONFIG_IRQ_TIME_ACCOUNTING
> +		if (static_branch_likely(&sched_clock_irqtime)) {
> +			u64 now, irq;
> +			s64 delta;
> +
> +			now = cpu_clock(cpu);
> +			irq = irq_time_read(cpu);
> +			delta = (s64)(irq - rq->psi_irq_time);
> +			if (delta > 0) {
> +				rq->psi_irq_time = irq;
> +				psi_account_irqtime(rq, prev, next, now, delta);
> +				cgroup_account_cputime(prev, delta);
> +				/* We account both softirq and irq into softirq */
> +				cgroup_account_cputime_field(prev, CPUTIME_SOFTIRQ, delta);
> +			}
> +		}
> +#endif

This should be inside its own function - to avoid duplication of
course, but also the ifdefs and overly detailed accounting code in the
middle of core scheduling logic.

#ifdef CONFIG_IRQ_TIME_ACCOUNTING
static void account_irqtime(struct rq *rq, ...)
{
	...
}
#else
static inline void account_irqtime(...) {}
#endif