Re: [PATCH 3/8] sched/fair: remove redundant cpu_cgrp_subsys->fork()

From: Peter Zijlstra
Date: Mon Jul 11 2022 - 03:35:31 EST


On Sat, Jul 09, 2022 at 11:13:48PM +0800, Chengming Zhou wrote:
> We use cpu_cgrp_subsys->fork() to set task group for the new fair task
> in cgroup_post_fork().
>
> Since commit b1e8206582f9 ("sched: Fix yet more sched_fork() races")
> has already set task group for the new fair task in sched_cgroup_fork(),
> so cpu_cgrp_subsys->fork() can be removed.
>
> cgroup_can_fork() --> pin parent's sched_task_group
> sched_cgroup_fork()
> __set_task_cpu --> set task group
> cgroup_post_fork()
> ss->fork() := cpu_cgroup_fork() --> set again
>
> After this patch's change, task_change_group_fair() only need to
> care about task cgroup migration, make the code much simplier.

This:

> This patch also move the task se depth setting to set_task_rq(), which
> will set correct depth for the new task se in sched_cgroup_fork().
>
> The se depth setting in attach_entity_cfs_rq() is removed since
> set_task_rq() is a better place to do this when task moves across
> CPUs/groups.

really should have been it's own patch. And this actually scares me. Did
you test with priority inheritance bumping the task to FIFO while things
change?

This has nothing to do with fork().