Re: [PATCH] sched/topology: Enable topology_span_sane check only for debug builds

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 23, 2024 at 06:39:37PM +0200, Valentin Schneider wrote:
> On 22/10/24 10:57, Saurabh Sengar wrote:
> > 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.
> >
> > Fixes: ccf74128d66c ("sched/topology: Assert non-NUMA topology masks don't (partially) overlap")
> > Signed-off-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
> 
> Please see:
> http://lore.kernel.org/r/20241010155111.230674-1-steve.wahl@xxxxxxx
> 
> 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.

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.

> 
> 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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux