linux-kernel - Re: [PATCH v2 04/10] sched/fair: remove redundant cpu_cgrp

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, 14 Jul 2022 14:31:37 +0200
From:   Dietmar Eggemann <dietmar.eggemann@....com>
To:     Chengming Zhou <zhouchengming@...edance.com>, mingo@...hat.com,
        peterz@...radead.org, vincent.guittot@...aro.org,
        rostedt@...dmis.org, bsegall@...gle.com, vschneid@...hat.com
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 04/10] sched/fair: remove redundant
 cpu_cgrp_subsys->fork()

On 13/07/2022 06:04, 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()

Don't we set the task group (`p->sched_task_group = tg`) now directly in
sched_cgroup_fork() and not in __set_task_cpu()?

>     __set_task_cpu	--> set task group
>   cgroup_post_fork()
>     ss->fork() := cpu_cgroup_fork()	--> set again

nit pick:

cpu_cgroup_fork() -> sched_change_group(..., TASK_SET_GROUP)

> After this patch's change, task_change_group_fair() only need to
> care about task cgroup migration, make the code much simplier.
> 
> Signed-off-by: Chengming Zhou <zhouchengming@...edance.com>
> Reviewed-by: Vincent Guittot <vincent.guittot@...aro.org>

LGTM too.

Reviewed-by: Dietmar Eggemann <dietmar.eggemann@....com>

[...]