Date: Mon, 3 Oct 2016 15:37:45 +0100
From: Matt Fleming <matt@codeblueprint.co.uk>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>,
        Byungchul Park <byungchul.park@lge.com>,
        Frederic Weisbecker <fweisbec@gmail.com>,
        Luca Abeni <luca.abeni@unitn.it>, Rik van Riel <riel@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Wanpeng Li <wanpeng.li@hotmail.com>, Yuyang Du <yuyang.du@intel.com>,
        Petr Mladek <pmladek@suse.com>, Jan Kara <jack@suse.cz>,
        Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
        linux-kernel@vger.kernel.org, Mel Gorman <mgorman@techsingularity.net>,
        Mike Galbraith <umgwanakikbuti@gmail.com>
Subject: Re: [PATCH v2 1/7] sched/fair: Update the rq clock before detaching
 tasks
Message-ID: <20161003143745.GG16071@codeblueprint.co.uk>
References: <20160921133813.31976-1-matt@codeblueprint.co.uk>
 <20160921133813.31976-2-matt@codeblueprint.co.uk>
 <20161003124907.GD3142@twins.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20161003124907.GD3142@twins.programming.kicks-ass.net>
User-Agent: Mutt/1.5.24+41 (02bc14ed1569) (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 977
Lines: 23

On Mon, 03 Oct, at 02:49:07PM, Peter Zijlstra wrote:
> On Wed, Sep 21, 2016 at 02:38:07PM +0100, Matt Fleming wrote:
> > detach_task_cfs_rq() may indirectly call rq_clock() to inform the
> > cpufreq code that the rq utilisation has changed. In which case, we
> > need to update the rq clock.
> 
> Hurm,. so it would've been good to know the callchain that got you
> there.
> 
> There's two functions that use detach_task_cfs_rq(), one is through
> sched_change_group() and that does indeed lack a rq_clock update.
> 
> The other is through switched_from() where its far harder (but still
> possible afaict) to miss the update.
 
It was the former callchain.

> Now, neither cases are really fast paths, but it would be good to try
> and avoid too many update_rq_clock() calls in the same rq-lock section.
> So I'm not entirely sure about the placement here.
> 
> But let me go stare at the actual debug framework thing first.. I think
> this patch is fallout/fixups from that.