From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757578AbcAYRF2 (ORCPT ); Mon, 25 Jan 2016 12:05:28 -0500 Received: from outbound-smtp05.blacknight.com ([81.17.249.38]:60620 "EHLO outbound-smtp05.blacknight.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757095AbcAYRF0 (ORCPT ); Mon, 25 Jan 2016 12:05:26 -0500 Date: Mon, 25 Jan 2016 17:05:17 +0000 From: Mel Gorman To: Peter Zijlstra Cc: Ingo Molnar , Matt Fleming , Mike Galbraith , LKML Subject: Re: [PATCH] sched: Make schedstats a runtime tunable that is disabled by default Message-ID: <20160125170517.GH3162@techsingularity.net> References: <1453716331-21029-1-git-send-email-mgorman@techsingularity.net> <20160125112606.GV6357@twins.programming.kicks-ass.net> <20160125133944.GE3162@techsingularity.net> <20160125145944.GZ6357@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20160125145944.GZ6357@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 25, 2016 at 03:59:44PM +0100, Peter Zijlstra wrote: > 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. This would also be misleading. Once enabled, the stats start being updated. An already sleeping process will not have wait_start set so the trace information for wakeups will initially be completely bogus. > Or alternatively fail to enable the > tracepoints (which would then get us people going: 'WTF this used to > work'). > Each option is at least visible to some extent so there would be a period of time of wtf for analysing scheduler performance. I'm not sure there is a way of failing to set a tracepoint but I'll check it out. > 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. > I don't think many are required but some of them are expensive to keep track of. Look at enqueue_sleeper as an example of the amount of work required just to have the tracepoint available. -- Mel Gorman SUSE Labs