[PATCH 2/2] sched/fair: Honor asymmetric SMT priority in idle selection
Shrikanth Hegde
sshegde at linux.ibm.com
Mon Sep 7 09:48:25 PDT 2026
On 9/7/26 9:27 AM, K Prateek Nayak wrote:
> Hello Andrea,
>
> On 9/4/2026 2:48 PM, Andrea Righi wrote:
>> +/*
>> + * Return true when @cpu has a higher asymmetric-packing priority than
>> + * @other in their shared SMT scheduling domain.
>> + */
>> +static bool sched_smt_asym_prefer(int cpu, int other)
>> +{
>> + struct sched_domain *sd = rcu_dereference_all(cpu_rq(cpu)->sd);
>> +
>> + if (!sd)
>> + return false;
>> +
>> + if (!(sd->flags & SD_SHARE_CPUCAPACITY) ||
>> + !(sd->flags & SD_ASYM_PACKING))
>> + return false;
>> +
>> + if (!cpumask_test_cpu(other, sched_domain_span(sd)))
>> + return false;
>> +
>> + return sched_asym_prefer(cpu, other);
>> +}
>> +
>> +/*
>> + * Return the highest-priority available CPU in @cpu's SMT core that is also in @cpus.
>> + */
>> +static int __select_idle_smt_cpu(struct task_struct *p, int cpu, const struct cpumask *cpus)
>> +{
>> + int best = cpu;
>> + int sibling;
>> +
>> + for_each_cpu_and(sibling, cpu_smt_mask(cpu), cpus) {
>> + if (sibling == best || !choose_idle_cpu(sibling, p))
>> + continue;
>> +
>> + if (sched_smt_asym_prefer(sibling, best))
>> + best = sibling;
>
> nit. Since sched_smt_asym_prefer() is only used here, and we know rq->sd
> is the one that can have SD_SHARE_CPUCAPACITY | SD_ASYM_PACKING, perhaps
> you can inline the check here do a:
>
> sd = rcu_dereference_all(cpu_rq(cpu)->sd);
>
> if (!sd)
> return cpu;
>
> if (!(sd->flags & SD_SHARE_CPUCAPACITY) || !(sd->flags & SD_ASYM_PACKING))
> return cpu;
>
> for_each_cpu_and (sibling, sched_domain_span(sd), cpus) {
> ...
> }
>
> ...
>
>
> That way, you don't need to dereference cpu_rq(cpu)->sd every time in
> sched_smt_asym_prefer() and check cpumask_test_cpu(). Both, domain
> span and task affinity will be covered at once.
>
> Thoughts?
>
>> + }
>> +
>> + return best;
>> +}
>> +
>> +static inline int
>> +select_idle_smt_cpu(struct task_struct *p, int cpu, const struct cpumask *cpus)
>> +{
>> + if (!sched_smt_asym_active())
>> + return cpu;
>> +
>> + return __select_idle_smt_cpu(p, cpu, cpus);
>> +}
>> +
>> +/*
>> + * Redirect an available SMT CPU to a higher-priority available sibling allowed by task affinity.
>> + */
>> +static inline int select_idle_smt_priority(struct task_struct *p, int cpu)
>> +{
>> + return select_idle_smt_cpu(p, cpu, p->cpus_ptr);
>> +}
>> +
>> /*
>> * Scans the local SMT mask to see if the entire core is idle, and records this
>> * information in sd_balance_shared->has_idle_cores.
>> @@ -8645,7 +8702,7 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
>> }
>>
>> if (idle)
>> - return core;
>> + return select_idle_smt_cpu(p, core, cpus);
>>
>> cpumask_andnot(cpus, cpus, cpu_smt_mask(core));
>> return -1;
>> @@ -8668,7 +8725,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);
>> }
>>
>> return -1;
>> @@ -8720,7 +8777,7 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
>> return -1;
>> idle_cpu = __select_idle_cpu(cpu, p);
>> if ((unsigned int)idle_cpu < nr_cpumask_bits)
>> - return idle_cpu;
>> + return select_idle_smt_priority(p, idle_cpu);
>
> Question for Shrikanth: On larger SMT (SMT-4, SMT-8), does the ranking
> make that big of a difference if the core is already busy?
>
Only on Power7 we had AYSM PACKING.
There IPC of CPU0 > CPU1 > CPU2 > CPU4 for the four siblings IIRC irrespective of busy
or idle.
PS: I haven't seen the patches in detail yet.
> Does the overehead of additional search get offset by the benefit of
> being placed on a better ranked thread? If not, maybe the paths for
> !has_idle_core can stay as is?
>
>> }
>> }
>> cpumask_andnot(cpus, cpus, sched_group_span(sg));
>> @@ -8745,7 +8802,8 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
>> if (has_idle_core)
>> set_idle_cores(target, false);
>>
>> - return idle_cpu;
>> + return (unsigned int)idle_cpu < nr_cpumask_bits ?
>> + select_idle_smt_priority(p, idle_cpu) : idle_cpu;
>
> Since every path does a select_idle_smt_priority() - be it coming from
> select_idle_core(), the early-return from the cluster scan, or just an
> idle CPU from the LLc scan, can't we simply just do it once in
> select_idle_sibling()?
>
> Something like:
>
> (Only build tested)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index f79fcba4afec..7c97585141dd 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8964,7 +8964,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>
> if (choose_idle_cpu(target, p) &&
> asym_fits_cpu(task_util, util_min, util_max, target))
> - return target;
> + goto out;
>
> /*
> * If the previous CPU is cache affine and idle, don't be stupid:
> @@ -8974,8 +8974,10 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
> asym_fits_cpu(task_util, util_min, util_max, prev)) {
>
> if (!static_branch_unlikely(&sched_cluster_active) ||
> - cpus_share_resources(prev, target))
> - return prev;
> + cpus_share_resources(prev, target)) {
> + target = prev;
> + goto out;
> + }
>
> prev_aff = prev;
> }
> @@ -8993,7 +8995,8 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
> prev == smp_processor_id() &&
> this_rq()->nr_running <= 1 &&
> asym_fits_cpu(task_util, util_min, util_max, prev)) {
> - return prev;
> + target = prev;
> + goto out;
> }
>
> /* Check a recently used CPU as a potential idle candidate: */
> @@ -9007,8 +9010,10 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
> asym_fits_cpu(task_util, util_min, util_max, recent_used_cpu)) {
>
> if (!static_branch_unlikely(&sched_cluster_active) ||
> - cpus_share_resources(recent_used_cpu, target))
> - return recent_used_cpu;
> + cpus_share_resources(recent_used_cpu, target)) {
> + target = recent_used_cpu;
> + goto out;
> + }
>
> } else {
> recent_used_cpu = -1;
> @@ -9030,7 +9035,8 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
> */
> if (sd) {
> i = select_idle_capacity(p, sd, target);
> - return ((unsigned)i < nr_cpumask_bits) ? i : target;
> + target = ((unsigned)i < nr_cpumask_bits) ? i : target;
> + goto out;
> }
> }
>
> @@ -9043,27 +9049,31 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>
> if (!has_idle_core && cpus_share_cache(prev, target)) {
> i = select_idle_smt(p, sd, prev);
> - if ((unsigned int)i < nr_cpumask_bits)
> - return i;
> + if ((unsigned int)i < nr_cpumask_bits) {
> + target = i;
> + goto out;
> + }
> }
> }
>
> i = select_idle_cpu(p, sd, has_idle_core, target);
> if ((unsigned)i < nr_cpumask_bits)
> - return i;
> -
> + target = i;
> /*
> * For cluster machines which have lower sharing cache like L2 or
> * LLC Tag, we tend to find an idle CPU in the target's cluster
> * first. But prev_cpu or recent_used_cpu may also be a good candidate,
> * use them if possible when no idle CPU found in select_idle_cpu().
> */
> - if ((unsigned int)prev_aff < nr_cpumask_bits)
> - return prev_aff;
> - if ((unsigned int)recent_used_cpu < nr_cpumask_bits)
> - return recent_used_cpu;
> + else if ((unsigned int)prev_aff < nr_cpumask_bits)
> + target = prev_aff;
> + else if ((unsigned int)recent_used_cpu < nr_cpumask_bits)
> + target = recent_used_cpu;
> +out:
> + if (!sched_smt_asym_active())
> + return target;
>
> - return target;
> + return select_idle_smt_priority(p, target);
> }
>
> /**
> ---
>
> That way, it lives in a single place, and we don't have to pepper
> select_idle_smt_priority() everywhere. Thoughts?
>
More information about the linux-arm-kernel
mailing list