cgroup: implement cgroup2 thread mode, v3

v4: - Updated to marking each cgroup threaded as suggested by PeterZ.

Documentation/cgroup-v2.txt +21 -0

+On creation, a cgroup is always a domain cgroup and can be made
+threaded by writing "threaded" to the "cgroup.type" file.  The
+operation is single direction::
+
+  # echo threaded > cgroup.type
+
+Once threaded, the cgroup can't be made a domain again.  To enable the
+thread mode, the following conditions must be met.
+
+- As the cgroup will join the parent's resource domain.  The parent
+  must either be a valid (threaded) domain or a threaded cgroup.
+
+- The cgroup must be empty.  No enabled controllers, child cgroups or
+  processes.
+
+Topology-wise, a cgroup can be in an invalid state.  Please consider
+the following toplogy::
+
+  A (threaded domain) - B (threaded) - C (domain, just created)
+
+C is created as a domain but isn't connected to a parent which can

AFAICT this is not in fact what I suggested... :/

My proposal did not have that invalid state. It would simply refuse to change the type from thread to domain in the case where the parent is not a domain.

Also, my proposal maintained the normal property inheritance rules. A child cgroup's creation 'type' would be that of its parent and not always be 'domain'.

Let me read more (and more careful) to see if there's other things.

Hello, Peter.

Quoted 1 line · Peter Zijlstra

AFAICT this is not in fact what I suggested... :/

Heh, sorry about misattributing that. I was mostly referring to the overall idea of marking each cgroup domain or threaded rather than subtree.

Quoted 7 lines

My proposal did not have that invalid state. It would simply refuse to change the type from thread to domain in the case where the parent is not a domain.

Also, my proposal maintained the normal property inheritance rules. A child cgroup's creation 'type' would be that of its parent and not always be 'domain'.

But aren't both of the above get weird when the parent can host both domain and threaded children?

	  R
       /
      A(D)

If you create another child B under R, it's naturally gonna be a domain. Let's say you turn that to threaded.

	   R
       /   \
     A(D) B(T)

And now try to create another child C, should that be a domain or threaded?

If we only inherit from the second level on, which is in itself already confusing, that still leads to invalid configs for non-root thread roots.

Quoted 25 lines · Tejun Heo

Hello, Peter.

Quoted 1 line · Peter Zijlstra

AFAICT this is not in fact what I suggested... :/

Heh, sorry about misattributing that. I was mostly referring to the overall idea of marking each cgroup domain or threaded rather than subtree.

Quoted 7 lines

My proposal did not have that invalid state. It would simply refuse to change the type from thread to domain in the case where the parent is not a domain.

Also, my proposal maintained the normal property inheritance rules. A child cgroup's creation 'type' would be that of its parent and not always be 'domain'.

But aren't both of the above get weird when the parent can host both domain and threaded children?

	  R
       /
      A(D)

If you create another child B under R, it's naturally gonna be a domain. Let's say you turn that to threaded.

Quoted 6 lines

	   R
       /   \
     A(D) B(T)

And now try to create another child C, should that be a domain or threaded?

Domain of course, as R must be a domain, and hence all its children start out as such.

Quoted 3 lines

If we only inherit from the second level on, which is in itself already confusing, that still leads to invalid configs for non-root thread roots.

I don't see how. I don't get the example Waiman gave, what is wrong with:

	R (D)
	|
	A (D)
       / \
     C(D) B(T) 

? Afaict that's a perfectly valid configuration.

Quoted 2 lines

And now try to create another child C, should that be a domain or threaded?

Domain of course, as R must be a domain, and hence all its children start out as such.

Quoted 3 lines

If we only inherit from the second level on, which is in itself already confusing, that still leads to invalid configs for non-root thread roots.

I don't see how. I don't get the example Waiman gave, what is wrong with:

	R (D)
	|
	A (D)
       / \
     C(D) B(T) 

? Afaict that's a perfectly valid configuration.

Okay, we're kinda off the rails now. Just to verify that we're on the same page, are you also saying that the following should be a valid configuration?

	R (D)
        |
	A (D and has processes in it and controllers enabled)
        |
        C (D and has processes in it) 

Quoted 10 lines · Tejun Heo

Okay, we're kinda off the rails now. Just to verify that we're on the same page, are you also saying that the following should be a valid configuration?

	R (D)
        |
	A (D and has processes in it and controllers enabled)
        |
        C (D and has processes in it) 

Argh, the no internal process thing again -- I completely forgot about that :-(

Were there other things that caught your eyes?

Quoted 1 line · Tejun Heo

Were there other things that caught your eyes?

I didn't immediately see the point of "domain (threaded" output, and I

think it might be useful to have a "threaded" column in /proc/cgroups.

But no, I've been over this a few times now and I think we're good.

Yes, I think we can work with this. Thanks!

Quoted 23 lines · Tejun Heo

v4: - Updated to marking each cgroup threaded as suggested by PeterZ.

Documentation/cgroup-v2.txt +21 -0

+On creation, a cgroup is always a domain cgroup and can be made
+threaded by writing "threaded" to the "cgroup.type" file.  The
+operation is single direction::
+
+  # echo threaded > cgroup.type
+
+Once threaded, the cgroup can't be made a domain again.  To enable the
+thread mode, the following conditions must be met.
+
+- As the cgroup will join the parent's resource domain.  The parent
+  must either be a valid (threaded) domain or a threaded cgroup.
+
+- The cgroup must be empty.  No enabled controllers, child cgroups or
+  processes.
+
+Topology-wise, a cgroup can be in an invalid state.  Please consider
+the following toplogy::
+
+  A (threaded domain) - B (threaded) - C (domain, just created)
+
+C is created as a domain but isn't connected to a parent which can

Documentation/cgroup-v2.txt If the process becomes a zombie and the cgroup it was associated with · line 295

Quoted 10 lines

Documentation/cgroup-v2.txt +10 -0

+host child domains.  C can't be used until it is turned into a
+threaded cgroup.  "cgroup.type" file will report "domain (invalid)" in
+these cases.  Operations which fail due to invalid topology use
+EOPNOTSUPP as the errno.
+
+A domain cgroup is turned into a threaded domain when one of its child
+cgroup becomes threaded or threaded controllers are enabled in the
+"cgroup.subtree_control" file while there are processes in the cgroup.
+A threaded domain reverts to a normal domain when the conditions
+clear.

AFAICT this is not in fact what I suggested... :/

My proposal did not have that invalid state. It would simply refuse to change the type from thread to domain in the case where the parent is not a domain.

Also, my proposal maintained the normal property inheritance rules. A child cgroup's creation 'type' would be that of its parent and not always be 'domain'.

Quoted 15 lines · Peter Zijlstra

Quoted 14 lines · Tejun Heo

v4: - Updated to marking each cgroup threaded as suggested by PeterZ.

Documentation/cgroup-v2.txt +12 -0

+On creation, a cgroup is always a domain cgroup and can be made
+threaded by writing "threaded" to the "cgroup.type" file.  The
+operation is single direction::
+
+  # echo threaded > cgroup.type
+
+Once threaded, the cgroup can't be made a domain again.  To enable the
+thread mode, the following conditions must be met.
+
+- As the cgroup will join the parent's resource domain.  The parent
+  must either be a valid (threaded) domain or a threaded cgroup.
+

Quoted 8 lines

Documentation/cgroup-v2.txt If the process becomes a zombie and the cgroup it was associated with · line 284

Quoted 8 lines

+- The cgroup must be empty. No enabled controllers, child cgroups or

Documentation/cgroup-v2.txt +7 -0

+  processes.
+
+Topology-wise, a cgroup can be in an invalid state.  Please consider
+the following toplogy::
+
+  A (threaded domain) - B (threaded) - C (domain, just created)
+

Thinking about it some more. There is a place for invalid domain. It is not the child of a threaded cgroup. It is the siblings of a threaded cgroup whose parent is not root.

Root - A (domain) - B (domain)
\ C (domain)

With "echo threaded > B/cgroup.type":

Root - A (threaded domain) - B (threaded)
\ C (domain, invalid)

Any children of a threaded cgroup should be threaded.

Hello, Waiman.

Quoted 11 lines · Waiman Long

Thinking about it some more. There is a place for invalid domain. It is not the child of a threaded cgroup. It is the siblings of a threaded cgroup whose parent is not root.

Root - A (domain) - B (domain)
\ C (domain)

With "echo threaded > B/cgroup.type":

Root - A (threaded domain) - B (threaded)
\ C (domain, invalid)

Yes, I noted that when I was replying to Peter.

Quoted 1 line

Any children of a threaded cgroup should be threaded.

It's really difficult to discuss if you just declare that something should be a certain way without giving rationale for thinking so.

If we could get rid of the invalid state completely that way, I'd completely agree with you but that isn't the case here as you noted yourself, so the choice between the two isn't something trivially clear. Both choices come with their pros and cons. We can absoultely discuss them comparing the pros and cons.

Quoted 5 lines

If we could get rid of the invalid state completely that way, I'd completely agree with you but that isn't the case here as you noted yourself, so the choice between the two isn't something trivially clear. Both choices come with their pros and cons. We can absoultely discuss them comparing the pros and cons.

I am not advocating on removing the invalid state now as I note about

sibling cgroups. I am just saying that there is no point in not doing an automatic conversion to threaded for newly created children of threaded cgroups (not thread root). I don't see any cons in doing that.

Hello,

Quoted 6 lines · Waiman Long

Quoted 5 lines

If we could get rid of the invalid state completely that way, I'd completely agree with you but that isn't the case here as you noted yourself, so the choice between the two isn't something trivially clear. Both choices come with their pros and cons. We can absoultely discuss them comparing the pros and cons.

I am not advocating on removing the invalid state now as I note about

Yeah, removing invalid state would be great but we can't at least yet.

Quoted 3 lines

sibling cgroups. I am just saying that there is no point in not doing an automatic conversion to threaded for newly created children of threaded cgroups (not thread root). I don't see any cons in doing that.

So, the cons I see is inconsistency, now and in the future.

This may seem less clear with system root because we can have both domain and theraded children below it, which makes a newly created cgroup being a domain seem natural. More importantly, we can't do it any other way because we'd break existing users otherwise - creating a threaded cgroup would cause future first level cgroups to be threaded which will be very unexpected.

Let's think about a non-root threaded domain. At least for now, a non-root threaded domain is terminal - they can't host valid domain children. As the alternative term "thread root" implies, the threaded domain can be the root of a threaded subtree and nothing else, so it's kinda weird to make a new child cgroup there start out as a domain which can't be used, just like it'd be for the second level descendant cgroup.

However, the alternative is even stranger. Let's say we make the first level child automatically threaded, but that is inconsistent with when we first enable threaded mode. We either would have to turn all siblings at the same time or disallow enabling threaded mode if there are domain siblings, which I fear would be unnecessarily restrictive.

Another point is that what if we eventually make non-root threaded roots able to host domain children? Making children automatically threaded wouldn't make any sense then, right? I'll come back to this later.

So, it looks like if we're gonna automatically turn on threaded mode for new cgroups, the only thing we can do right now is what you're suggesting; however, we didn't arrive there through some straight-forward intuition or overall design. It started as a simple idea (I want it to be automatic) but the end result is a contorted destination shaped by constraints and happenstance.

To me, behaving differently on the first-level threaded children than on second+ level ones is too strange to be justified by the convenience of not having to turn on threaded on new cgroups.

On top of that, what happens if we get to implement PeterZ's idea of skipping over threaded internal cgroups to allow domains under threaded subtrees? That'd imply that we'd be able to host domains under threaded domains too. The end result would be completely non-sensical. We'd be defaulting to different modes for different reasons where half of those reasons won't hold anymore. This isn't surprising given that there's nothing actually consistent about the suggested default behavior.

For me, that is the only good reason why we should keep the current behavior. So I am fine with that.

+ cgrp->dom_cgrp = cgrp->dom_cgrp;

However, I am still puzzled by above line of code, should it be just

  cgrp->dom_cgrp = cgrp;

Documentation/cgroup-v2.txt +10 -0

+Threads
+~~~~~~~
+
+cgroup v2 supports thread granularity for a subset of controllers to
+support use cases requiring hierarchical resource distribution across
+the threads of a group of processes.  By default, all threads of a
+process belong to the same cgroup, which also serves as the resource
+domain to host resource consumptions which are not specific to a
+process or thread.  The thread mode allows threads to be spread across
+a subtree while still maintaining the common resource domain for them.

Documentation/cgroup-v2.txt +16 -0

+  cgroup.type
+
+	A read-write single value file which exists on non-root
+	cgroups.
+
+	When read, it indicates the current type of the cgroup, which
+	can be one of the following values.
+
+	- "domain" : A normal valid domain cgroup.
+
+	- "domain (threaded)" : A threaded domain cgroup which is
+          serving as the root of a threaded subtree.
+
+	- "domain (invalid)" : A cgroup which is in an invalid state.
+	  It can't be populated or have controllers enabled.  It may
+	  be allowed to become a threaded cgroup.

Quoted 4 lines

Documentation/cgroup-v2.txt +4 -0

+Marking a cgroup threaded makes it join the resource domain of its
+parent as a threaded cgroup.  The parent may be another threaded
+cgroup whose resource domain is further up in the hierarchy.  The root
+of a threaded subtree, that is, the nearest ancestor which is not

Documentation/cgroup-v2.txt If the process becomes a zombie and the cgroup it was associated with · line 246

Quoted 2 lines

Documentation/cgroup-v2.txt +2 -0

+threaded, is called threaded domain and serves as the resource domain
+for the entire subtree.

The cgroup code uses the term "thread root" in quite a number of places. So a developer may be confused when comparing the code and the documentation. I would recommend either introducing "thread root" as an alias for threaded domain here in the documentation or documenting that "threaded domain = thread root" in the code.

Documentation/cgroup-v2.txt Core Interface Files · line 775

Quoted 3 lines

cgroup.procs

Documentation/cgroup-v2.txt +0 -0

 	A read-write new-line separated values file which exists on
 	all cgroups.

Do we need to document that cgroup.procs isn't writable in a threaded cgroup?

kernel/cgroup/cgroup.c static struct cgroup *cgroup_create(struct cgroup *parent) · line 4609

Quoted 5 lines

kernel/cgroup/cgroup.c +1 -0

@@ -4301,6 +4606,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent)
 	cgrp->self.parent = &parent->self;
 	cgrp->root = root;
 	cgrp->level = level;
+	cgrp->dom_cgrp = cgrp->dom_cgrp;

It is a no-op. I think it is better to modify it to

+ cgrp->dom_cgrp = cgroup_is_threaded(parent) ? parent->dom_cgrp : cgrp;

Then we won't have an invalid domain state.

From: Waiman Long <[email protected]>

Update debug controller so that it prints out debug info about thread mode.

1) The relationship between proc_cset and threaded_csets are displayed.
2) The status of being a thread root or threaded cgroup is displayed.

This patch is extracted from Waiman's larger patch.

Patch-originally-by Waiman Long <[email protected]>

Signed-off-by Tejun Heo <[email protected]>

---
 kernel/cgroup/cgroup-internal.h |  2 ++
 kernel/cgroup/cgroup.c          |  4 +--
 kernel/cgroup/debug.c           | 58 +++++++++++++++++++++++++++++++----------
 3 files changed, 48 insertions(+), 16 deletions(-)

kernel/cgroup/cgroup-internal.h +2 -0

index f10eb19..c167a40 100644
@@ -153,6 +153,8 @@ static inline void get_css_set(struct css_set *cset)
 
 bool cgroup_ssid_enabled(int ssid);
 bool cgroup_on_dfl(const struct cgroup *cgrp);
+bool cgroup_is_thread_root(struct cgroup *cgrp);
+bool cgroup_is_threaded(struct cgroup *cgrp);
 
 struct cgroup_root *cgroup_root_from_kf(struct kernfs_root *kf_root);
 struct cgroup *task_cgroup_from_root(struct task_struct *task,

kernel/cgroup/cgroup.c +2 -2

index 7097ce4..2f34020 100644
@@ -333,7 +333,7 @@ static bool cgroup_has_tasks(struct cgroup *cgrp)
 	return cgrp->nr_populated_csets;
 }
 
-static bool cgroup_is_threaded(struct cgroup *cgrp)
+bool cgroup_is_threaded(struct cgroup *cgrp)
 {
 	return cgrp->dom_cgrp != cgrp;
 }

@@ -372,7 +372,7 @@ static bool cgroup_can_be_thread_root(struct cgroup *cgrp)
 }
 
 /* is @cgrp root of a threaded subtree? */
-static bool cgroup_is_thread_root(struct cgroup *cgrp)
+bool cgroup_is_thread_root(struct cgroup *cgrp)
 {
 	/* thread root should be a domain */
 	if (cgroup_is_threaded(cgrp))

kernel/cgroup/debug.c +0 -0

index dac46af..062904f 100644
@@ -114,27 +114,54 @@ static int cgroup_css_links_read(struct seq_file *seq, void *v)
 {
 	struct cgroup_subsys_state *css = seq_css(seq);

kernel/cgroup/debug.c static int cgroup_css_links_read(struct seq_file *seq, void *v) · line 124

Quoted 4 lines

kernel/cgroup/debug.c +4 -0

+	if (cgroup_is_thread_root(css->cgroup))
+		seq_puts(seq, "[thread root]\n");
+	else if (cgroup_is_threaded(css->cgroup))
+		seq_puts(seq, "[threaded]\n");

The cgroup status will not be needed anymore as you have introduced the cgroup.type control file that returns the proper information.

Hello,

Quoted 6 lines · Waiman Long

kernel/cgroup/debug.c static int cgroup_css_links_read(struct seq_file *seq, void *v) · line 124

Quoted 4 lines

kernel/cgroup/debug.c +4 -0

+	if (cgroup_is_thread_root(css->cgroup))
+		seq_puts(seq, "[thread root]\n");
+	else if (cgroup_is_threaded(css->cgroup))
+		seq_puts(seq, "[threaded]\n");

The cgroup status will not be needed anymore as you have introduced the cgroup.type control file that returns the proper information.

Ah, right, will drop this file.

Thanks.

Quoted 9 lines · Tejun Heo

Hello,

Quoted 6 lines · Waiman Long

kernel/cgroup/debug.c static int cgroup_css_links_read(struct seq_file *seq, void *v) · line 124

Quoted 4 lines

kernel/cgroup/debug.c +4 -0

+	if (cgroup_is_thread_root(css->cgroup))
+		seq_puts(seq, "[thread root]\n");
+	else if (cgroup_is_threaded(css->cgroup))
+		seq_puts(seq, "[threaded]\n");

The cgroup status will not be needed anymore as you have introduced the cgroup.type control file that returns the proper information.

Quoted 4 lines

Ah, right, will drop this file.

Thanks.

Oh, I am not saying that we should drop the whole patch. I just want to drop the above 4 line of codes as they are not needed.

Hello,

This is v3 of cgroup2 thread mode patchset. The changes from v2[L] are

* Switched to marking each cgroup threaded instead of doing it per-subtree as suggested by PeterZ. This allows more flexibility and removes certain interface quirks.

* Dropped RFC tag and excluded cpu controller patches from this patchset as threaded mode behaviors can easily be verified with the pid controller. Will follow up with cpu controller patchset later.

It is largely based on the discussions that we had at the plumbers last year. Here's the rough outline.

* Thread mode is explicitly enabled on a cgroup by writing "threaded" into "cgroup.type" file. The cgroup shouldn't have any processes or child cgroups. A threaded cgroup joins the the parent's resource domain and becomes a part of the threaded subtree anchored at the nearest domain ancestor, which is called the threaded domain cgroup of the subtree.

* Threads can be put anywhere in a threaded subtree by writing TIDs into "cgroup.threads" file. Process granularity and no-internal-process constraint don't apply in a threaded subtree.

* To be used in a threaded subtree, controllers should explicitly declare thread mode support and should be able to handle internal competition in some way.

* The threaded domain cgroup of a threaded subtree serves as the resource domain for the whole subtree. This is where all the controllers are guaranteed to have a common ground and resource consumptions in the threaded subtree which aren't tied to a specific thread are charged. Non-threaded controllers never see beyond thread root and can assume that all controllers will follow the same rules upto that point.

* Unlike other cgroups, the system root cgroup can serve as parent to domain child cgroups and threaded domains to threaded subtrees.

This allows threaded controllers to implement thread granular resource control without getting in the way of system level resource partitioning.

For more details on the interface and behavior, please refer to 0005.

This patchset contains the following six patches.

0001-cgroup-reorganize-cgroup.procs-task-write-path.patch 0002-cgroup-add-flags-to-css_task_iter_start-and-implemen.patch 0003-cgroup-introduce-cgroup-dom_cgrp-and-threaded-css_se.patch 0004-cgroup-implement-CSS_TASK_ITER_THREADED.patch 0005-cgroup-implement-cgroup-v2-thread-support.patch 0006-cgroup-update-debug-controller-to-print-out-thread-m.patch

0001-0005 implement cgroup2 thread mode. 0006 enables debug controller on it.

The patchset is based on the current cgroup/for-4.14 27f26753f8c0 ("cgroup: replace css_set walking populated test with testing cgrp->nr_populated_csets") and also available in the following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-cgroup2-threads-v3

Quoted 1 line

git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-cgroup2-threads-v3

Your new patches don't seem to be pushed to your git tree yet. I couldn't find them there.