Re: [PATCH 2/2] sched/fair: Honor asymmetric SMT priority in idle selection

[Dietmar Eggemann <dietmar.eggemann@arm.com>]

On 31.08.26 20:10, Andrea Righi wrote:
> SD_ASYM_PACKING orders CPUs that share an SMT core, but idle CPU
> selection does not consult that order. A task can therefore wake on an
> arbitrary sibling and remain there until load balancing corrects the
> placement. On SMT implementations where changing the active sibling
> repartitions core resources, that initial choice can cause a large and
> persistent performance loss.

I assume this sentence refers to Olympus/Vera and Power7?

[...]

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 8dff37059faf7..3c49aa63742cb 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8587,6 +8587,65 @@ static inline bool test_idle_cores(int cpu)
>  	return false;
>  }
>  
> +/*
> + * Return true when @cpu has a higher asymmetric-packing priority than @other in their SMT
> + * scheduling domain.
> + */
> +static bool sched_smt_asym_prefer(int cpu, int other)
> +{
> +	struct sched_domain *sd;
> +
> +	for_each_domain(cpu, sd) {
> +		/*
> +		 * Only honor priorities declared at shared-capacity SMT levels.
> +		 * SD_ASYM_PACKING at higher levels may describe core ordering.
> +		 */
> +		if (!(sd->flags & SD_SHARE_CPUCAPACITY))
> +			break;
> +
> +		if ((sd->flags & SD_ASYM_PACKING) && cpumask_test_cpu(other, sched_domain_span(sd)))

Looks like 'other' is always part of the mask?

> +			return sched_asym_prefer(cpu, other);
> +	}
> +
> +	return false;
> +}

SMT will always the lowest SD, so for_each_domain() is not necessary:

static bool sched_smt_asym_prefer(int cpu, int other)
{
        struct sched_domain *sd = rcu_dereference_all(cpu_rq(cpu)->sd);

        if (sd && ((sd->flags & (SD_SHARE_CPUCAPACITY |
SD_ASYM_PACKING)) == (SD_SHARE_CPUCAPACITY | SD_ASYM_PACKING)))
                return sched_asym_prefer(cpu, other);

        return false;
}

[...]

> @@ -8668,7 +8727,7 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t
>  		if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
>  			continue;
>  		if (choose_idle_cpu(cpu, p))
> -			return cpu;
> +			return select_idle_smt_priority(p, cpu);

This one is weird for SMT2. AFAICS, select_idle_smt() is called when
there are no idle cores. So if you find an idle CPU this is what you
will return anyway.

I guess your tests on Olympus/Vera do wakeups via select_idle_capacity()
so you haven't touched this one.

[...]