Re: [PATCH 3/4] sched,cgroup: Fix cpu_cgroup_fork()

From: Peter Zijlstra
Date: Fri Jun 17 2016 - 10:06:25 EST


On Fri, Jun 17, 2016 at 03:58:37PM +0200, Vincent Guittot wrote:
> On 17 June 2016 at 14:01, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > From: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> >
> > A new fair task is detached and attached from/to task_group with:
> >
> > cgroup_post_fork()
> > ss->fork(child) := cpu_cgroup_fork()
> > sched_move_task()
> > task_move_group_fair()
> >
> > Which is wrong, because at this point in fork() the task isn't fully
> > initialized and it cannot 'move' to another group, because its not
> > attached to any group as yet.
> >
> > In fact, cpu_cgroup_fork needs a small part of sched_move_task so we
> > can just call this small part directly instead sched_move_task. And
> > the task doesn't really migrate because it is not yet attached so we
> > need the sequence:
> >
> > do_fork()
> > sched_fork()
> > __set_task_cpu()
> >
> > cgroup_post_fork()
> > set_task_rq() # set task group and runqueue
> >
> > wake_up_new_task()
> > select_task_rq() can select a new cpu
> > __set_task_cpu
> > post_init_entity_util_avg
> > attach_task_cfs_rq()
> > activate_task
> > enqueue_task
> >
> > This patch makes that happen.
> >
>
> With this patch and patch 1, the fork sequence looks correct in my test
>
> > Maybe-Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
>
> You can remove the Maybe if you want

Thanks!