linux-kernel - Re: [PATCH] sched: Make schedstats a runtime tunable that is disabled by default

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160125145944.GZ6357@twins.programming.kicks-ass.net>
Date:	Mon, 25 Jan 2016 15:59:44 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Mel Gorman <mgorman@...hsingularity.net>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Matt Fleming <matt@...eblueprint.co.uk>,
	Mike Galbraith <mgalbraith@...e.de>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sched: Make schedstats a runtime tunable that is
 disabled by default

On Mon, Jan 25, 2016 at 01:39:44PM +0000, Mel Gorman wrote:
> On Mon, Jan 25, 2016 at 12:26:06PM +0100, Peter Zijlstra wrote:
> > On Mon, Jan 25, 2016 at 10:05:31AM +0000, Mel Gorman wrote:
> > > schedstats is very useful during debugging and performance tuning but it
> > > incurs overhead. As such, even though it can be disabled at build time,
> > > it is often enabled as the information is useful.  This patch adds a
> > > kernel command-line and sysctl tunable to enable or disable schedstats on
> > > demand. It is disabled by default as someone who knows they need it can
> > > also learn to enable it when necessary.
> > 
> > So the reason its often enabled in distro configs is (IIRC) that it
> > enables trace_sched_stat_{wait,sleep,iowait,blocked}().
> > 
> > I've not looked at the details of this patch, but I suspect this patch
> > would make these tracepoints available but non-functional unless you
> > poke the magic button.
> > 
> 
> It's potentially slightly worse than that. The tracepoints are available,
> functional but produce garbage unless the magic button is poked and do
> a lot of useful work producing that garbage. I missed a few hunks that
> are included below. With this, the tracepoints will exist but unless the
> magic button is poked, they'll never fire. Considering the paths
> affected, this will require retesting but if it's ok, would you be ok in
> general with a patch like this that forces a button to be pushed if
> the user is doing performance analysis?

Its rather unintuitive and error prone semantics :/

Ideally we'd auto-magically enable the magic knob if any of these
affected tracepoints become active. Or alternatively fail to enable the
tracepoints (which would then get us people going: 'WTF this used to
work').

One of the things on my TODO is look at how much of sched_stat is
required for these tracepoints and see if we can enable just that
(hopefully) little bit, while not doing the rest of the accounting.

Of course, it'll be our luck that tracking the data for these
tracepoints is the most expensive part of schedstats ...

Ingo?