Cgroup v2 bug: "domain invalid" node can't be converted to "threaded"

Cgroup v2 bug: "domain invalid" node can't be converted to "threaded"

[Michael Kerrisk (man-pages)]

Hello Tejun,

While experimenting with cgroups, we've found what looks to be a bug.
I eventually simplified this to what I think are the minimum steps to
demonstrate the issue.

The following shell session demonstrates the issue:

[Kernel was booted with cgroup_no_v1=all]
# cd /sys/fs/cgroup/unified
# cat cgroup.subtree_control    # show that no controllers are enabled

# mkdir -p mycgrp/a/b/c
# echo threaded > mycgrp/a/b/cgroup.type

At this point, the hierarchy looks as follows:

    mycgrp [d]
        a [dt]
            b [t]
                c [inv]

Now let's make node "a" threaded (and thus "mycgrp" s made "domain threaded"):

# echo threaded > mycgrp/a/cgroup.type

By this point, we now have a hierarchy that looks as follows:

    mycgrp [dt]
        a [t]
            b [t]
                c [inv]

But, when we try to convert the node "c" from "domain invalid" to
"threaded", we get ENOTSUP on the write():

# echo threaded > mycgrp/a/b/c/cgroup.type
sh: echo: write error: Operation not supported

Your thoughts?

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: Cgroup v2 bug: "domain invalid" node can't be converted to "threaded"

[Tejun Heo]

Hello, Michael.

Great catch.  Can you please verify whether the following patch fixes
the issue?

Thanks.

------ 8< ------
Subject: cgroup: Fix dom_cgrp propagation when enabling threaded mode

A cgroup which is already a threaded domain may be converted into a
threaded cgroup if the prerequisite conditions are met.  When this
happens, all threaded descendant should also have their ->dom_cgrp
updated to the new threaded domain cgroup.  Unfortunately, this
propagation was missing leading to the following failure.

  # cd /sys/fs/cgroup/unified
  # cat cgroup.subtree_control    # show that no controllers are enabled

  # mkdir -p mycgrp/a/b/c
  # echo threaded > mycgrp/a/b/cgroup.type

  At this point, the hierarchy looks as follows:

      mycgrp [d]
	  a [dt]
	      b [t]
		  c [inv]

  Now let's make node "a" threaded (and thus "mycgrp" s made "domain threaded"):

  # echo threaded > mycgrp/a/cgroup.type

  By this point, we now have a hierarchy that looks as follows:

      mycgrp [dt]
	  a [t]
	      b [t]
		  c [inv]

  But, when we try to convert the node "c" from "domain invalid" to
  "threaded", we get ENOTSUP on the write():

  # echo threaded > mycgrp/a/b/c/cgroup.type
  sh: echo: write error: Operation not supported

This patch fixes the problem by

* Moving the opencoded ->dom_cgrp save and restoration in
  cgroup_enable_threaded() into cgroup_{save|restore}_control() so
  that mulitple cgroups can be handled.
  
* Updating all threaded descendants' ->dom_cgrp to point to the new
  dom_cgrp when enabling threaded mode.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
Link: https://lore.kernel.org/r/CAKgNAkhHYCMn74TCNiMJ=ccLd7DcmXSbvw3CbZ1YREeG7iJM5g@mail.gmail.com
Fixes: 454000adaa2a ("cgroup: introduce cgroup->dom_cgrp and threaded css_set handling")
Cc: stable@vger.kernel.org # v4.14+
---
 kernel/cgroup/cgroup.c |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2836,11 +2836,12 @@ restart:
 }
 
 /**
- * cgroup_save_control - save control masks of a subtree
+ * cgroup_save_control - save control masks and dom_cgrp of a subtree
  * @cgrp: root of the target subtree
  *
- * Save ->subtree_control and ->subtree_ss_mask to the respective old_
- * prefixed fields for @cgrp's subtree including @cgrp itself.
+ * Save ->subtree_control, ->subtree_ss_mask and ->dom_cgrp to the
+ * respective old_ prefixed fields for @cgrp's subtree including @cgrp
+ * itself.
  */
 static void cgroup_save_control(struct cgroup *cgrp)
 {
@@ -2850,6 +2851,7 @@ static void cgroup_save_control(struct c
 	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp) {
 		dsct->old_subtree_control = dsct->subtree_control;
 		dsct->old_subtree_ss_mask = dsct->subtree_ss_mask;
+		dsct->old_dom_cgrp = dsct->dom_cgrp;
 	}
 }
 
@@ -2875,11 +2877,12 @@ static void cgroup_propagate_control(str
 }
 
 /**
- * cgroup_restore_control - restore control masks of a subtree
+ * cgroup_restore_control - restore control masks and dom_cgrp of a subtree
  * @cgrp: root of the target subtree
  *
- * Restore ->subtree_control and ->subtree_ss_mask from the respective old_
- * prefixed fields for @cgrp's subtree including @cgrp itself.
+ * Restore ->subtree_control, ->subtree_ss_mask and ->dom_cgrp from the
+ * respective old_ prefixed fields for @cgrp's subtree including @cgrp
+ * itself.
  */
 static void cgroup_restore_control(struct cgroup *cgrp)
 {
@@ -2889,6 +2892,7 @@ static void cgroup_restore_control(struc
 	cgroup_for_each_live_descendant_post(dsct, d_css, cgrp) {
 		dsct->subtree_control = dsct->old_subtree_control;
 		dsct->subtree_ss_mask = dsct->old_subtree_ss_mask;
+		dsct->dom_cgrp = dsct->old_dom_cgrp;
 	}
 }
 
@@ -3196,6 +3200,8 @@ static int cgroup_enable_threaded(struct
 {
 	struct cgroup *parent = cgroup_parent(cgrp);
 	struct cgroup *dom_cgrp = parent->dom_cgrp;
+	struct cgroup *dsct;
+	struct cgroup_subsys_state *d_css;
 	int ret;
 
 	lockdep_assert_held(&cgroup_mutex);
@@ -3225,12 +3231,13 @@ static int cgroup_enable_threaded(struct
 	 */
 	cgroup_save_control(cgrp);
 
-	cgrp->dom_cgrp = dom_cgrp;
+	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp)
+		if (dsct == cgrp || cgroup_is_threaded(dsct))
+			dsct->dom_cgrp = dom_cgrp;
+
 	ret = cgroup_apply_control(cgrp);
 	if (!ret)
 		parent->nr_threaded_children++;
-	else
-		cgrp->dom_cgrp = cgrp;
 
 	cgroup_finalize_control(cgrp, ret);
 	return ret;

Re: Cgroup v2 bug: "domain invalid" node can't be converted to "threaded"

[Michael Kerrisk (man-pages)]

Hello Tejun,

On 10/02/2018 11:07 PM, Tejun Heo wrote:
> Hello, Michael.
> 
> Great catch.  Can you please verify whether the following patch fixes
> the issue?
> 
> Thanks.
Against which kernel version should this apply? I get these build
errors on kernel 4.18:

[[
  CC      kernel/cgroup/cgroup.o
kernel/cgroup/cgroup.c: In function ‘cgroup_save_control’:
kernel/cgroup/cgroup.c:2851:9: error: ‘struct cgroup’ has no member named ‘old_dom_cgrp’; did you mean ‘dom_cgrp’?
   dsct->old_dom_cgrp = dsct->dom_cgrp;
         ^~~~~~~~~~~~
         dom_cgrp
kernel/cgroup/cgroup.c: In function ‘cgroup_restore_control’:
kernel/cgroup/cgroup.c:2892:26: error: ‘struct cgroup’ has no member named ‘old_dom_cgrp’; did you mean ‘dom_cgrp’?
   dsct->dom_cgrp = dsct->old_dom_cgrp;
                          ^~~~~~~~~~~~
                          dom_cgrp
make[2]: *** [scripts/Makefile.build:318: kernel/cgroup/cgroup.o] Error 1
make[1]: *** [scripts/Makefile.build:558: kernel/cgroup] Error 2
make: *** [Makefile:1029: kernel] Error 2
$ vi ~/p1.eml kernel/cgroup/cgroup.c
/home/mtk/p1.eml ==> /hdd/backup/home/mtk/p1.eml/2018-10-04_20:11:57
kernel/cgroup/cgroup.c ==> /hdd/backup/hdd/workspace/KERNEL/build/linux-4.18/kernel/cgroup/cgroup.c/2018-10-04_20:11:57
]]

Thanks,

Michael


> ------ 8< ------
> Subject: cgroup: Fix dom_cgrp propagation when enabling threaded mode
> 
> A cgroup which is already a threaded domain may be converted into a
> threaded cgroup if the prerequisite conditions are met.  When this
> happens, all threaded descendant should also have their ->dom_cgrp
> updated to the new threaded domain cgroup.  Unfortunately, this
> propagation was missing leading to the following failure.
> 
>   # cd /sys/fs/cgroup/unified
>   # cat cgroup.subtree_control    # show that no controllers are enabled
> 
>   # mkdir -p mycgrp/a/b/c
>   # echo threaded > mycgrp/a/b/cgroup.type
> 
>   At this point, the hierarchy looks as follows:
> 
>       mycgrp [d]
> 	  a [dt]
> 	      b [t]
> 		  c [inv]
> 
>   Now let's make node "a" threaded (and thus "mycgrp" s made "domain threaded"):
> 
>   # echo threaded > mycgrp/a/cgroup.type
> 
>   By this point, we now have a hierarchy that looks as follows:
> 
>       mycgrp [dt]
> 	  a [t]
> 	      b [t]
> 		  c [inv]
> 
>   But, when we try to convert the node "c" from "domain invalid" to
>   "threaded", we get ENOTSUP on the write():
> 
>   # echo threaded > mycgrp/a/b/c/cgroup.type
>   sh: echo: write error: Operation not supported
> 
> This patch fixes the problem by
> 
> * Moving the opencoded ->dom_cgrp save and restoration in
>   cgroup_enable_threaded() into cgroup_{save|restore}_control() so
>   that mulitple cgroups can be handled.
>   
> * Updating all threaded descendants' ->dom_cgrp to point to the new
>   dom_cgrp when enabling threaded mode.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
> Link: https://lore.kernel.org/r/CAKgNAkhHYCMn74TCNiMJ=ccLd7DcmXSbvw3CbZ1YREeG7iJM5g@mail.gmail.com
> Fixes: 454000adaa2a ("cgroup: introduce cgroup->dom_cgrp and threaded css_set handling")
> Cc: stable@vger.kernel.org # v4.14+
> ---
>  kernel/cgroup/cgroup.c |   25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -2836,11 +2836,12 @@ restart:
>  }
>  
>  /**
> - * cgroup_save_control - save control masks of a subtree
> + * cgroup_save_control - save control masks and dom_cgrp of a subtree
>   * @cgrp: root of the target subtree
>   *
> - * Save ->subtree_control and ->subtree_ss_mask to the respective old_
> - * prefixed fields for @cgrp's subtree including @cgrp itself.
> + * Save ->subtree_control, ->subtree_ss_mask and ->dom_cgrp to the
> + * respective old_ prefixed fields for @cgrp's subtree including @cgrp
> + * itself.
>   */
>  static void cgroup_save_control(struct cgroup *cgrp)
>  {
> @@ -2850,6 +2851,7 @@ static void cgroup_save_control(struct c
>  	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp) {
>  		dsct->old_subtree_control = dsct->subtree_control;
>  		dsct->old_subtree_ss_mask = dsct->subtree_ss_mask;
> +		dsct->old_dom_cgrp = dsct->dom_cgrp;
>  	}
>  }
>  
> @@ -2875,11 +2877,12 @@ static void cgroup_propagate_control(str
>  }
>  
>  /**
> - * cgroup_restore_control - restore control masks of a subtree
> + * cgroup_restore_control - restore control masks and dom_cgrp of a subtree
>   * @cgrp: root of the target subtree
>   *
> - * Restore ->subtree_control and ->subtree_ss_mask from the respective old_
> - * prefixed fields for @cgrp's subtree including @cgrp itself.
> + * Restore ->subtree_control, ->subtree_ss_mask and ->dom_cgrp from the
> + * respective old_ prefixed fields for @cgrp's subtree including @cgrp
> + * itself.
>   */
>  static void cgroup_restore_control(struct cgroup *cgrp)
>  {
> @@ -2889,6 +2892,7 @@ static void cgroup_restore_control(struc
>  	cgroup_for_each_live_descendant_post(dsct, d_css, cgrp) {
>  		dsct->subtree_control = dsct->old_subtree_control;
>  		dsct->subtree_ss_mask = dsct->old_subtree_ss_mask;
> +		dsct->dom_cgrp = dsct->old_dom_cgrp;
>  	}
>  }
>  
> @@ -3196,6 +3200,8 @@ static int cgroup_enable_threaded(struct
>  {
>  	struct cgroup *parent = cgroup_parent(cgrp);
>  	struct cgroup *dom_cgrp = parent->dom_cgrp;
> +	struct cgroup *dsct;
> +	struct cgroup_subsys_state *d_css;
>  	int ret;
>  
>  	lockdep_assert_held(&cgroup_mutex);
> @@ -3225,12 +3231,13 @@ static int cgroup_enable_threaded(struct
>  	 */
>  	cgroup_save_control(cgrp);
>  
> -	cgrp->dom_cgrp = dom_cgrp;
> +	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp)
> +		if (dsct == cgrp || cgroup_is_threaded(dsct))
> +			dsct->dom_cgrp = dom_cgrp;
> +
>  	ret = cgroup_apply_control(cgrp);
>  	if (!ret)
>  		parent->nr_threaded_children++;
> -	else
> -		cgrp->dom_cgrp = cgrp;
>  
>  	cgroup_finalize_control(cgrp, ret);
>  	return ret;
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: Cgroup v2 bug: "domain invalid" node can't be converted to "threaded"

[Tejun Heo]

Hello,

On Thu, Oct 04, 2018 at 08:14:55PM +0200, Michael Kerrisk (man-pages) wrote:
> On 10/02/2018 11:07 PM, Tejun Heo wrote:
> > Hello, Michael.
> > 
> > Great catch.  Can you please verify whether the following patch fixes
> > the issue?
> > 
> > Thanks.
> Against which kernel version should this apply? I get these build
> errors on kernel 4.18:

lol, sorry about that.  The header change ended up in the wrong patch.
Updated patch follows.

Thanks.

------ 8< ------
Subject: cgroup: Fix dom_cgrp propagation when enabling threaded mode

A cgroup which is already a threaded domain may be converted into a
threaded cgroup if the prerequisite conditions are met.  When this
happens, all threaded descendant should also have their ->dom_cgrp
updated to the new threaded domain cgroup.  Unfortunately, this
propagation was missing leading to the following failure.

  # cd /sys/fs/cgroup/unified
  # cat cgroup.subtree_control    # show that no controllers are enabled

  # mkdir -p mycgrp/a/b/c
  # echo threaded > mycgrp/a/b/cgroup.type

  At this point, the hierarchy looks as follows:

      mycgrp [d]
	  a [dt]
	      b [t]
		  c [inv]

  Now let's make node "a" threaded (and thus "mycgrp" s made "domain threaded"):

  # echo threaded > mycgrp/a/cgroup.type

  By this point, we now have a hierarchy that looks as follows:

      mycgrp [dt]
	  a [t]
	      b [t]
		  c [inv]

  But, when we try to convert the node "c" from "domain invalid" to
  "threaded", we get ENOTSUP on the write():

  # echo threaded > mycgrp/a/b/c/cgroup.type
  sh: echo: write error: Operation not supported

This patch fixes the problem by

* Moving the opencoded ->dom_cgrp save and restoration in
  cgroup_enable_threaded() into cgroup_{save|restore}_control() so
  that mulitple cgroups can be handled.
  
* Updating all threaded descendants' ->dom_cgrp to point to the new
  dom_cgrp when enabling threaded mode.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
Link: https://lore.kernel.org/r/CAKgNAkhHYCMn74TCNiMJ=ccLd7DcmXSbvw3CbZ1YREeG7iJM5g@mail.gmail.com
Fixes: 454000adaa2a ("cgroup: introduce cgroup->dom_cgrp and threaded css_set handling")
Cc: stable@vger.kernel.org # v4.14+
---
 include/linux/cgroup-defs.h |    1 +
 kernel/cgroup/cgroup.c      |   25 ++++++++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)

--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -412,6 +412,7 @@ struct cgroup {
 	 * specific task are charged to the dom_cgrp.
 	 */
 	struct cgroup *dom_cgrp;
+	struct cgroup *old_dom_cgrp;		/* used while enabling threaded */
 
 	/* per-cpu recursive resource statistics */
 	struct cgroup_rstat_cpu __percpu *rstat_cpu;
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2836,11 +2836,12 @@ restart:
 }
 
 /**
- * cgroup_save_control - save control masks of a subtree
+ * cgroup_save_control - save control masks and dom_cgrp of a subtree
  * @cgrp: root of the target subtree
  *
- * Save ->subtree_control and ->subtree_ss_mask to the respective old_
- * prefixed fields for @cgrp's subtree including @cgrp itself.
+ * Save ->subtree_control, ->subtree_ss_mask and ->dom_cgrp to the
+ * respective old_ prefixed fields for @cgrp's subtree including @cgrp
+ * itself.
  */
 static void cgroup_save_control(struct cgroup *cgrp)
 {
@@ -2850,6 +2851,7 @@ static void cgroup_save_control(struct c
 	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp) {
 		dsct->old_subtree_control = dsct->subtree_control;
 		dsct->old_subtree_ss_mask = dsct->subtree_ss_mask;
+		dsct->old_dom_cgrp = dsct->dom_cgrp;
 	}
 }
 
@@ -2875,11 +2877,12 @@ static void cgroup_propagate_control(str
 }
 
 /**
- * cgroup_restore_control - restore control masks of a subtree
+ * cgroup_restore_control - restore control masks and dom_cgrp of a subtree
  * @cgrp: root of the target subtree
  *
- * Restore ->subtree_control and ->subtree_ss_mask from the respective old_
- * prefixed fields for @cgrp's subtree including @cgrp itself.
+ * Restore ->subtree_control, ->subtree_ss_mask and ->dom_cgrp from the
+ * respective old_ prefixed fields for @cgrp's subtree including @cgrp
+ * itself.
  */
 static void cgroup_restore_control(struct cgroup *cgrp)
 {
@@ -2889,6 +2892,7 @@ static void cgroup_restore_control(struc
 	cgroup_for_each_live_descendant_post(dsct, d_css, cgrp) {
 		dsct->subtree_control = dsct->old_subtree_control;
 		dsct->subtree_ss_mask = dsct->old_subtree_ss_mask;
+		dsct->dom_cgrp = dsct->old_dom_cgrp;
 	}
 }
 
@@ -3196,6 +3200,8 @@ static int cgroup_enable_threaded(struct
 {
 	struct cgroup *parent = cgroup_parent(cgrp);
 	struct cgroup *dom_cgrp = parent->dom_cgrp;
+	struct cgroup *dsct;
+	struct cgroup_subsys_state *d_css;
 	int ret;
 
 	lockdep_assert_held(&cgroup_mutex);
@@ -3225,12 +3231,13 @@ static int cgroup_enable_threaded(struct
 	 */
 	cgroup_save_control(cgrp);
 
-	cgrp->dom_cgrp = dom_cgrp;
+	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp)
+		if (dsct == cgrp || cgroup_is_threaded(dsct))
+			dsct->dom_cgrp = dom_cgrp;
+
 	ret = cgroup_apply_control(cgrp);
 	if (!ret)
 		parent->nr_threaded_children++;
-	else
-		cgrp->dom_cgrp = cgrp;
 
 	cgroup_finalize_control(cgrp, ret);
 	return ret;

Re: Cgroup v2 bug: "domain invalid" node can't be converted to "threaded"

[Michael Kerrisk (man-pages)]

Hello Tejun,

On 10/04/2018 08:20 PM, Tejun Heo wrote:
> Hello,
> 
> On Thu, Oct 04, 2018 at 08:14:55PM +0200, Michael Kerrisk (man-pages) wrote:
>> On 10/02/2018 11:07 PM, Tejun Heo wrote:
>>> Hello, Michael.
>>>
>>> Great catch.  Can you please verify whether the following patch fixes
>>> the issue?
>>>
>>> Thanks.
>> Against which kernel version should this apply? I get these build
>> errors on kernel 4.18:
> 
> lol, sorry about that.  The header change ended up in the wrong patch.
> Updated patch follows.

That fixes the problem for me (built against 4.19-rc6).

When you roll the final patch, please include Amin and Joao in the 
Reported-by:

Thanks,

Michael


> ------ 8< ------
> Subject: cgroup: Fix dom_cgrp propagation when enabling threaded mode
> 
> A cgroup which is already a threaded domain may be converted into a
> threaded cgroup if the prerequisite conditions are met.  When this
> happens, all threaded descendant should also have their ->dom_cgrp
> updated to the new threaded domain cgroup.  Unfortunately, this
> propagation was missing leading to the following failure.
> 
>   # cd /sys/fs/cgroup/unified
>   # cat cgroup.subtree_control    # show that no controllers are enabled
> 
>   # mkdir -p mycgrp/a/b/c
>   # echo threaded > mycgrp/a/b/cgroup.type
> 
>   At this point, the hierarchy looks as follows:
> 
>       mycgrp [d]
> 	  a [dt]
> 	      b [t]
> 		  c [inv]
> 
>   Now let's make node "a" threaded (and thus "mycgrp" s made "domain threaded"):
> 
>   # echo threaded > mycgrp/a/cgroup.type
> 
>   By this point, we now have a hierarchy that looks as follows:
> 
>       mycgrp [dt]
> 	  a [t]
> 	      b [t]
> 		  c [inv]
> 
>   But, when we try to convert the node "c" from "domain invalid" to
>   "threaded", we get ENOTSUP on the write():
> 
>   # echo threaded > mycgrp/a/b/c/cgroup.type
>   sh: echo: write error: Operation not supported
> 
> This patch fixes the problem by
> 
> * Moving the opencoded ->dom_cgrp save and restoration in
>   cgroup_enable_threaded() into cgroup_{save|restore}_control() so
>   that mulitple cgroups can be handled.
>   
> * Updating all threaded descendants' ->dom_cgrp to point to the new
>   dom_cgrp when enabling threaded mode.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
> Link: https://lore.kernel.org/r/CAKgNAkhHYCMn74TCNiMJ=ccLd7DcmXSbvw3CbZ1YREeG7iJM5g@mail.gmail.com
> Fixes: 454000adaa2a ("cgroup: introduce cgroup->dom_cgrp and threaded css_set handling")
> Cc: stable@vger.kernel.org # v4.14+
> ---
>  include/linux/cgroup-defs.h |    1 +
>  kernel/cgroup/cgroup.c      |   25 ++++++++++++++++---------
>  2 files changed, 17 insertions(+), 9 deletions(-)
> 
> --- a/include/linux/cgroup-defs.h
> +++ b/include/linux/cgroup-defs.h
> @@ -412,6 +412,7 @@ struct cgroup {
>  	 * specific task are charged to the dom_cgrp.
>  	 */
>  	struct cgroup *dom_cgrp;
> +	struct cgroup *old_dom_cgrp;		/* used while enabling threaded */
>  
>  	/* per-cpu recursive resource statistics */
>  	struct cgroup_rstat_cpu __percpu *rstat_cpu;
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -2836,11 +2836,12 @@ restart:
>  }
>  
>  /**
> - * cgroup_save_control - save control masks of a subtree
> + * cgroup_save_control - save control masks and dom_cgrp of a subtree
>   * @cgrp: root of the target subtree
>   *
> - * Save ->subtree_control and ->subtree_ss_mask to the respective old_
> - * prefixed fields for @cgrp's subtree including @cgrp itself.
> + * Save ->subtree_control, ->subtree_ss_mask and ->dom_cgrp to the
> + * respective old_ prefixed fields for @cgrp's subtree including @cgrp
> + * itself.
>   */
>  static void cgroup_save_control(struct cgroup *cgrp)
>  {
> @@ -2850,6 +2851,7 @@ static void cgroup_save_control(struct c
>  	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp) {
>  		dsct->old_subtree_control = dsct->subtree_control;
>  		dsct->old_subtree_ss_mask = dsct->subtree_ss_mask;
> +		dsct->old_dom_cgrp = dsct->dom_cgrp;
>  	}
>  }
>  
> @@ -2875,11 +2877,12 @@ static void cgroup_propagate_control(str
>  }
>  
>  /**
> - * cgroup_restore_control - restore control masks of a subtree
> + * cgroup_restore_control - restore control masks and dom_cgrp of a subtree
>   * @cgrp: root of the target subtree
>   *
> - * Restore ->subtree_control and ->subtree_ss_mask from the respective old_
> - * prefixed fields for @cgrp's subtree including @cgrp itself.
> + * Restore ->subtree_control, ->subtree_ss_mask and ->dom_cgrp from the
> + * respective old_ prefixed fields for @cgrp's subtree including @cgrp
> + * itself.
>   */
>  static void cgroup_restore_control(struct cgroup *cgrp)
>  {
> @@ -2889,6 +2892,7 @@ static void cgroup_restore_control(struc
>  	cgroup_for_each_live_descendant_post(dsct, d_css, cgrp) {
>  		dsct->subtree_control = dsct->old_subtree_control;
>  		dsct->subtree_ss_mask = dsct->old_subtree_ss_mask;
> +		dsct->dom_cgrp = dsct->old_dom_cgrp;
>  	}
>  }
>  
> @@ -3196,6 +3200,8 @@ static int cgroup_enable_threaded(struct
>  {
>  	struct cgroup *parent = cgroup_parent(cgrp);
>  	struct cgroup *dom_cgrp = parent->dom_cgrp;
> +	struct cgroup *dsct;
> +	struct cgroup_subsys_state *d_css;
>  	int ret;
>  
>  	lockdep_assert_held(&cgroup_mutex);
> @@ -3225,12 +3231,13 @@ static int cgroup_enable_threaded(struct
>  	 */
>  	cgroup_save_control(cgrp);
>  
> -	cgrp->dom_cgrp = dom_cgrp;
> +	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp)
> +		if (dsct == cgrp || cgroup_is_threaded(dsct))
> +			dsct->dom_cgrp = dom_cgrp;
> +
>  	ret = cgroup_apply_control(cgrp);
>  	if (!ret)
>  		parent->nr_threaded_children++;
> -	else
> -		cgrp->dom_cgrp = cgrp;
>  
>  	cgroup_finalize_control(cgrp, ret);
>  	return ret;
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: Cgroup v2 bug: "domain invalid" node can't be converted to "threaded"

[Tejun Heo]

On Thu, Oct 04, 2018 at 11:20:17AM -0700, Tejun Heo wrote:
> Subject: cgroup: Fix dom_cgrp propagation when enabling threaded mode
> 
> A cgroup which is already a threaded domain may be converted into a
> threaded cgroup if the prerequisite conditions are met.  When this
> happens, all threaded descendant should also have their ->dom_cgrp
> updated to the new threaded domain cgroup.  Unfortunately, this
> propagation was missing leading to the following failure.

Applied to cgroup/for-4.19-fixes w/ reported-bys added.

Thanks.

-- 
tejun