Re: [PATCH 2/2] sched/fair: Honor asymmetric SMT priority in idle selection
From: Shrikanth Hegde <sshegde@linux.ibm.com>
Date: 2026-09-07 16:49:20
Also in:
lkml
On 9/7/26 9:27 AM, K Prateek Nayak wrote:
Hello Andrea, On 9/4/2026 2:48 PM, Andrea Righi wrote:quoted
+/* + * 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?quoted
+ } + + 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.
quoted hunk ↗ jump to hunk
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?quoted
} } 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?