Re: [PATCH v6 1/4] sched/fair: Fix attaching task sched avgs twice when switching to fair or changing task group

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Jun 16, 2016 at 11:21:55PM +0200, Vincent Guittot wrote:
> On 16 June 2016 at 22:07, Peter Zijlstra <peterz@infradead.org> wrote:

> > So I don't see how it can end up being attached again.
> 
> In fact it has already been attached during the sched_move_task. The
> sequence for the 1st task that is attached to a cfs_rq is :
> 
> sched_move_task()
>   task_move_group_fair()
>     detach_task_cfs_rq()
>     set_task_rq()
>     attach_task_cfs_rq()
>       attach_entity_load_avg()
>         se->avg.last_load_update = cfs_rq->avg.last_load_update == 0
> 
> Then we enqueue the task

> > enqueue_entity()
> >   enqueue_entity_load_avg()
> >     update_cfs_rq_load_avg()
> >       now = clock()
> >       __update_load_avg(&cfs_rq->avg)
> >         cfs_rq->avg.last_load_update = now
> >         // ages 0 load/util for: now - 0
> >     if (migrated)
> >       attach_entity_load_avg()
> >         se->avg.last_load_update = cfs_rq->avg.last_load_update; // now != 0

> but se->avg.last_load_update is still 0 so
> migrated is set and we attach the task one more time

Ok, we indeed attach twice here, however I don't actually see it be a
problem (much). Because the aging during the enqueue will age the entire
first attach right back down to 0 again; now-0 is a _huge_ delta and
will flatten everything right down to 0.

Of course, that also ages the new entity right back down to 0, making it
appear like it has no load what so ever.

But yes, /me puts in changelog.