Re: [PATCH v3 1/4] sched/fair: Check CPU capacity before comparing group types during load balance

[Tim Chen]

On Thu, 2026-05-14 at 11:34 -0700, Ricardo Neri wrote:
> update_sd_pick_busiest() may incorrectly select a fully_busy group as the
> busiest group when its per-CPU capacity exceeds that of the destination
> CPU. This happens because the type of busiest group is initialized to
> group_has_spare and allows the fully_busy group to win the type comparison.
> 
> update_sd_pick_busiest() should not choose a candidate scheduling group
> with at most one runnable task if its per-CPU capacity is greater than that
> of the destination CPU. Such a check already exists, but it is done too
> late: after the type comparison, preventing a subsequent fully_busy group
> of equal per-CPU capacity from being correctly selected.
> 
> Move this check to occur before comparing group types.

Looks good to me.

Reviewed-by: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>

> 
> Fixes: 0b0695f2b34a ("sched/fair: Rework load_balance()")
> Reviewed-by: Christian Loehle <christian.loehle@xxxxxxx>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@xxxxxxxxxxxxxxx>
> ---
> Changes in v3:
>  * Added a Fixes tag. (Christian)
>  * Added Reviewed-by tag from Christian. Thanks!
> 
> Changes in v2:
>  * Added a note clarifying that SMT and SD_ASYM_CPUCAPACITY are mutually
>    exclusive. (Tim)
>  * Kept parentheses around bitwise operators for clarity.
>  * Rewrote patch description for clarity.
> ---
>  kernel/sched/fair.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 3ebec186f982..e06e74d9ce0e 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10818,6 +10818,20 @@ static bool update_sd_pick_busiest(struct lb_env *env,
>  	     sds->local_stat.group_type != group_has_spare))
>  		return false;
>  
> +	/*
> +	 * Candidate sg has no more than one task per CPU and has higher
> +	 * per-CPU capacity. Migrating tasks to less capable CPUs may harm
> +	 * throughput. Maximize throughput, power/energy consequences are not
> +	 * considered.
> +	 *
> +	 * Systems with SMT are unaffected, as asymmetric capacity is not set
> +	 * in such cases.
> +	 */
> +	if ((env->sd->flags & SD_ASYM_CPUCAPACITY) &&
> +	    (sgs->group_type <= group_fully_busy) &&
> +	    (capacity_greater(sg->sgc->min_capacity, capacity_of(env->dst_cpu))))
> +		return false;
> +
>  	if (sgs->group_type > busiest->group_type)
>  		return true;
>  
> @@ -10920,17 +10934,6 @@ static bool update_sd_pick_busiest(struct lb_env *env,
>  		break;
>  	}
>  
> -	/*
> -	 * Candidate sg has no more than one task per CPU and has higher
> -	 * per-CPU capacity. Migrating tasks to less capable CPUs may harm
> -	 * throughput. Maximize throughput, power/energy consequences are not
> -	 * considered.
> -	 */
> -	if ((env->sd->flags & SD_ASYM_CPUCAPACITY) &&
> -	    (sgs->group_type <= group_fully_busy) &&
> -	    (capacity_greater(sg->sgc->min_capacity, capacity_of(env->dst_cpu))))
> -		return false;
> -
>  	return true;
>  }
>