linux-kernel - Re: [PATCH v10 7/8] cgroup/cpuset: Update description of cpuset.cpus.partition in cgroup-v2.rst

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: <20220504112552.GA15266@blackbody.suse.cz>
Date:   Wed, 4 May 2022 13:25:52 +0200
From:   Michal Koutný <mkoutny@...e.com>
To:     Waiman Long <longman@...hat.com>
Cc:     Tejun Heo <tj@...nel.org>, Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Jonathan Corbet <corbet@....net>,
        Shuah Khan <shuah@...nel.org>, cgroups@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kselftest@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Roman Gushchin <guro@...com>, Phil Auld <pauld@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Frederic Weisbecker <frederic@...nel.org>,
        Marcelo Tosatti <mtosatti@...hat.com>
Subject: Re: [PATCH v10 7/8] cgroup/cpuset: Update description of
 cpuset.cpus.partition in cgroup-v2.rst

Hello.

On Tue, May 03, 2022 at 12:21:48PM -0400, Waiman Long <longman@...hat.com> wrote:
>  Documentation/admin-guide/cgroup-v2.rst | 145 +++++++++++++-----------
>  1 file changed, 79 insertions(+), 66 deletions(-)

A note across various lines -- it seems your new text accidentally mixes
both spaces and tabs for indentation.

> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> index 69d7a6983f78..94e1e3771830 100644
> --- a/Documentation/admin-guide/cgroup-v2.rst
> +++ b/Documentation/admin-guide/cgroup-v2.rst
> [...]
> +	The value shown in "cpuset.cpus.effective" of a partition root is
> +	the CPUs that the parent partition root can dedicate to the new
> +	partition root.  They are subtracted from "cpuset.cpus.effective"
> +	of the parent and may be different from "cpuset.cpus"

I find this paragraph a bit hard to comprehend (I read it as it talks
about three levels of cgroups (parent, child, grandparent). It is
correct but I'd suggect following formulation (where I additionally
simplifed it by talking about "available" cpus):

> The value shown in "cpuset.cpus.effective" of a partition root is
> the CPUs that the partition root can dedicate to a potential new child
> partition root. The new child subtracts available CPUs from its parent
> "cpuset.cpus.effective".


> +	For a partition root to become valid, the following conditions
> +	must be met.
> +
> +	1) The "cpuset.cpus" is exclusive, i.e. they are not shared by
> +	   any of its siblings (exclusivity rule).
> +	2) The parent cgroup is a valid partition root.
> +	3) The "cpuset.cpus" is not empty and must contain at least
> +	   one of the CPUs from parent's "cpuset.cpus", i.e. they overlap.
> +        4) The "cpuset.cpus.effective" must be a subset of "cpuset.cpus"
> +           and cannot be empty unless there is no task associated with
> +           this partition.

This sounds good to me.

> +        Care must be taken to change a valid partition root to "member"
> +        as all its child partitions, if present, will become invalid.

This does not talk about recovering. Is it intentional? (I.e. to left
implementation defined)

Except the remarks above, I find the concepts described here good. I'll
reply to implementation separately & later.

Regards,
Michal