linux-kernel - Re: [PATCH 4/4] sched,fair: Fix PELT integrity for new tasks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 23 Jun 2016 19:15:11 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Dietmar Eggemann <dietmar.eggemann@....com>
Cc:	Vincent Guittot <vincent.guittot@...aro.org>,
	Yuyang Du <yuyang.du@...el.com>,
	Ingo Molnar <mingo@...nel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Mike Galbraith <umgwanakikbuti@...il.com>,
	Benjamin Segall <bsegall@...gle.com>,
	Paul Turner <pjt@...gle.com>,
	Morten Rasmussen <morten.rasmussen@....com>,
	Matt Fleming <matt@...eblueprint.co.uk>
Subject: Re: [PATCH 4/4] sched,fair: Fix PELT integrity for new tasks

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.