[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <47c5f6eb-1622-4aeb-b9ff-6ebf63cc8097@amd.com>
Date: Fri, 6 Feb 2026 10:50:53 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Valentin Schneider <vschneid@...hat.com>, Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>, <linux-kernel@...r.kernel.org>
CC: Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt
<rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Mel Gorman
<mgorman@...e.de>, Chen Yu <yu.c.chen@...el.com>, Shrikanth Hegde
<sshegde@...ux.ibm.com>, "Gautham R. Shenoy" <gautham.shenoy@....com>
Subject: Re: [PATCH v3 3/8] sched/topology: Switch to assigning "sd->shared"
from s_data
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
Powered by blists - more mailing lists