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

[Dietmar Eggemann]

On 07.05.26 08:47, Vincent Guittot wrote:
> On Wed, 6 May 2026 at 20:11, Andrea Righi <arighi@xxxxxxxxxx> wrote:
>>
>> Hi Dietmar and Vincent,
>>
>> On Wed, May 06, 2026 at 07:01:35PM +0200, Dietmar Eggemann wrote:
>>> 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
>>
>> So, the logic in select_idle_cpu() is that as soon as nr <= 0, we stops the walk
>> and returns -1, without any "only stop if the answer is good enough" guard.
>>
>> With this change in select_idle_capacity() when nr is exhausted, we stop only if
>> best_cpu is "good enough" (ASYM_IDLE_CORE_UCLAMP_MISFIT), otherwise we keep
>> scanning. Therefore, we're not perfectly mirroring select_idle_cpu().

But when '--nr <= 0', does it actually make sense to continue scanning
for an _idle_ CPU?

  for_each_cpu_wrap(cpu, cpus, target)

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

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

I thought we want to bail since it doesn't. The likelihood that
choose_idle_cpu() will return 0 is high so from the point of '--nr <= 0'
we would not be able to reach the condition to alter best_cpu anymore?

Isn't this similar to select_idle_cpu()?

  for_each_cpu_wrap(cpu, cpus, target + 1)

    else
      if (--nr <= 0)
        return -1;
      idle_cpu = __select_idle_cpu(cpu, p);
                   choose_idle_cpu(cpu, p)
      if ((unsigned int)idle_cpu < nr_cpumask_bits)
        break;

[...]