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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <01d82628-7ae8-4e85-9e0d-e18a1c93f284@arm.com>
Date: Mon, 8 Dec 2025 18:43:16 +0000
From: Christian Loehle <christian.loehle@....com>
To: Vincent Guittot <vincent.guittot@...aro.org>, mingo@...hat.com,
 peterz@...radead.org, juri.lelli@...hat.com, dietmar.eggemann@....com,
 rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
 vschneid@...hat.com, linux-kernel@...r.kernel.org, pierre.gondois@....com,
 kprateek.nayak@....com
Cc: qyousef@...alina.io, hongyan.xia2@....com, luis.machado@....com
Subject: Re: [RFC PATCH 5/6 v8] sched/fair: Enable idle core tracking for !SMT

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@...aro.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.