Re: [PATCH v3 3/8] sched/topology: Switch to assigning "sd->shared" from s_data

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

Hello Valentin,

On 2/5/2026 10:23 PM, Valentin Schneider wrote:
> On 20/01/26 11:32, K Prateek Nayak wrote:
>> @@ -2655,8 +2655,19 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
>>               unsigned int imb_span = 1;
>>
>>               for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
>> +			struct sched_domain *parent = sd->parent;
>>                       struct sched_domain *child = sd->child;
>>
>> +			/* Attach sd->shared to the topmost SD_SHARE_LLC domain. */
>> +			if ((sd->flags & SD_SHARE_LLC) &&
>> +			    (!parent || !(parent->flags & SD_SHARE_LLC))) {
>> +				int llc_id = cpumask_first(sched_domain_span(sd));
>> +
>> +				sd->shared = *per_cpu_ptr(d.sds, llc_id);
>> +				atomic_set(&sd->shared->nr_busy_cpus, sd->span_weight);
>> +				atomic_inc(&sd->shared->ref);
>> +			}
>> +
> 
> We now have two if's looking for the highest_flag_domain(i, SD_SHARE_LLC),
> but given this needs to write the sd->imb_numa_nr for every SD I couldn't
> factorize this into something that looked sane :(

Yeah! The "imb_numa_nr" cares about the "sd_llc" *after* we've crossed
it and "sd->shared" assignment cares when we are *at* the sd_llc.

Since we have to assign the "sd->shared" before claim_allocations(),
I couldn't find a better spot to assign it.

That said, "imb_numa_nr" calculation can be modified to use the "sd_llc"
and its "parent". I'll let you be the judge of whether the following is
better or worse ;-)

  (Only build tested)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index ac268da91778..e98bb812de35 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -2614,13 +2614,23 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
 		unsigned int imb_span = 1;
 
 		for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
-			struct sched_domain *child = sd->child;
+			struct sched_domain *parent = sd->parent;
 
-			if (!(sd->flags & SD_SHARE_LLC) && child &&
-			    (child->flags & SD_SHARE_LLC)) {
-				struct sched_domain __rcu *top_p;
+			/* Topmost SD_SHARE_LLC domain. */
+			if ((sd->flags & SD_SHARE_LLC) &&
+			    (!parent || !(parent->flags & SD_SHARE_LLC))) {
+				int sd_id = cpumask_first(sched_domain_span(sd));
+				struct sched_domain *top_p;
 				unsigned int nr_llcs;
 
+				sd->shared = *per_cpu_ptr(d.sds, sd_id);
+				atomic_set(&sd->shared->nr_busy_cpus, sd->span_weight);
+				atomic_inc(&sd->shared->ref);
+
+				/* No SD_NUMA domains. */
+				if (!parent)
+					break;
+
 				/*
 				 * For a single LLC per node, allow an
 				 * imbalance up to 12.5% of the node. This is
@@ -2641,7 +2651,7 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
 				 * factors and that there is a correlation
 				 * between LLCs and memory channels.
 				 */
-				nr_llcs = sd->span_weight / child->span_weight;
+				nr_llcs = parent->span_weight / sd->span_weight;
 				if (nr_llcs == 1)
 					imb = sd->span_weight >> 3;
 				else
@@ -2650,11 +2660,11 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
 				sd->imb_numa_nr = imb;
 
 				/* Set span based on the first NUMA domain. */
-				top_p = sd->parent;
+				top_p = parent;
 				while (top_p && !(top_p->flags & SD_NUMA)) {
 					top_p = top_p->parent;
 				}
-				imb_span = top_p ? top_p->span_weight : sd->span_weight;
+				imb_span = top_p ? top_p->span_weight : parent->span_weight;
 			} else {
 				int factor = max(1U, (sd->span_weight / imb_span));
 
---

> 
>>                       if (!(sd->flags & SD_SHARE_LLC) && child &&
>>                           (child->flags & SD_SHARE_LLC)) {
>>                               struct sched_domain __rcu *top_p;
> 

-- 
Thanks and Regards,
Prateek