Hello Tejun
According to Documentation/cgroup-v2.txt, a "domain invalid" cgroup
can't have member tasks. And indeed this is generally not permitted.
However, someone recently showed me a scenario where a "domain
invalid" cgroup can have member processes. See the following example:
# mkdir -p /sys/fs/cgroup/unified/grp0/grp1
# sleep 1000 &
[1] 10549
# echo 10549 > /sys/fs/cgroup/unified/grp0/grp1/cgroup.procs
# echo threaded > /sys/fs/cgroup/unified/grp0/cgroup.type
# cat /sys/fs/cgroup/unified/grp0/cgroup.type
threaded
# cat /sys/fs/cgroup/unified/grp0/grp1/cgroup.type
domain invalid
# cat /sys/fs/cgroup/unified/grp0/grp1/cgroup.threads
10549
From the above, we see that the cgroup grp0/grp1 is of type "domain
invalid" and has a member thread. This seems to be a violation of the
documented rules, and is I assume a bug, since in the above scenario,
we are denied from adding further tasks to the grp0/grp1 cgroup:
# sleep 2000 &
[2] 10553
# echo 10553 > /sys/fs/cgroup/unified/grp0/grp1/cgroup.procs
sh: echo: write error: Operation not supported
Could you comment please?
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On Tue, Feb 20, 2018 at 08:26:59PM +0100, Michael Kerrisk (man-pages) wrote:
> Hello Tejun
>
> According to Documentation/cgroup-v2.txt, a "domain invalid" cgroup
> can't have member tasks. And indeed this is generally not permitted.
>
> However, someone recently showed me a scenario where a "domain
> invalid" cgroup can have member processes. See the following example:
>
> # mkdir -p /sys/fs/cgroup/unified/grp0/grp1
> # sleep 1000 &
> [1] 10549
> # echo 10549 > /sys/fs/cgroup/unified/grp0/grp1/cgroup.procs
> # echo threaded > /sys/fs/cgroup/unified/grp0/cgroup.type
> # cat /sys/fs/cgroup/unified/grp0/cgroup.type
> threaded
> # cat /sys/fs/cgroup/unified/grp0/grp1/cgroup.type
> domain invalid
> # cat /sys/fs/cgroup/unified/grp0/grp1/cgroup.threads
> 10549
>
> From the above, we see that the cgroup grp0/grp1 is of type "domain
> invalid" and has a member thread. This seems to be a violation of the
> documented rules, and is I assume a bug, since in the above scenario,
> we are denied from adding further tasks to the grp0/grp1 cgroup:
>
> # sleep 2000 &
> [2] 10553
> # echo 10553 > /sys/fs/cgroup/unified/grp0/grp1/cgroup.procs
> sh: echo: write error: Operation not supported
>
> Could you comment please?
Hmm... nr_populated_domain_children check should have caught that
condition and rejected it. Will look into what's going on.
Thanks.
--
tejun
On Tue, Feb 20, 2018 at 11:35:47AM -0800, Tejun Heo wrote:
> Hmm... nr_populated_domain_children check should have caught that
> condition and rejected it. Will look into what's going on.
Ah, okay, I was special-casing the first level children case too
early. If you nest the test case by one more level, it fails as
intended. I'll fix the checks.
Thanks.
--
tejun
Hi Tehjun,
On 21 February 2018 at 00:01, Tejun Heo <[email protected]> wrote:
> On Tue, Feb 20, 2018 at 11:35:47AM -0800, Tejun Heo wrote:
>> Hmm... nr_populated_domain_children check should have caught that
>> condition and rejected it. Will look into what's going on.
>
> Ah, okay, I was special-casing the first level children case too
> early. If you nest the test case by one more level, it fails as
> intended. I'll fix the checks.
Thanks for looking into this so quickly! Please CC me on the patch.
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
From d1897c9538edafd4ae6bbd03cc075962ddde2c21 Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Wed, 21 Feb 2018 11:39:22 -0800
A domain cgroup isn't allowed to be turned threaded if its subtree is
populated or domain controllers are enabled. cgroup_enable_threaded()
depended on cgroup_can_be_thread_root() test to enforce this rule. A
parent which has populated domain descendants or have domain
controllers enabled can't become a thread root, so the above rules are
enforced automatically.
However, for the root cgroup which can host mixed domain and threaded
children, cgroup_can_be_thread_root() doesn't check any of those
conditions and thus first level cgroups ends up escaping those rules.
This patch fixes the bug by adding explicit checks for those rules in
cgroup_enable_threaded().
Reported-by: Michael Kerrisk (man-pages) <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
Fixes: 8cfd8147df67 ("cgroup: implement cgroup v2 thread support")
Cc: [email protected] # v4.14+
---
kernel/cgroup/cgroup.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 8cda3bc..4bfb290 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3183,6 +3183,16 @@ static int cgroup_enable_threaded(struct cgroup *cgrp)
if (cgroup_is_threaded(cgrp))
return 0;
+ /*
+ * If @cgroup is populated or has domain controllers enabled, it
+ * can't be switched. While the below cgroup_can_be_thread_root()
+ * test can catch the same conditions, that's only when @parent is
+ * not mixable, so let's check it explicitly.
+ */
+ if (cgroup_is_populated(cgrp) ||
+ cgrp->subtree_control & ~cgrp_dfl_threaded_ss_mask)
+ return -EOPNOTSUPP;
+
/* we're joining the parent's domain, ensure its validity */
if (!cgroup_is_valid_domain(dom_cgrp) ||
!cgroup_can_be_thread_root(dom_cgrp))
--
2.9.5