From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757692AbcAYPq4 (ORCPT ); Mon, 25 Jan 2016 10:46:56 -0500 Received: from mail-wm0-f43.google.com ([74.125.82.43]:33960 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757582AbcAYPqj (ORCPT ); Mon, 25 Jan 2016 10:46:39 -0500 Date: Mon, 25 Jan 2016 16:46:35 +0100 From: Ingo Molnar To: Peter Zijlstra Cc: Mel Gorman , Matt Fleming , Mike Galbraith , LKML Subject: Re: [PATCH] sched: Make schedstats a runtime tunable that is disabled by default Message-ID: <20160125154635.GB4031@gmail.com> 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=us-ascii 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 * 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. 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? IIRC it needed only a small subset of schedstats to make those tracepoints work. We already have too much overhead in the scheduler as-is - and the extra cache footprint does not even show on the typically cache-rich enterprise CPUs most of the scalability testing goes on. My minimum requirement for such runtime enablement would be to make it entirely static-branch patched and triggered at the call sites as well - not hidden inside schedstat functions. Thanks, Ingo