Re: [PATCH 1/5] sched/fair: Attach sched_domain_shared to sd_asym_cpucapacity

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Andrea,

On 4/24/2026 2:16 PM, Andrea Righi wrote:
>>>  	rcu_assign_pointer(per_cpu(sd_llc, cpu), sd);
>>>  	per_cpu(sd_llc_size, cpu) = size;
>>>  	per_cpu(sd_llc_id, cpu) = id;
>>> +
>>> +	/* TODO: Rename sd_llc_shared to fit the new role. */
>>>  	rcu_assign_pointer(per_cpu(sd_llc_shared, cpu), sds);
>>
>> Would love for folks to chime in but IMO "sd_wakeup_shared" sounds
>> pretty reasonable since it is mainly the wakeup path that depends on
>> this except for one !ASYM load balancing trigger.
> 
> sd_wakeup_shared captures the bigger consumer (wakeup), but not the nohz
> balancer kick logic.
> 
> Maybe "sd_balance_shared" (balance in a broad sense, wakeup is still affecting
> balancing at the end) or "sd_effective_shared" (if we want to stress that
> topology may move: LLC vs asym)?

Works for me! I don't have any strong feelings on this.

[..snip..]

>>> +		/*
>>> +		 * In case of ASYM_CPUCAPACITY, attach sd->shared to
>>> +		 * sd_asym_cpucapacity for wakeup stat tracking.
>>> +		 *
>>> +		 * Caveats:
>>> +		 *
>>> +		 * 1) has_asym is system-wide, but a given CPU may still
>>> +		 *    lack an SD_ASYM_CPUCAPACITY_FULL ancestor (e.g., an
>>> +		 *    exclusive cpuset carving out a symmetric capacity island).
>>> +		 *    Such CPUs must fall through to the LLC seeding path below.
>>> +		 *
>>> +		 * 2) Skip the asym attach if the asym ancestor is an
>>> +		 *    overlapping domain (SD_NUMA). On those topologies let the
>>> +		 *    LLC path own the shared object instead.
>>> +		 *
>>> +		 * XXX: This assumes SD_ASYM_CPUCAPACITY_FULL domain
>>> +		 * always has more than one group else it is prone to
>>> +		 * degeneration.
>>
>> I looked into this and we only set SD_ASYM_CPUCAPACITY if we find more
>> than one capacity and SD_ASYM_CPUCAPACITY_FULL implies there are atleast
>> two CPUs covering differnt capcities in the span.
>>
>> The very first SD_ASYM_CPUCAPACITY_FULL domain should be safe from
>> degeneration when it is non-overlapping.
> 
> Makes sense, maybe we can replace the XXX part with note like this:
> 
>  * Note: SD_ASYM_CPUCAPACITY_FULL is only set when multiple distinct
>  * capacities exist in the domain span, so the asym domain we attach
>  * to cannot degenerate into a single-capacity group. The relevant
>  * edge cases are instead covered by the caveats above.

Ack! That should make it clear. Thank you.

[..snip..]

>>>  		if (sd->flags & SD_SHARE_LLC) {
>>> -			int sd_id = cpumask_first(sched_domain_span(sd));
>>> -
>>> -			sd->shared = *per_cpu_ptr(d.sds, sd_id);
>>> -			atomic_set(&sd->shared->nr_busy_cpus, sd->span_weight);
>>> -			atomic_inc(&sd->shared->ref);
>>> +			/*
>>> +			 * Initialize the sd->shared for SD_SHARE_LLC unless
>>> +			 * the asym path above already claimed it.
>>> +			 */
>>> +			if (!asym_claimed)
>>> +				init_sched_domain_shared(&d, sd);
>>
>> Tbh, if "has_asym" is true, we probabaly don't even need this since the
>> nr_busy_cpus accounting gets us nothing.
>>
>> Might save a little overhead and space on those systems but I would
>> love to hear if there are any concerns if we just drop the
>> sd_llc->shared when we detect asym capacities.
> 
> Hm... but "has_asym" is global, we may still need LLC-owned shared for symmetric
> islands and NUMA-overlap cases, no?

"has_asym" is local to build_sched_domains() right? So it should operate
per cpuset partition since we call build_sched_domains() for every
"cpu_map".

Ack to everything that was snipped off!

-- 
Thanks and Regards,
Prateek