Re: [PATCH v6 5/6] sched/fair: Allow load balancing between CPUs of identical capacity

[Vincent Guittot]

On Thu, 23 Jul 2026 at 09:11, Christian Loehle <christian.loehle@xxxxxxx> wrote:
>
> On 7/21/26 03:43, Ricardo Neri wrote:
> > sched_balance_find_src_rq() avoids selecting a runqueue with a single
> > running task as busiest if doing so results in migrating the task to a
> > CPU with less than ~5% of extra capacity. It also unintentionally
> > prevents migrations between CPUs of identical capacity.
> >
> > When CONFIG_SCHED_CLUSTER is enabled, load should be balanced across
> > clusters of CPUs with the same capacity. Allowing migration between CPUs
> > of identical capacity is necessary to meet this goal.
> >
> > Use get_actual_cpu_capacity() to reflect architectural capacity as well
> > as diminished capacity due to hardware or cpufreq pressure. Guard this
> > check with the sched_cluster_active static key so that systems without
> > cluster topology are unaffected.
> >
> > Tested-by: Christian Loehle <christian.loehle@xxxxxxx>
> > Tested-by: Andrea Righi <arighi@xxxxxxxxxx>
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@xxxxxxxxxxxxxxx>
> > ---
> > Changes in v6:
> >  * Switched to use get_actual_cpu_capacity() instead of
> >    arch_scale_cpu_capacity(). The former considers rq->avg_hw.load_avg and
> >    cpufreq_pressure and their impact on CPU capacity. (Vincent)
> >  * Renamed the variable same_arch_cluster as cluster_equal_cap for
> >    clarity. (Andrea)
> >  * Added Tested-by tag from Andrea. Thanks!
> >
> > Changes in v5:
> >  * Optimized logic to identify same-arch clusters only when needed.
> >  * Added Tested-by tag from Christian. Thanks!
> >
> > Changes in v4:
> >  * Implemented the check for cluster with a local variable for improved
> >    readability.
> >
> > Changes in v3:
> >  * Reverted the inverted capacity check; the inverted form incorrectly
> >    allows migrations to CPUs of slightly less capacity.
> >  * Guarded the check for architectural capacity with the
> >    sched_cluster_active static key.
> >
> > Changes in v2:
> >  * Used arch_scale_cpu_capacity() instead of capacity_of() to ignore
> >    runtime variability.
> >  * Inverted the check for runtime capacity. (Christian)
> >  * Reworded patch description for clarity.
> > ---
> >  kernel/sched/fair.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index feea47e6abea..de4189b562ac 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -13104,13 +13104,20 @@ static struct rq *sched_balance_find_src_rq(struct lb_env *env,
> >                */
> >               if (env->sd->flags & SD_ASYM_CPUCAPACITY &&
> >                   nr_running == 1) {
> > +                     bool cluster_equal_cap = static_branch_unlikely(&sched_cluster_active) &&
> > +                                              (get_actual_cpu_capacity(env->dst_cpu) ==
> > +                                               get_actual_cpu_capacity(i));
>
>
> I guess it's extremely unlikely, but it _feels_ wrong to have to clusters of different
> arch_scale_cpu_capacity() equal to true here because of system/thermal pressure (which is
> obviously considered more transient).
> Adding && arch_scale_cpu_capacity(env->dst_cpu) == arch_scale_cpu_capacity(i) might even
> make the check cheaper because it's better for the branch predictor than get_actual_cpu_capacity().
> Vincent, would you be fine with requiring both: equal get_actual_cpu_capacity() and
> arch_scale_cpu_capacity()?

TBH, I don't have a strong opinion on this. I have in mind that the
cpufreq pressure can last a long time (i.e., several hundreds of ms)
so it could make sense to spread tasks between clusters even if one
has lower max capacity. The task will migrate back to the cpu with
higher capacity once the cpufreq pressure is removed but this would
need some test results


>
> Given how unlikely it actually is in any case:
> Reviewed-by: Christian Loehle <christian.loehle@xxxxxxx>
>
> >                       bool smt_degraded_cap = sched_smt_active() && !is_core_idle(i);
> >
> >                       /*
> >                        * Busy SMT siblings reduce the capacity of CPU @i. Do
> >                        * not skip it in this case.
> > +                      *
> > +                      * CONFIG_SCHED_CLUSTER requires balancing load across
> > +                      * clusters of identical capacity, accounting for
> > +                      * hardware and cpufreq pressure.
> >                        */
> > -                     if (!smt_degraded_cap &&
> > +                     if (!smt_degraded_cap && !cluster_equal_cap &&
> >                           !capacity_greater(capacity_of(env->dst_cpu), capacity))
> >                               continue;
> >               }
> >
>