linux-kernel - Re: [PATCH 3/8] 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]

Message-ID: <YsvSpyrJxNv7jsQz@hirez.programming.kicks-ass.net>
Date:   Mon, 11 Jul 2022 09:35:03 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Chengming Zhou <zhouchengming@...edance.com>
Cc:     mingo@...hat.com, vincent.guittot@...aro.org,
        dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
        vschneid@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/8] sched/fair: remove redundant cpu_cgrp_subsys->fork()

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().