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

[Koba Ko]

On Tue, Mar 24, 2026 at 10:39:41AM +0100, Andrea Righi wrote:
> 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@xxxxxxxxxx> 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
> 

How about modifying asym_cpu_capacity_update_data to group all CPUs within 5% capacity difference into the same group?
```
+#define capacity_greater(cap1, cap2) ((cap1) * 1024 > (cap2) * 1078)

        list_for_each_entry(entry, &asym_cap_list, link) {
-               if (capacity == entry->capacity)
+               if (!capacity_greater(capacity, entry->capacity) &&
+                   !capacity_greater(entry->capacity, capacity))

```

> > 
> > 
> > 
> > > 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@xxxxxxxxxx>
> > > Signed-off-by: Andrea Righi <arighi@xxxxxxxxxx>
> > > ---
> > >  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
> > >