On 5/4/22 07:25, Michal Koutný wrote:
Hello.
On Tue, May 03, 2022 at 12:21:48PM -0400, Waiman Long <longman@xxxxxxxxxx> 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.
+
+ 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)
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
