Re: [PATCH] sched/cache: Honor asym packing over cache aware scheduling on hybrid system

[Tim Chen]

On Tue, 2026-09-29 at 21:21 +0300, Kayra Cizmeci wrote:
> Hi Tim :>,
> 
> > > Let's say when entering can_migrate_llc_task(), dst_cpu is CPU0, while src_cpu is CPU1.
> > > And CPU0 has a bigger asym_prio than CPU1. No SMT. When entering can_migrate_llc_task() and sched_asym()
> > > from there sched_use_asym_prio() returns true without checking if the core is fully idle or not.
> 
> > sched_asym() does check whether the destination core is idle in sched_use_asym_prio() for non SMT domain.
> > 
> >                   (false for non-SMT sd)           (check idle core)
> >        return sd->flags & SD_SHARE_CPUCAPACITY || is_core_idle(cpu);
> > 
> > That is also a pre-condition for setting group_asym_packing.
> 
> Sorry for not showing the code earlier, here it is: 
> 
> static inline bool is_core_idle(int cpu)
> {
> 	int sibling;
> 
> 	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
> 		if (cpu == sibling)
> 			continue;
> 
> 		if (!idle_cpu(sibling))
> 			return false;
> 	}
> 
> 	return true;
> }
> static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
> {
> 	if (!(sd->flags & SD_ASYM_PACKING))
> 		return false;
> 
> 	if (!sched_smt_active())
> 		return true;
> 
> 	return sd->flags & SD_SHARE_CPUCAPACITY || is_core_idle(cpu);
> }
> 
> On sched_use_asym_prio(), before the idle check a CPU without SMT returns true.
> But I'm actually wrong on that one because on that one we're laying
> on the idle protection outside to the CPU. If there are not any
> other brothers, and I'm idle then my brother-family
> is idle... Ah I messed up describing this.
> 
> But is there are any idle protection outside? I couldn't
> find any. I could be missing something tho.

Actually idle cpu is being checked as a pre-condition for setting
asym_packing.

               /* Check if dst CPU is idle and preferred to this group */
                if (env->idle && sgs->sum_h_nr_running &&
                    sched_group_asym(env, sgs, group))
                        sgs->group_asym_packing = 1;

Are you saying that we should update the first chunk to add an env->idle check
to match the asym_packing migration pre-requisite? Like below?

@@ -10847,6 +10849,10 @@ static enum llc_mig can_migrate_llc_task(struct lb_env *env,
 	if (cpu < 0 || cpus_share_cache(src_cpu, dst_cpu))
 		return mig_unrestricted;
 
+	/* Prioritize asym packing over cache awareness */
+	if (env->idle && sched_asym(env->sd, dst_cpu, src_cpu))
+		return mig_unrestricted;
+  

I think this is a valid point.

Tim

> 
> > > And sched_asym_prefer() comes back true too, so sched_asym() returns true and, we just returned mig_unrestricted.
> > > 
> > > I could be missing something, If I'm not tho is that on purpose? If it is, the last paragraph needs to change
> > > since it says that "asym packing tries to migrate task to an empty core" and after that "higher performing idle core"
> 
> > I think I did try to point out the idle core aspect in my commit log:
> 
> > "When asym packing tries to migrate task to an empty
> > core that has higher priority than source cpu, let asym packing win.
> > Moving tasks to a higher performing idle core will buy more
> > performance than cache co-location."
> 
> I know. I was trying to say that if we're not choosing an idle
> core this needs to change.
> 
> Thanks,
> Kayra
>