From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1756720AbcFHNQH (ORCPT <rfc822;w@1wt.eu>);
	Wed, 8 Jun 2016 09:16:07 -0400
Received: from mail-wm0-f66.google.com ([74.125.82.66]:33015 "EHLO
	mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753656AbcFHNQD (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 8 Jun 2016 09:16:03 -0400
Date: Wed, 8 Jun 2016 15:15:59 +0200
From: Ingo Molnar <mingo@kernel.org>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>,
        linux-kernel@vger.kernel.org, Mel Gorman <mgorman@techsingularity.net>,
        Matt Fleming <matt@codeblueprint.co.uk>,
        Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Subject: Re: [PATCH 2/2] sched/debug: fix deadlock when enabling sched events
Message-ID: <20160608131559.GA15843@gmail.com>
References: <cover.1465322027.git.jpoimboe@redhat.com>
 <50a0f7f2a5f8d69e0440c3430f06be7ccbe59903.1465322027.git.jpoimboe@redhat.com>
 <20160608075612.GA9369@gmail.com>
 <20160608131237.3tixvrtpqvfv5kmg@treble>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160608131237.3tixvrtpqvfv5kmg@treble>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Wed, Jun 08, 2016 at 09:56:12AM +0200, Ingo Molnar wrote:
> > 
> > * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -789,6 +789,13 @@ static void update_curr_fair(struct rq *rq)
> > >  	update_curr(cfs_rq_of(&rq->curr->se));
> > >  }
> > >  
> > > +void trace_sched_stat_register(void)
> > > +{
> > > +#ifdef CONFIG_SCHEDSTATS
> > > +	force_schedstat_enabled();
> > > +#endif
> > > +}
> > 
> > I think it would be cleaner to provide an empty force_schedstat_enabled() 
> > definition in sched.h, on !CONFIG_SCHEDSTATS.
> 
> Yes, agreed.
> 
> > But it might make sense to further decouple schedstats from tracing?
> 
> Looking at how we could do that:
> 
> - The sched_stat_wait tracepoint is dependent on the wait_start schedstat.

Can we maintain wait_start unconditionally? Having config dependent tracepoints 
sucks.

> - The sched_stat_sleep tracepoint is dependent on the sleep_start
>   schedstat.

Ditto.

> - The sched_stat_iowait and sched_stat_blocked tracepoints are dependent
>   on the block_start schedstat.

Ditto.

> We could move those three schedstats values out of sched_statistics and
> into sched_entity, and then always update them regardless of
> CONFIG_SCHEDSTATS.
> 
> That would ensure these tracepoints always work.  But then again it
> would add a little bit of runtime overhead.
> 
> I don't have a strong opinion either way.  Thoughts?

Please do a before/after 'size vmlinux' dump of a defconfig build with tracing 
disabled. I.e. how much code overhead does the updating of those variables add?

Thanks,

	Ingo