sched/topology: Enable topology_span_sane check only for debug builds
Subsystems (2)
PATCH 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_DEBUG builds so that this penalty can be avoided for the systems who wants to avoid it.
--- kernel/sched/topology.c | 4 ++++ 1 file changed, 4 insertions(+)
kernel/sched/topology.c +4 -0
@@ -2354,6 +2354,7 @@ static struct sched_domain *build_sched_domain(struct sched_domain_topology_levereturn sd; } +#ifdef CONFIG_SCHED_DEBUG /* * Ensure topology masks are sane, i.e. there are no conflicts (overlaps) for * any two given CPUs at this (non-NUMA) topology level.
@@ -2387,6 +2388,7 @@ static bool topology_span_sane(struct sched_domain_topology_level *tl, return true; } +#endif /* * Build sched domains for a given set of CPUs and attach the sched domains
@@ -2417,8 +2419,10 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att sd =NULL ;for_each_sd_topology (tl) { +#ifdef CONFIG_SCHED_DEBUG if (WARN_ON (!topology_span_sane (tl, cpu_map, i))) goto error; +#endif sd =build_sched_domain (tl, cpu_map, attr, sd, i);
Signature · 1 line
2.43.0
kernel test robot
build botFYI: kernel test robot notices the stable kernel rule is not satisfied.
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1
Rule: add the tag "Cc: [email protected]" in the sign-off area to have the patch automatically included in the stable tree.
Subject: [PATCH] sched/topology: Enable topology_span_sane check only for debug builds
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Valentin Schneider
reviewerQuoted 8 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.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.
Quoted 5 lines
Restrict this to
SCHED_DEBUGbuilds 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]>
Please see:
http://lore.kernel.org/r/[email protected]
Also note that most distros ship with CONFIG_SCHED_DEBUG=y, so while I'm not 100% against it this would at the very least need to be gated behind e.g. the sched_verbose cmdline argument to be useful.
But before that I'd like the "just run it once" option to be explored first.
Quoted 15 lines · Valentin Schneider
Quoted 8 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.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.
Quoted 5 lines
Restrict this to
SCHED_DEBUGbuilds 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]>
Quoted 6 lines
Please see:
http://lore.kernel.org/r/[email protected]Also note that most distros ship with
CONFIG_SCHED_DEBUG=y, so while I'm not 100% against it this would at the very least need to be gated behind e.g. thesched_verbosecmdline argument to be useful.
Thanks for your review. I thought of using sched_verbose first, but I assumed that many systems might not be using this command line option and I didn't want them to have change in behaviour after my patch.
But if you think this is the right approach, I can send the V2.
Quoted 3 lines
But before that I'd like the "just run it once" option to be explored first.
That's a great improvement, but I understand there will still be a linear penalty to pay for this sanity check. In my opinion, regardless of whether these improvements are accepted or not, we should make this sanity check optional.
- Saurabh