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

Hi Prateek,

On Mon, Sep 07, 2026 at 09:27:22AM +0530, 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?

Yes, agreed. I like this way more.

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?

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?

On Olympus it'd be fine either way, since it's an SMT2. For wider SMT systems I
also defer the question to Shrikanth, I don't have any of them to test. :)

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:

Yes, consolidating it in select_idle_sibling() looks cleaner. One comment below.

Not sure about this final fallback. Is it worth doing an additional
select_idle_smt_priority() after idle scan failed or stopped because the
SIS_UTIL scan budget was exhausted?

It seems better to jump to out only when one of these paths has actually
selected a candidate:

	i = select_idle_cpu(p, sd, has_idle_core, target);
	if ((unsigned int)i < nr_cpumask_bits) {
		target = i;
		goto out;
	}

The prev_aff and recent_used_cpu fallbacks can jump to "out" as well, since they
were already verified as suitable candidates. If none of those paths succeeds, I
think the existing final "return target" should remain unchanged.

Does that make sense?

Thanks for looking at this!
-Andrea

 	/*
 	 * 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?

-- 
Thanks and Regards,
Prateek