[PATCH-cgroup 1/2] cgroup: Show # of subsystem CSSes in /proc/cgroups

[PATCH-cgroup 1/2] cgroup: Show # of subsystem CSSes in /proc/cgroups

[Waiman Long]

The /proc/cgroups file shows the number of cgroups for each of the
subsystems.  With cgroup v1, the number of CSSes is the same as the
number of cgroups. That is not the case anymore with cgroup v2. The
/proc/cgroups file cannot show the actual number of CSSes for the
subsystems that are bound to cgroup v2.

So if a v2 cgroup subsystem is leaking cgroups (typically memory
cgroup), we can't tell by looking at /proc/cgroups which cgroup
subsystems may be responsible.  This patch adds a css counter in the
cgroup_subsys structure to keep track of the number of CSSes for each
of the cgroup subsystems.

The proc_cgroupstats_show() function is modified to show the number
of CSSes instead. This will help us pinpoint which subsystem may be
responsible for the increasing number of dying but still allocated
cgroups.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/cgroup-defs.h | 3 +++
 kernel/cgroup/cgroup-v1.c   | 2 +-
 kernel/cgroup/cgroup.c      | 4 ++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index b36690ca0d3f..522ab77f0406 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -776,6 +776,9 @@ struct cgroup_subsys {
 	 * specifies the mask of subsystems that this one depends on.
 	 */
 	unsigned int depends_on;
+
+	/* Number of CSSes, used only for /proc/cgroups */
+	atomic_t nr_csses;
 };
 
 extern struct percpu_rw_semaphore cgroup_threadgroup_rwsem;
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index b9dbf6bf2779..56c2b030cd00 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -678,7 +678,7 @@ int proc_cgroupstats_show(struct seq_file *m, void *v)
 	for_each_subsys(ss, i)
 		seq_printf(m, "%s\t%d\t%d\t%d\n",
 			   ss->legacy_name, ss->root->hierarchy_id,
-			   atomic_read(&ss->root->nr_cgrps),
+			   atomic_read(&ss->nr_csses),
 			   cgroup_ssid_enabled(i));
 
 	return 0;
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index c8e4b62b436a..4a818192950f 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5375,6 +5375,7 @@ static void css_free_rwork_fn(struct work_struct *work)
 		int id = css->id;
 
 		ss->css_free(css);
+		atomic_dec(&ss->nr_csses);
 		cgroup_idr_remove(&ss->css_idr, id);
 		cgroup_put(cgrp);
 
@@ -5567,6 +5568,7 @@ static struct cgroup_subsys_state *css_create(struct cgroup *cgrp,
 	if (IS_ERR(css))
 		return css;
 
+	atomic_inc(&ss->nr_csses);
 	init_and_link_css(css, ss, cgrp);
 
 	err = percpu_ref_init(&css->refcnt, css_release, 0, GFP_KERNEL);
@@ -6005,6 +6007,8 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss, bool early)
 	/* Create the root cgroup state for this subsystem */
 	ss->root = &cgrp_dfl_root;
 	css = ss->css_alloc(NULL);
+	atomic_set(&ss->nr_csses, 1);
+
 	/* We don't handle early failures gracefully */
 	BUG_ON(IS_ERR(css));
 	init_and_link_css(css, ss, &cgrp_dfl_root.cgrp);
-- 
2.39.3

[PATCH-cgroup 2/2] cgroup: Remove nr_cgrps

[Waiman Long]

With the previous patch, the nr_cgrps field in the cgroup_root structure
is no longer being used. Just remove it.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/cgroup-defs.h | 3 ---
 kernel/cgroup/cgroup.c      | 5 -----
 2 files changed, 8 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 522ab77f0406..542334bf72df 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -581,9 +581,6 @@ struct cgroup_root {
 	/* must follow cgrp for cgrp->ancestors[0], see above */
 	struct cgroup *cgrp_ancestor_storage;
 
-	/* Number of cgroups in the hierarchy, used only for /proc/cgroups */
-	atomic_t nr_cgrps;
-
 	/* Hierarchy-specific flags */
 	unsigned int flags;
 
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 4a818192950f..b6854ba702ca 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1327,7 +1327,6 @@ static void cgroup_destroy_root(struct cgroup_root *root)
 
 	cgroup_lock_and_drain_offline(&cgrp_dfl_root.cgrp);
 
-	BUG_ON(atomic_read(&root->nr_cgrps));
 	BUG_ON(!list_empty(&cgrp->self.children));
 
 	/* Rebind all subsystems back to the default hierarchy */
@@ -2061,7 +2060,6 @@ void init_cgroup_root(struct cgroup_fs_context *ctx)
 	struct cgroup *cgrp = &root->cgrp;
 
 	INIT_LIST_HEAD_RCU(&root->root_list);
-	atomic_set(&root->nr_cgrps, 1);
 	cgrp->root = root;
 	init_cgroup_housekeeping(cgrp);
 
@@ -2159,7 +2157,6 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
 	spin_unlock_irq(&css_set_lock);
 
 	BUG_ON(!list_empty(&root_cgrp->self.children));
-	BUG_ON(atomic_read(&root->nr_cgrps) != 1);
 
 	ret = 0;
 	goto out;
@@ -5383,7 +5380,6 @@ static void css_free_rwork_fn(struct work_struct *work)
 			css_put(parent);
 	} else {
 		/* cgroup free path */
-		atomic_dec(&cgrp->root->nr_cgrps);
 		if (!cgroup_on_dfl(cgrp))
 			cgroup1_pidlist_destroy_all(cgrp);
 		cancel_work_sync(&cgrp->release_agent_work);
@@ -5693,7 +5689,6 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
 
 	/* allocation complete, commit to creation */
 	list_add_tail_rcu(&cgrp->self.sibling, &cgroup_parent(cgrp)->self.children);
-	atomic_inc(&root->nr_cgrps);
 	cgroup_get_live(parent);
 
 	/*
-- 
2.39.3

Re: [PATCH-cgroup 1/2] cgroup: Show # of subsystem CSSes in /proc/cgroups

[Tejun Heo]

Hello, Waiman.

On Fri, Jul 05, 2024 at 08:56:21PM -0400, Waiman Long wrote:
> The /proc/cgroups file shows the number of cgroups for each of the
> subsystems.  With cgroup v1, the number of CSSes is the same as the
> number of cgroups. That is not the case anymore with cgroup v2. The
> /proc/cgroups file cannot show the actual number of CSSes for the
> subsystems that are bound to cgroup v2.
> 
> So if a v2 cgroup subsystem is leaking cgroups (typically memory
> cgroup), we can't tell by looking at /proc/cgroups which cgroup
> subsystems may be responsible.  This patch adds a css counter in the
> cgroup_subsys structure to keep track of the number of CSSes for each
> of the cgroup subsystems.

The count sounds useful to me but can we add it in cgroup.stats instead?

Thanks.

-- 
tejun

Re: [PATCH-cgroup 1/2] cgroup: Show # of subsystem CSSes in /proc/cgroups

[Waiman Long]

On 7/8/24 14:45, Tejun Heo wrote:
> Hello, Waiman.
>
> On Fri, Jul 05, 2024 at 08:56:21PM -0400, Waiman Long wrote:
>> The /proc/cgroups file shows the number of cgroups for each of the
>> subsystems.  With cgroup v1, the number of CSSes is the same as the
>> number of cgroups. That is not the case anymore with cgroup v2. The
>> /proc/cgroups file cannot show the actual number of CSSes for the
>> subsystems that are bound to cgroup v2.
>>
>> So if a v2 cgroup subsystem is leaking cgroups (typically memory
>> cgroup), we can't tell by looking at /proc/cgroups which cgroup
>> subsystems may be responsible.  This patch adds a css counter in the
>> cgroup_subsys structure to keep track of the number of CSSes for each
>> of the cgroup subsystems.
> The count sounds useful to me but can we add it in cgroup.stats instead?

That information is certainly useful to display to cgroup.stat which 
currently only shows nr_descendants and nr_dying_descendants. So does 
"nr_<subsys_name> <cnt>" for each cgroup subsystems look good to you or 
do you have other suggestion?

The reason for this patch is because I got a bug report about leaking 
blkio cgroup due to the information shown in /proc/cgroups. I know you 
want do deprecate it for cgroup v2. How about adding a iine like "# 
Deprecated for cgroup v2, use cgroup.stats file for cgroup counts" at 
the top of /proc/cgroups when cgroup v2 is active?

Cheers,
Longman

Re: [PATCH-cgroup 1/2] cgroup: Show # of subsystem CSSes in /proc/cgroups

[Tejun Heo]

Hello,

On Mon, Jul 08, 2024 at 03:01:39PM -0400, Waiman Long wrote:
> That information is certainly useful to display to cgroup.stat which
> currently only shows nr_descendants and nr_dying_descendants. So does
> "nr_<subsys_name> <cnt>" for each cgroup subsystems look good to you or do
> you have other suggestion?

Yeah, something like that. Johannes, what do you think?

> The reason for this patch is because I got a bug report about leaking blkio
> cgroup due to the information shown in /proc/cgroups. I know you want do
> deprecate it for cgroup v2. How about adding a iine like "# Deprecated for
> cgroup v2, use cgroup.stats file for cgroup counts" at the top of
> /proc/cgroups when cgroup v2 is active?

I wonder whether that would break some parsers but we can try that and see
what happens.

Thanks.

-- 
tejun

Re: [PATCH-cgroup 1/2] cgroup: Show # of subsystem CSSes in /proc/cgroups

[Waiman Long]

On 7/8/24 15:05, Tejun Heo wrote:
> Hello,
>
> On Mon, Jul 08, 2024 at 03:01:39PM -0400, Waiman Long wrote:
>> That information is certainly useful to display to cgroup.stat which
>> currently only shows nr_descendants and nr_dying_descendants. So does
>> "nr_<subsys_name> <cnt>" for each cgroup subsystems look good to you or do
>> you have other suggestion?
> Yeah, something like that. Johannes, what do you think?
OK, will work on that.
>
>> The reason for this patch is because I got a bug report about leaking blkio
>> cgroup due to the information shown in /proc/cgroups. I know you want do
>> deprecate it for cgroup v2. How about adding a iine like "# Deprecated for
>> cgroup v2, use cgroup.stats file for cgroup counts" at the top of
>> /proc/cgroups when cgroup v2 is active?
> I wonder whether that would break some parsers but we can try that and see
> what happens.

I suppose the parser should skip line that begin with #. When testing 
this patch, I realize that systemd does parse /proc/cgroups when in 
cgroup v1 mode and change to it may break the bootup process. This file 
doesn't seem to be parsed at all when in cgroup v2 mode.

Cheers,
Longman