sched/topology: Enable topology_span_sane check only for debug builds
Subsystems (2)
PATCH v2 1/1 sched/topology: Enable topology_span_sane check only for debug builds
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 wants to avoid it.
---
[V2]
- Use kernel cmdline param instead of compile time flag.
kernel/sched/topology.c | 7 +++++++ 1 file changed, 7 insertions(+)
kernel/sched/topology.c +7 -0
@@ -2363,6 +2363,13 @@ static bool topology_span_sane(struct sched_domain_topology_level *tl, {int i = cpu +1 ; +/* Skip the topology sanity check for non-debug, as it is a time-consuming operatin */ +if (!sched_debug_verbose) { +pr_info_once ("%s: Skipping topology span sanity check. Use `sched_verbose` boot parameter to enable it.\n" , + __func__); +return true; + } +/* NUMA levels are allowed to overlap */ if (tl->flags &SDTL_OVERLAP )return true;
Signature · 1 line
2.43.0
K Prateek Nayak
reviewer(+ Steve)
Hello Saurabh,
Quoted 3 lines · Saurabh Sengar
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.
Steve too was optimizing this path. I believe his latest version can be found at:
https://lore.kernel.org/lkml/[email protected]/
Does that approach help improving bootup time for you? Valentine suggested the same approach as yours on a previous version of Steve's optimization but Steve believed returning true can possibly have other implication in the sched-domain building path. The thread can be found at:
https://lore.kernel.org/lkml/[email protected]/
Quoted 23 lines
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_verbosekernel cmdline option so that this penalty can be avoided for the systems who wants to avoid it.Fixes ccf74128d66c ("sched/topology: Assert non-NUMA topology masks don't (partially) overlap")Signed-off-by Saurabh Sengar <[email protected]>---
[V2]
- Use kernel cmdline param instead of compile time flag.kernel/sched/topology.c | 7 +++++++ 1 file changed, 7 insertions(+)kernel/sched/topology.c +0
-0
kernel/sched/topology.c static bool topology_span_sane(struct sched_domain_topology_level *tl, · line 2367
Quoted 5 lines
kernel/sched/topology.c +2
-0@@ -2363,6 +2363,13 @@ static bool topology_span_sane(struct sched_domain_topology_level *tl, {int i = cpu +1 ; +/* Skip the topology sanity check for non-debug, as it is a time-consuming operatin */ +if (!sched_debug_verbose) {
nit.
I think the convention in topology.c is to call "sched_debug()" and not check "sched_debug_verbose" directly.
Quoted 8 lines
kernel/sched/topology.c +5
-0+pr_info_once ("%s: Skipping topology span sanity check. Use `sched_verbose` boot parameter to enable it.\n" , + __func__); +return true; + } +/* NUMA levels are allowed to overlap */ if (tl->flags &SDTL_OVERLAP )return true;
Signature · 2 lines
Thanks and Regards, Prateek
Quoted 11 lines · K Prateek Nayak
(+ Steve)
Hello Saurabh,
Quoted 3 lines · Saurabh Sengar
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.Steve too was optimizing this path. I believe his latest version can be found at:
https://lore.kernel.org/lkml/[email protected]/
Yes, Saurabh, I'd be very interested in whether my current patch relieves your situation.
Thanks,
--> Steve Wahl
Signature · 1 line
Steve Wahl, Hewlett Packard Enterprise
Quoted 12 lines · K Prateek Nayak
(+ Steve)
Hello Saurabh,
Quoted 3 lines · Saurabh Sengar
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.Steve too was optimizing this path. I believe his latest version can be found at:
https://lore.kernel.org/lkml/[email protected]/
Quoted 7 lines
Does that approach help improving bootup time for you? Valentine suggested the same approach as yours on a previous version of Steve's optimization but Steve believed returning true can possibly have other implication in the sched-domain building path. The thread can be found at:
https://lore.kernel.org/lkml/[email protected]/
I see that Steve's patch focuses on optimizing the topology sanity check, whereas my patch aims to make it optional. I believe both patches can coexist, as even with optimization, there will still be some performance overhead for this check. My goal is to provide an option to bypass it if desired. Please do check my discussion with Valentin on V1.
Quoted 30 lines
Quoted 23 lines
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_verbosekernel cmdline option so that this penalty can be avoided for the systems who wants to avoid it.Fixes ccf74128d66c ("sched/topology: Assert non-NUMA topology masks don't (partially) overlap")Signed-off-by Saurabh Sengar <[email protected]>---
[V2]
- Use kernel cmdline param instead of compile time flag.kernel/sched/topology.c | 7 +++++++ 1 file changed, 7 insertions(+)kernel/sched/topology.c +0
-0kernel/sched/topology.c static bool topology_span_sane(struct sched_domain_topology_level *tl, · line 2366
Quoted 4 lines
kernel/sched/topology.c +2
-0@@ -2363,6 +2363,13 @@ static bool topology_span_sane(struct sched_domain_topology_level *tl, {int i = cpu +1 ; +/* Skip the topology sanity check for non-debug, as it is a time-consuming operatin */ +if (!sched_debug_verbose) {nit.
Quoted 2 lines
I think the convention in
topology.cis to call "sched_debug()" and not check "sched_debug_verbose" directly.
I will add this in next version
It would be greatly appreciated if a maintainer could share their thoughts on these two patches aimed at enhancing the performance of large-scale machines.
- Saurabh