Date: Mon, 3 Oct 2016 14:49:07 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Matt Fleming <matt@codeblueprint.co.uk>
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: <20161003124907.GD3142@twins.programming.kicks-ass.net>
References: <20160921133813.31976-1-matt@codeblueprint.co.uk>
 <20160921133813.31976-2-matt@codeblueprint.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160921133813.31976-2-matt@codeblueprint.co.uk>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 856
Lines: 21

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.


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.