On 5/4/22 07:25, Michal Koutný wrote:
Hello.
On Tue, May 03, 2022 at 12:21:48PM -0400, Waiman Long longman@redhat.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.
You are right. I will fix that.
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".
Thanks for the suggestion, will modify the text as suggested.
- For a partition root to become valid, the following conditions
- must be met.
- The "cpuset.cpus" is exclusive, i.e. they are not shared by
any of its siblings (exclusivity rule).
- The parent cgroup is a valid partition root.
- 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 withthis 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)
This new patch series does have the ability to recover now. I am just not emphasizing the recovery aspect of it in the doc file. I will add a sentence about it.
Except the remarks above, I find the concepts described here good. I'll reply to implementation separately & later.
Thanks, Longman