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 S1751463AbcFWRPW (ORCPT <rfc822;w@1wt.eu>);
	Thu, 23 Jun 2016 13:15:22 -0400
Received: from merlin.infradead.org ([205.233.59.134]:49294 "EHLO
	merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750785AbcFWRPV (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 23 Jun 2016 13:15:21 -0400
Date: Thu, 23 Jun 2016 19:15:11 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>,
        Yuyang Du <yuyang.du@intel.com>, Ingo Molnar <mingo@kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Mike Galbraith <umgwanakikbuti@gmail.com>,
        Benjamin Segall <bsegall@google.com>, Paul Turner <pjt@google.com>,
        Morten Rasmussen <morten.rasmussen@arm.com>,
        Matt Fleming <matt@codeblueprint.co.uk>
Subject: Re: [PATCH 4/4] sched,fair: Fix PELT integrity for new tasks
Message-ID: <20160623171511.GM30154@twins.programming.kicks-ass.net>
References: <20160617120454.150630859@infradead.org>
 <CAKfTPtAUtteP0V5-1u2n0YFDdFbZfwSqigHjOfM32Vbw3iAPbg@mail.gmail.com>
 <20160617142814.GT30154@twins.programming.kicks-ass.net>
 <20160617160239.GL30927@twins.programming.kicks-ass.net>
 <20160617161831.GM30927@twins.programming.kicks-ass.net>
 <5767D51F.3080600@arm.com>
 <CAKfTPtBB3jyOWrv8=+S2zRXD+pSXPWgoZ3aJUscE5yJDpkDmzw@mail.gmail.com>
 <5768027E.1090408@arm.com>
 <20160621084119.GN30154@twins.programming.kicks-ass.net>
 <576C01DA.2050406@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <576C01DA.2050406@arm.com>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Jun 23, 2016 at 04:35:54PM +0100, Dietmar Eggemann wrote:
> > The things we ran into with these patches were that:
> > 
> >  1) You need to update the cfs_rq _before_ any entity attach/detach
> >     (and might need to update_tg_load_avg when update_cfs_rq_load_avg()
> >     returns true).
> > 
> >  2) (fair) entities are always attached, switched_from/to deal with !fair.
> > 
> >  3) cpu migration is the only exception and uses the last_update_time=0
> >     thing -- because refusal to take second rq->lock.
> 
> 2) is about changing sched classes, 3) is about changing cpus but what
> about 4) changing task groups?
> 
> There is still this last_update_time = 0 between
> detach_task_cfs_rq()/set_task_rq() and attach_task_cfs_rq() in
> task_move_group_fair() preventing the call __update_load_avg(...
> p->se->avg, ...) in attach_task_cfs_rq() -> attach_entity_load_avg().
> 
> Shouldn't be necessary any more since cfs_rq 'next' is up-to-date now.
> 
> Assuming here that the exception in 3) relates to the fact that the
> rq->lock is not taken.
> 
> Or is 4) a second exception in the sense that the se has been aged in
> remove_entity_load_avg() (3)) resp. detach_entity_load_avg() (4))?

Ah, good point, so move between groups is also special in that the time
between cgroups doesn't need to match.

And since we've aged the se to 'now' on detach, we can assume its
up-to-date (and hence last_update_time=0) for attach, which then only
updates its cfs_rq to 'now' before attaching the se.