Re: [PATCH RFC 1/2] kernel/sched: Introduce idle SMT priority

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sched: Unintended loss of SMT balancing within same-capacity clusters during wakeup
--

commit e7b6978e87f4966f837633079b0ad5b78914e3c2
Author: Mete Durlu <meted@linux.ibm.com>

kernel/sched: Introduce idle SMT priority

On systems with asymmetric CPU capacities and SMT, this patch introduces
SCHED_IDLE_SMT_PRIO to allow architectures to prefer idle SMT siblings of
busy high-capacity cores over fully idle low-capacity cores.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 7455a83a6a990..3e21233ee2ce2 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8813,11 +8813,14 @@ static int
>  select_idle_capacity(struct task_struct *p, struct sched_domain *sd, int target)
>  {
>  	/*
> -	 * On !SMT systems, has_idle_core is always false and preferred_core
> -	 * is always true (CPU == core), so the SMT preference logic below
> -	 * collapses to the plain capacity scan.
> -	 */
> -	bool has_idle_core = sched_smt_active() && test_idle_cores(target);
> +	 * On !SMT systems or when idle SMT thread priority is active,
> +	 * has_idle_core is always false and preferred_core is always true
> +	 * (CPU == core), so the SMT preference logic below collapses to the
> +	 * plain capacity scan.
> +	 */
> +	bool has_idle_core = sched_smt_active() &&
> +			     !sched_idle_smt_prio_active() &&
> +			     test_idle_cores(target);

[Severity: High]
By unconditionally setting has_idle_core to false here when
sched_idle_smt_prio_active() is true, does this bypass idle core detection
system-wide for all capacities?

[ ... ]

> @@ -8860,8 +8863,8 @@ select_idle_capacity(struct task_struct *p, struct sched_domain *sd, int target)
>  
>  		/*
>  		 * Perfect fit: capacity satisfies util + uclamp and the CPU
> -		 * sits on a fully-idle SMT core, this is a !SMT system, or
> -		 * there is no idle core to find.
> +		 * sits on a fully-idle SMT core, this is a !SMT system, idle
> +		 * smt priority is active, or there is no idle core to find.
>  		 * Short-circuit the rank-based selection and return
>  		 * immediately.
>  		 */

[Severity: High]
Since has_idle_core is now false, preferred_core will evaluate to true
for all CPUs scanned.

Could this cause the loop below:

    if (fits > 0 && preferred_core)
        return cpu;

to short-circuit prematurely and return an idle SMT thread of a busy core,
failing to scan for fully idle cores of the identical capacity? This appears
to lead to an unintended loss of SMT balancing within same-capacity clusters
during wakeup.

[ ... ]

> @@ -8936,15 +8939,17 @@ static inline bool asym_fits_cpu(unsigned long util,
>  		 * Return true only if the cpu fully fits the task requirements
>  		 * which include the utilization and the performance hints.
>  		 *
> -		 * When SMT is active, also require that the core has no busy
> -		 * siblings.
> +		 * When SMT is active or idle SMT priority is disabled, also
> +		 * require that the core has no busy siblings.
>  		 *
>  		 * Note: gating on is_core_idle() also makes the early-bailout
>  		 * candidates in select_idle_sibling() (target, prev,
>  		 * recent_used_cpu) idle-core-aware on ASYM+SMT, which the
>  		 * NO_ASYM path does not do.
>  		 */
> -		return (!sched_smt_active() || is_core_idle(cpu)) &&
> +		return (!sched_smt_active() ||
> +		       sched_idle_smt_prio_active() ||
> +		       is_core_idle(cpu)) &&
>  		       (util_fits_cpu(util, util_min, util_max, cpu) > 0);
>  	}

[Severity: High]
Does bypassing the is_core_idle() check here when sched_idle_smt_prio_active()
is true inadvertently allow the early-bailout candidates (target, prev,
recent_used_cpu) to further exacerbate same-capacity SMT packing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-hiperdispatchfix-v1-0-73fe41081070@linux.ibm.com?part=1