Re: [PATCH 2/2] sched/fair: Skip detach and attach load avgs for new group task

[Vincent Guittot]

On 26 May 2016 at 21:44, Yuyang Du <yuyang.du@xxxxxxxxx> wrote:
> Hi Vincent,
>
> On Thu, May 26, 2016 at 01:50:56PM +0200, Vincent Guittot wrote:
>> On 26 May 2016 at 03:14, Yuyang Du <yuyang.du@xxxxxxxxx> wrote:
>> > Vincent reported that the first task to a new task group's cfs_rq will
>> > be attached in attach_task_cfs_rq() and once more when it is enqueued
>> > (see https://lkml.org/lkml/2016/5/25/388).
>> >
>> > Actually, it is worse, attach_task_cfs_rq() is called for new task even
>> > way before init_entity_runnable_average().
>> >
>> > Solve this by avoiding attach as well as detach new task's sched avgs
>> > in task_move_group_fair(). To do it, we need to know whether the task
>> > is forked or not, so we pass this info all the way from sched_move_task()
>> > to attach_task_cfs_rq().
>>
>> Not sure that this is the right way to solve this problem because you
>> continue to attach the task twice without detaching it in the mean
>> time:
>> - once during the copy of the process in cpu_cgroup_fork (you skip the
>> attach of load average but the task is still attached to the local
>> cpu)
>
> Sorry, the task's what is still attached, and how? You mean the vruntime
> thingy? But the load/util avgs are not.

yes that's it. The sequence still looks weird IMHO.
the detach is called for a newly forked task that is not fully init
and has not been attached yet
IIUC the fork sequence, we only need to set group at this point so you
can skip completely the detach/attach_task_cfs_rq not only the
detach/attach_entity_load_avg

>
>> In the mean time, sched_entity is initialized and the last_update_time is reset
>
> last_update_time is set to 0 in initialization, and this is the first time
> it is touched, no?
>
>> - one more time when the task is enqueued because the last_update_time
>> has been reset (this time you don't skip the attache of load_avg
>
> This is expected/wanted. We don't skip this because this will be the first-time
> attach.
>
>> Should you better detach the sched_entity with a copy of its parent
>> metrics before initializing it and attaching it to the new cpu ?
>
> Thanks,
> Yuyang