Re: [PATCH 4/5] sched/fair: Add SIS_UTIL support to select_idle_capacity()

[Vincent Guittot]

On Mon, 27 Apr 2026 at 07:13, K Prateek Nayak <kprateek.nayak@xxxxxxx> wrote:
>
> Hello Vincent,
>
> On 4/24/2026 6:02 PM, Vincent Guittot wrote:
> >> +       if (sched_feat(SIS_UTIL) && sd->shared) {
> >> +               /*
> >> +                * Increment because !--nr is the condition to stop scan.
> >> +                *
> >> +                * Since "sd" is "sd_llc" for target CPU dereferenced in the
> >> +                * caller, it is safe to directly dereference "sd->shared".
> >> +                * Topology bits always ensure it assigned for "sd_llc" and it
> >> +                * cannot disappear as long as we have a RCU protected
> >> +                * reference to one the associated "sd" here.
> >> +                */
> >> +               nr = READ_ONCE(sd->shared->nr_idle_scan) + 1;
> >> +               /* overloaded LLC is unlikely to have idle cpu/core */
> >> +               if (nr == 1)
> >> +                       return -1;
> >
> > The comment below applies to select_idle_cpu but we want same behavior
> > for both function
> > If test_idle_cores is true we will not look for it whereas we don't
> > care about nr value when test_idle_core is true in the
> > for_each_cpu_wrap loop
>
> Ack but the initial "nr" based bailout in select_idle_cpu() applies even
> when test_idle_cores() is true too right?

Yes there is an early bail out for nr == 1 even with idle core.

>
> >
> >
> >> +       }
> >> +
> >>         for_each_cpu_wrap(cpu, cpus, target) {
> >>                 bool preferred_core = !prefers_idle_core || is_core_idle(cpu);
> >>                 unsigned long cpu_cap = capacity_of(cpu);
> >>
> >> +               /* We have found a good enough target. Just use it. */
> >> +               if (--nr <= 0 && best_fits == -4)
> >> +                       return best_cpu;
> >
> > In select_idle_cpu(), we return immediatly when nr == 0 and
> > test_idle_cores is false but we loop on all cpus if test_idle_cores is
> > true until we found an idle core. In the case of
> > select_idle_capacity(), I agree that util_fits_cpu() add another level
> > but shouldn't we continue to loop even if we found a best_fits == -4
>
> I see what you mean! We can additionally guard the bailout on
> "!prefers_idle_core".
>
> For the case where test_idle_cores() returns false, is stopping for a
> UCLAMP_MIN restricted CPU alright if SIS_UTIL bailout suggests it might
> not be fruitful to search further?
>
> Do you think it might trigger too many misfit balancing?

For SMT, I don't think this will make a real diff but for !SMT this
would indeed trigger more misfits.

>
> >
> >> +
> >>                 if (!choose_idle_cpu(cpu, p))
> >>                         continue;
> >>
> >> --
> >> 2.54.0
> >>
>
> --
> Thanks and Regards,
> Prateek
>