Re: [PATCH] sched/topology: Avoid spurious asymmetry from CPU capacity noise

[Andrea Righi <arighi@nvidia.com>]

Hi Vincent,

On Tue, Mar 24, 2026 at 08:39:34AM +0100, Vincent Guittot wrote:
> On Tue, 24 Mar 2026 at 01:55, Andrea Righi <arighi@nvidia.com> wrote:
> >
> > On some platforms, the firmware may expose per-CPU performance
> > differences (e.g., via ACPI CPPC highest_perf) even when the system is
> > effectively symmetric. These small variations, typically due to silicon
> > binning, are reflected in arch_scale_cpu_capacity() and end up being
> > interpreted as real capacity asymmetry.
> >
> > As a result, the scheduler incorrectly enables SD_ASYM_CPUCAPACITY,
> > triggering asymmetry-specific behaviors, even though all CPUs have
> > comparable performance.
> >
> > Prevent this by treating CPU capacities within 20% of the maximum value
> 
> 20% is a bit high, my snapdragon rb5 has a mid CPU with a capacity of
> 871 but we still want to keep them different
> 
> Why would 5% not be enough?

Sure, 5% seems a more reasonable margin. I'll just reuse capacity_greater()
as suggested by Christian.

Thanks,
-Andrea

> 
> 
> 
> > as equivalent when building the asymmetry topology. This filters out
> > firmware noise, while preserving correct behavior on real heterogeneous
> > systems, where capacity differences are significantly larger.
> >
> > Reported-by: Felix Abecassis <fabecassis@nvidia.com>
> > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > ---
> >  kernel/sched/topology.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 061f8c85f5552..fe71ea9f3bda7 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -1432,9 +1432,8 @@ static void free_asym_cap_entry(struct rcu_head *head)
> >         kfree(entry);
> >  }
> >
> > -static inline void asym_cpu_capacity_update_data(int cpu)
> > +static inline void asym_cpu_capacity_update_data(int cpu, unsigned long capacity)
> >  {
> > -       unsigned long capacity = arch_scale_cpu_capacity(cpu);
> >         struct asym_cap_data *insert_entry = NULL;
> >         struct asym_cap_data *entry;
> >
> > @@ -1471,13 +1470,27 @@ static inline void asym_cpu_capacity_update_data(int cpu)
> >  static void asym_cpu_capacity_scan(void)
> >  {
> >         struct asym_cap_data *entry, *next;
> > +       unsigned long max_cap = 0;
> > +       unsigned long capacity;
> >         int cpu;
> >
> >         list_for_each_entry(entry, &asym_cap_list, link)
> >                 cpumask_clear(cpu_capacity_span(entry));
> >
> >         for_each_cpu_and(cpu, cpu_possible_mask, housekeeping_cpumask(HK_TYPE_DOMAIN))
> > -               asym_cpu_capacity_update_data(cpu);
> > +               max_cap = max(max_cap, arch_scale_cpu_capacity(cpu));
> > +
> > +       /*
> > +        * Treat small capacity differences (< 20% max capacity) as noise,
> > +        * to prevent enabling SD_ASYM_CPUCAPACITY when it's not really
> > +        * needed.
> > +        */
> > +       for_each_cpu_and(cpu, cpu_possible_mask, housekeeping_cpumask(HK_TYPE_DOMAIN)) {
> > +               capacity = arch_scale_cpu_capacity(cpu);
> > +               if (capacity * 5 >= max_cap * 4)
> > +                       capacity = max_cap;
> > +               asym_cpu_capacity_update_data(cpu, capacity);
> > +       }
> >
> >         list_for_each_entry_safe(entry, next, &asym_cap_list, link) {
> >                 if (cpumask_empty(cpu_capacity_span(entry))) {
> > --
> > 2.53.0
> >