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

[Vincent Guittot <vincent.guittot@linaro.org>]

On Fri, 13 Nov 2026 at 22:32, Andrea Righi <arighi@nvidia.com> wrote:
>
> On Fri, Nov 13, 2026 at 10:26:00AM -0800, Usama Arif wrote:
> > On Fri, 13 Nov 2026 18:28:41 +0100 Andrea Righi <arighi@nvidia.com> wrote:
> >
> > > Hi Usama,
> > >
> >
> > Hello Andrea!
> >
> > > 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?
> > >
> >
> > It is consistent.
> >
> > Symmetric path also has this (I didnt check this before sending the patch).
> > The service is running on Grace servers which report capacity differences
> > and updating the kernel from 6.16 resulted in the problem I outlined below.
> >
> > Maybe the symmetric path needs to be audited as well?
> >
> > Or maybe this service should not have ever run with SIS_UTIL at all..
> > (The only thing is that kernel upgrade caused a serious regression)
> >
> > (Should have made this patch RFC :))
> >
> > > >
> > > > 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?
> > >
> >
> > This cant be done from a running kernel, right? My understanding is
> > that the cpu capacities come from firmware, so cant be changed at
> > runtime to be constant across all CPUs in kernel?
> >
> > Changing kernel on a production service can be a bit of a process.
>
> Yeah, this would require a patch and a reboot. Unfortunately there isn't
> an existing runtime switch to force symmetric/asym idle selection.
>
> >
> > > 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.
> > >
> >
> > Yeah I imagine this was the reason for the initial patch.
> >
> > The same workload on 6.16 kernel placed 69% of wakeups on an idle CPU,
> > the affected kernel with the patch that this is reverting only placed 7%.
>
> That's a substantial difference, I agree the regression should be
> addressed. Perhaps this points to a limitation of the SIS_UTIL
> heuristic?
>
> We could investigate incorporating observed idle CPU availability into
> the SIS_UTIL budget calculation, rather than relying solely on aggregate
> utilization. Periodic load balancing already collects idle CPU
> statistics, which might help distinguish actual saturation from high
> utilization with CPUs still available.

We want to keep same behavior for select_idle_cpu and
select_idle_capacity with both using the same SIS_UTIL algorithm.

That being said, the current algorithm is probably a bit too cautious,
particularly for systems with a small number of CPUs

And for a latency-sensitive workload, it would be good to consider
setting a shorter slice to the latency sensistive task would run 1st
even on a busy CPU

>
> Prateek, do you have any thoughts on this? Do you see a way to improve
> SIS_UTIL for this workload while keeping the symmetric and asymmetric
> paths consistent?
>
> >
> > Do you know how much CPU usage can be attributed to the scan on these
> > large systems if you do full scans?
>
> I don't have a measured CPU-cost figure for this case. My concern is
> the worst-case scan cost under frequent wakeups when few or no CPUs
> are idle.
>
> Thanks,
> -Andrea
>
> >
> > > 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
> > > >
> > > >
> > >
> >