linux-kernel - Re: [PATCH v3] sched/fair: Use sched_domain_span() for topology_span

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fb527d60-f31b-4383-ab03-e17f0ba96a0c@amd.com>
Date: Wed, 9 Jul 2025 09:31:14 +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>,
 Leon Romanovsky <leon@...nel.org>, linux-kernel@...r.kernel.org
Cc: Steve Wahl <steve.wahl@....com>, Borislav Petkov <bp@...en8.de>,
 Dietmar Eggemann <dietmar.eggemann@....com>,
 Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
 Mel Gorman <mgorman@...e.de>
Subject: Re: [PATCH v3] sched/fair: Use sched_domain_span() for
 topology_span_sane()

Hello Valentin,

Thank you for taking a look!

On 7/8/2025 5:14 PM, Valentin Schneider wrote:
>> @@ -2418,22 +2419,34 @@ static bool topology_span_sane(const struct cpumask *cpu_map)
>>                * breaks the linking done for an earlier span.
>>                */
>>               for_each_cpu(cpu, cpu_map) {
>> -			const struct cpumask *tl_cpu_mask = tl->mask(cpu);
>> +			struct sched_domain *sd = *per_cpu_ptr(sdd->sd, cpu);
>> +			struct cpumask *sd_span = sched_domain_span(sd);
>>                       int id;
>>
>> +			/*
>> +			 * If a child level for a CPU has already covered
>> +			 * the entire cpumap, build_sched_domain() for the
>> +			 * domains above is skipped. Use sd->private to detect
>> +			 * levels that have not been initialized in the CPU's
>> +			 * hierarchy and skip them.
>> +			 */
>> +			if (!sd->private)
>> +				continue;
>> +
> 
> So this works, but how about using a cpumask_empty(sd_span) check instead?
> It's IMO a bit more future proof than relying on how sd->private is used.

How about the following instead of cpumask_empty() to avoid two cpumask
operation in the common case where sd is initialized:

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index e682bf991ce6..e889ae012c17 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -2423,19 +2423,13 @@ static bool topology_span_sane(const struct cpumask *cpu_map)
 			struct cpumask *sd_span = sched_domain_span(sd);
 			int id;
 
-			/*
-			 * If a child level for a CPU has already covered
-			 * the entire cpumap, build_sched_domain() for the
-			 * domains above is skipped. Use sd->private to detect
-			 * levels that have not been initialized in the CPU's
-			 * hierarchy and skip them.
-			 */
-			if (!sd->private)
-				continue;
-
 			/* lowest bit set in this mask is used as a unique id */
 			id = cpumask_first(sd_span);
 
+			/* Skip if span is empty */
+			if (id >= nr_cpu_ids)
+				continue;
+
 			if (cpumask_test_cpu(id, id_seen)) {
 				/* First CPU has already been seen, ensure identical sd spans */
 				sd = *per_cpu_ptr(sdd->sd, id);
---

-- 
Thanks and Regards,
Prateek