linux-kernel - Re: [PATCH] sched/topology: Enable 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] [day] [month] [year] [list]

Message-ID: <20241025055742.GA8206@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date: Thu, 24 Oct 2024 22:57:42 -0700
From: Saurabh Singh Sengar <ssengar@...ux.microsoft.com>
To: Valentin Schneider <vschneid@...hat.com>
Cc: mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
	vincent.guittot@...aro.org, dietmar.eggemann@....com,
	rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	ssengar@...rosoft.com, srivatsa@...il.mit.edu
Subject: Re: [PATCH] sched/topology: Enable topology_span_sane check only for
 debug builds

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@...ux.microsoft.com>
> 
> Please see:
> http://lore.kernel.org/r/20241010155111.230674-1-steve.wahl@hpe.com
> 
> 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