Re: [RFC PATCH 5/6 v8] sched/fair: Enable idle core tracking for !SMT

[Christian Loehle <christian.loehle@arm.com>]

On 12/2/25 18:12, Vincent Guittot wrote:
> Enable has_idle_cores at llc level feature for !SMT system for which
> CPU equals core.
> 
> We don't enable has_idle_core feature of select_idle_cpu to be
> conservative and don't parse all CPUs of LLC.
> 
> At now, has_idle_cores can be cleared even if a CPU is idle because of
> SIS_UTIL but it looks reasonnable as the probablity to get an idle CPU is
> low anyway.
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>  kernel/sched/fair.c  | 29 +++++++----------------------
>  kernel/sched/sched.h | 42 +++++++++++++++++++++++++++++-------------
>  2 files changed, 36 insertions(+), 35 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 252254168c92..0c0c675f39cf 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> [snip]
> @@ -7849,80 +7831,83 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>         }
>  
>         /*
>          * For asymmetric CPU capacity systems, our domain of interest is
>          * sd_asym_cpucapacity rather than sd_llc.
>          */
>         if (sched_asym_cpucap_active()) {
>                 sd = rcu_dereference(per_cpu(sd_asym_cpucapacity, target));
>                 /*
>                  * On an asymmetric CPU capacity system where an exclusive
>                  * cpuset defines a symmetric island (i.e. one unique
>                  * capacity_orig value through the cpuset), the key will be set
>                  * but the CPUs within that cpuset will not have a domain with
>                  * SD_ASYM_CPUCAPACITY. These should follow the usual symmetric
>                  * capacity path.
>                  */
>                 if (sd) {
>                         i = select_idle_capacity(p, sd, target);
>                         return ((unsigned)i < nr_cpumask_bits) ? i : target;
>                 }
>         }
>  
>         sd = rcu_dereference(per_cpu(sd_llc, target));
>         if (!sd)
>                 return target;
>  
>         if (sched_smt_active()) {
>                 has_idle_core = test_idle_cores(target);
>  
>                 if (!has_idle_core && cpus_share_cache(prev, target)) {
>                         i = select_idle_smt(p, sd, prev);
>                         if ((unsigned int)i < nr_cpumask_bits)
>                                 return i;
>                 }
>         }
>  
>         i = select_idle_cpu(p, sd, has_idle_core, target);
>         if ((unsigned)i < nr_cpumask_bits)
>                 return i;
>  
> +       if (!sched_smt_active())
> +               set_idle_cores(target, 0);

I have added some more context for the patch that makes it rather obvious that
this is broken. For asym systems (and their subset EAS which this series is
concerned with) the above is unreachable as either select_idle_capacity() will
find a CPU or target is returned.
Thus this code here is never reached and idle_cores will be set but never stopped
and the push mechanism keep triggering when it shouldn't be.