On 3/11/2025 9:02 PM, Valentin Schneider wrote:
On 10/03/25 10:55, Naman Jain wrote:
From: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
On a x86 system under test with 1780 CPUs, topology_span_sane() takes
around 8 seconds cumulatively for all the iterations. It is an expensive
operation which does the sanity of non-NUMA topology masks.
CPU topology is not something which changes very frequently hence make
this check optional for the systems where the topology is trusted and
need faster bootup.
Restrict this to sched_verbose kernel cmdline option so that this penalty
can be avoided for the systems who want to avoid it.
Cc: stable@xxxxxxxxxxxxxxx
Fixes: ccf74128d66c ("sched/topology: Assert non-NUMA topology masks don't (partially) overlap")
Signed-off-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
Co-developed-by: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx>
Tested-by: K Prateek Nayak <kprateek.nayak@xxxxxxx>
---
Changes since v4:
https://lore.kernel.org/all/20250306055354.52915-1-namjain@xxxxxxxxxxxxxxxxxxx/
- Rephrased print statement and moved it to sched_domain_debug.
(addressing Valentin's comments)
Changes since v3:
https://lore.kernel.org/all/20250203114738.3109-1-namjain@xxxxxxxxxxxxxxxxxxx/
- Minor typo correction in comment
- Added Tested-by tag from Prateek for x86
Changes since v2:
https://lore.kernel.org/all/1731922777-7121-1-git-send-email-ssengar@xxxxxxxxxxxxxxxxxxx/
- Use sched_debug() instead of using sched_debug_verbose
variable directly (addressing Prateek's comment)
Changes since v1:
https://lore.kernel.org/all/1729619853-2597-1-git-send-email-ssengar@xxxxxxxxxxxxxxxxxxx/
- Use kernel cmdline param instead of compile time flag.
Adding a link to the other patch which is under review.
https://lore.kernel.org/lkml/20241031200431.182443-1-steve.wahl@xxxxxxx/
Above patch tries to optimize the topology sanity check, whereas this
patch makes it optional. We believe both patches can coexist, as even
with optimization, there will still be some performance overhead for
this check.
---
kernel/sched/topology.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index c49aea8c1025..d7254c47af45 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -132,8 +132,11 @@ static void sched_domain_debug(struct sched_domain *sd, int cpu)
{
int level = 0;
- if (!sched_debug_verbose)
+ if (!sched_debug_verbose) {
+ pr_info_once("%s: Scheduler topology debugging disabled, add 'sched_verbose' to the cmdline to enable it\n",
+ __func__);
return;
+ }
Nit: I've been told not to break warnings over multiple lines so they can
be grep'ed, but given this has the "sched_domain_debug:" prefix I think we
could get away with the below.
Regardless:
Reviewed-by: Valentin Schneider <vschneid@xxxxxxxxxx>
---
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index d7254c47af455..b4dc7c7d2c41c 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -133,7 +133,8 @@ static void sched_domain_debug(struct sched_domain *sd, int cpu)
int level = 0;
if (!sched_debug_verbose) {
- pr_info_once("%s: Scheduler topology debugging disabled, add 'sched_verbose' to the cmdline to enable it\n",
+ pr_info_once("%s: Scheduler topology debugging disabled, "
+ "add 'sched_verbose' to the cmdline to enable it\n",
__func__);
return;
}
Thanks for reviewing. I'll wait a couple more days to push the next
version.
Thanks,
Naman
