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

[K Prateek Nayak]

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?

> 
> 
>> +       }
>> +
>>         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?

> 
>> +
>>                 if (!choose_idle_cpu(cpu, p))
>>                         continue;
>>
>> --
>> 2.54.0
>>

-- 
Thanks and Regards,
Prateek