Re: [PATCH] Revert "sched/fair: Add SIS_UTIL support to select_idle_capacity()"

[Andrea Righi <arighi@nvidia.com>]

Hi Usama,

On Fri, Nov 13, 2026 at 07:31:57AM -0800, Usama Arif wrote:
> This reverts commit 61ea17a63719 ("sched/fair: Add SIS_UTIL support to
> select_idle_capacity()").
> 
> That commit applied the SIS_UTIL scan budget to select_idle_capacity().
> The early checks in select_idle_sibling() consider a few specific
> candidates: the target, previous and recently used CPUs. If none is
> selected, the asymmetric path calls select_idle_capacity() to search
> the allowed domain. With SIS_UTIL enabled, a zero scan budget makes it
> return -1 before that search. The caller then returns the selected
> target, even if an unexamined CPU in the domain is idle and fits the
> task.

Isn't this consistent with select_idle_cpu(), which also skips the scan
when the SIS_UTIL budget is zero? Missing an idle CPU is an intentional
tradeoff of that heuristic. Could you clarify why we should remove the
budget specifically from the asymmetric path?

> 
> The budget is derived from aggregate CFS utilization. High aggregate
> utilization can coexist with idle CPUs when a task wakes.
> 
> On a 72-CPU NVIDIA grace arm64 server, small reported capacity differences
> (1013-1024) within the single LLC activate asymmetric selection. Both
> the reference and affected kernels were observed with a zero scan
> hint. The reference kernel ignored it and still searched the domain.
> A latency-sensitive, wakeup-heavy production workload saw its reported
> minute p50 latencies average 129 ms on the reference kernel and 223 ms
> on the affected kernel.

Have you tried forcing the symmetric idle-selection path through
select_idle_cpu(), with SIS_UTIL both enabled and disabled?

It'd be useful to see whether disabling SIS_UTIL produces the same
improvement there and whether symmetric idle selection performs better
overall on this system.

> 
> Disabling SIS_UTIL reduced the aggregate involuntary context switch
> rate of worker main threads from 71.6k/s to 13.3k/s; re-enabling it
> raised the rate to 73.7k/s. A five-minute disabled run also reduced
> kernel CPU time per request by 10.9%. The available minute p50 samples
> averaged 149 ms, compared with 241 ms before the trial.
> 
> These measurements come from disabling SIS_UTIL on the installed
> kernel. The feature toggle also disables the symmetric scan limit and
> shared scan-hint updates.
> 
> Restore the search of the allowed domain, retaining the capacity,
> uclamp and SMT selection rules and their early returns. Domains with
> no suitable idle CPU can again require a full scan, potentially increasing
> scan CPU cost.

Yeah, this is my main concern, full scans can become quite expensive on
large systems with lots of CPUs in the same LLC.

Thanks,
-Andrea

> 
> Reported-by: Arnav Aghav <aghav@meta.com>
> Fixes: 61ea17a63719 ("sched/fair: Add SIS_UTIL support to select_idle_capacity()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  kernel/sched/fair.c | 19 -------------------
>  1 file changed, 19 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d4cc0caee6265..def5cda2e6bad 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9091,7 +9091,6 @@ select_idle_capacity(struct task_struct *p, struct sched_domain *sd, int target)
>  	int fits, best_fits = ASYM_IDLE_THREAD_MISFIT;
>  	int cpu, best_cpu = -1;
>  	struct cpumask *cpus;
> -	int nr = INT_MAX;
>  
>  	cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
>  	cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
> @@ -9100,28 +9099,10 @@ select_idle_capacity(struct task_struct *p, struct sched_domain *sd, int target)
>  	util_min = uclamp_eff_value(p, UCLAMP_MIN);
>  	util_max = uclamp_eff_value(p, UCLAMP_MAX);
>  
> -	if (sched_feat(SIS_UTIL) && sd->shared) {
> -		/*
> -		 * Same nr_idle_scan hint as select_idle_cpu(), nr only limits
> -		 * the scan when not preferring an idle core.
> -		 */
> -		nr = READ_ONCE(sd->shared->nr_idle_scan) + 1;
> -		/* overloaded domain is unlikely to have idle cpu/core */
> -		if (nr == 1)
> -			return -1;
> -	}
> -
>  	for_each_cpu_wrap(cpu, cpus, target) {
>  		bool preferred_core = !has_idle_core || is_core_idle(cpu);
>  		unsigned long cpu_cap = capacity_of(cpu);
>  
> -		/*
> -		 * Stop when the nr_idle_scan is exhausted (mirrors
> -		 * select_idle_cpu() logic).
> -		 */
> -		if (!has_idle_core && --nr <= 0)
> -			return best_cpu;
> -
>  		if (!choose_idle_cpu(cpu, p))
>  			continue;
>  
> -- 
> 2.53.1-Meta
> 
>