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

[Dietmar Eggemann]

On 06.05.26 14:59, Vincent Guittot wrote:
> On Tue, 28 Apr 2026 at 16:44, Andrea Righi <arighi@xxxxxxxxxx> wrote:
>>
>> From: K Prateek Nayak <kprateek.nayak@xxxxxxx>

[...]

>> @@ -8026,10 +8027,28 @@ 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 = !prefers_idle_core || is_core_idle(cpu);
>>                 unsigned long cpu_cap = capacity_of(cpu);
>>
>> +               /*
>> +                * Good-enough early exit (mirrors select_idle_cpu() logic).
>> +                */
>> +               if (!prefers_idle_core &&
>> +                   --nr <= 0 && best_fits == ASYM_IDLE_CORE_UCLAMP_MISFIT)
> 
> With SMT, !prefers_idle_core implies that there is no idle core; Is
> best_fits == ASYM_IDLE_CORE_UCLAMP_MISFIT really expected in such case
> ?
> 
> With !SMT, !prefers_idle_core is always true and we will bail out
> early as expected

I struggle to comprehend:

I assume the mirrored select_idle_cpu() logic is:

    for_each_cpu_wrap(cpu, cpus, target + 1)

      if (has_idle_core)

      else
        if (--nr <= 0)
          return -1

Should this condition not be just:

  if (!prefers_idle_core && --nr <= 0)
    return best_cpu

since if we do a:

  if (!choose_idle_cpu(cpu, p)))
    continue;

right after that?

best_cpu is -1 by default so sis() will return target, in case we
already found a best_cpu then sis() will return this instead.

What do I miss here?

> 
> 
>> +                       return best_cpu;
>> +
>>                 if (!choose_idle_cpu(cpu, p))
>>                         continue;

[...]