[PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Roman Gushchin]

Add a cgroup.stat interface to the base cgroup control files
with the following metrics:

nr_descendants		total number of descendant cgroups
nr_dying_descendants	total number of dying descendant cgroups
max_descendant_depth	maximum descent depth below the current cgroup

Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: kernel-team@fb.com
Cc: cgroups@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/cgroup-v2.txt | 12 ++++++++++++
 kernel/cgroup/cgroup.c      | 31 +++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index dec5afdaa36d..ae9cc0d917b3 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -854,6 +854,18 @@ All cgroup core files are prefixed with "cgroup."
 		1 if the cgroup or its descendants contains any live
 		processes; otherwise, 0.
 
+  cgroup.stat
+	A read-only flat-keyed file with the following entries:
+
+	  nr_descendants
+		Total number of descendant cgroups.
+
+	  nr_dying_descendants
+		Total number of dying descendant cgroups.
+
+	  max_descendant_depth
+		Maximum descent depth below the current cgroup.
+
 
 Controllers
 ===========
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 85f6a112344b..b7af1745d46c 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3216,6 +3216,33 @@ static int cgroup_events_show(struct seq_file *seq, void *v)
 	return 0;
 }
 
+static int cgroup_stats_show(struct seq_file *seq, void *v)
+{
+	struct cgroup_subsys_state *css;
+	unsigned long total = 0;
+	unsigned long offline = 0;
+	int max_level = 0;
+
+	rcu_read_lock();
+	css_for_each_descendant_pre(css, seq_css(seq)) {
+		if (css == seq_css(seq))
+			continue;
+		++total;
+		if (!(css->flags & CSS_ONLINE))
+			++offline;
+		if (css->cgroup->level > max_level)
+			max_level = css->cgroup->level;
+	}
+	rcu_read_unlock();
+
+	seq_printf(seq, "nr_descendants %lu\n", total);
+	seq_printf(seq, "nr_dying_descendants %lu\n", offline);
+	seq_printf(seq, "max_descendant_depth %d\n",
+		   max_level - seq_css(seq)->cgroup->level);
+
+	return 0;
+}
+
 static int cgroup_file_open(struct kernfs_open_file *of)
 {
 	struct cftype *cft = of->kn->priv;
@@ -4309,6 +4336,10 @@ static struct cftype cgroup_base_files[] = {
 		.file_offset = offsetof(struct cgroup, events_file),
 		.seq_show = cgroup_events_show,
 	},
+	{
+		.name = "cgroup.stat",
+		.seq_show = cgroup_stats_show,
+	},
 	{ }	/* terminate */
 };
 
-- 
2.13.3

[PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Roman Gushchin]

Add a cgroup.stat interface to the base cgroup control files
with the following metrics:

nr_descendants		total number of descendant cgroups
nr_dying_descendants	total number of dying descendant cgroups
max_descendant_depth	maximum descent depth below the current cgroup

Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: kernel-team@fb.com
Cc: cgroups@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/cgroup-v2.txt | 12 ++++++++++++
 kernel/cgroup/cgroup.c      | 31 +++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index dec5afdaa36d..ae9cc0d917b3 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -854,6 +854,18 @@ All cgroup core files are prefixed with "cgroup."
 		1 if the cgroup or its descendants contains any live
 		processes; otherwise, 0.
 
+  cgroup.stat
+	A read-only flat-keyed file with the following entries:
+
+	  nr_descendants
+		Total number of descendant cgroups.
+
+	  nr_dying_descendants
+		Total number of dying descendant cgroups.
+
+	  max_descendant_depth
+		Maximum descent depth below the current cgroup.
+
 
 Controllers
 ===========
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 85f6a112344b..b7af1745d46c 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3216,6 +3216,33 @@ static int cgroup_events_show(struct seq_file *seq, void *v)
 	return 0;
 }
 
+static int cgroup_stats_show(struct seq_file *seq, void *v)
+{
+	struct cgroup_subsys_state *css;
+	unsigned long total = 0;
+	unsigned long offline = 0;
+	int max_level = 0;
+
+	rcu_read_lock();
+	css_for_each_descendant_pre(css, seq_css(seq)) {
+		if (css == seq_css(seq))
+			continue;
+		++total;
+		if (!(css->flags & CSS_ONLINE))
+			++offline;
+		if (css->cgroup->level > max_level)
+			max_level = css->cgroup->level;
+	}
+	rcu_read_unlock();
+
+	seq_printf(seq, "nr_descendants %lu\n", total);
+	seq_printf(seq, "nr_dying_descendants %lu\n", offline);
+	seq_printf(seq, "max_descendant_depth %d\n",
+		   max_level - seq_css(seq)->cgroup->level);
+
+	return 0;
+}
+
 static int cgroup_file_open(struct kernfs_open_file *of)
 {
 	struct cftype *cft = of->kn->priv;
@@ -4309,6 +4336,10 @@ static struct cftype cgroup_base_files[] = {
 		.file_offset = offsetof(struct cgroup, events_file),
 		.seq_show = cgroup_events_show,
 	},
+	{
+		.name = "cgroup.stat",
+		.seq_show = cgroup_stats_show,
+	},
 	{ }	/* terminate */
 };
 
-- 
2.13.3

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Tejun Heo]

Hello,

On Thu, Jul 27, 2017 at 05:14:20PM +0100, Roman Gushchin wrote:
> Add a cgroup.stat interface to the base cgroup control files
> with the following metrics:
> 
> nr_descendants		total number of descendant cgroups
> nr_dying_descendants	total number of dying descendant cgroups
> max_descendant_depth	maximum descent depth below the current cgroup

Yeah, this'd be great to have.  Some comments below.

> +  cgroup.stat
> +	A read-only flat-keyed file with the following entries:
> +
> +	  nr_descendants
> +		Total number of descendant cgroups.
> +
> +	  nr_dying_descendants
> +		Total number of dying descendant cgroups.

Can you please go into more detail on what's going on with dying
descendants here?

> +static int cgroup_stats_show(struct seq_file *seq, void *v)
> +{
> +	struct cgroup_subsys_state *css;
> +	unsigned long total = 0;
> +	unsigned long offline = 0;
> +	int max_level = 0;
> +
> +	rcu_read_lock();
> +	css_for_each_descendant_pre(css, seq_css(seq)) {
> +		if (css == seq_css(seq))
> +			continue;
> +		++total;

Let's do post increment for consistency.

> +		if (!(css->flags & CSS_ONLINE))
> +			++offline;
> +		if (css->cgroup->level > max_level)
> +			max_level = css->cgroup->level;
> +	}
> +	rcu_read_unlock();

I wonder whether we want to keep these counters in sync instead of
trying to gather the number on read.  Walking all descendants can get
expensive pretty quickly and things like nr_descendants will be useful
for other purposes too.

Thanks.

-- 
tejun

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Waiman Long]

On 07/27/2017 12:14 PM, Roman Gushchin wrote:
> Add a cgroup.stat interface to the base cgroup control files
> with the following metrics:
>
> nr_descendants		total number of descendant cgroups
> nr_dying_descendants	total number of dying descendant cgroups
> max_descendant_depth	maximum descent depth below the current cgroup

Both nr_descendants and max_descendant_depth can be found easily from
userspace by scanning the cgroup directory. nr_dying_descendants is a
transient figure and it should be 0 most of the time. So I wonder how
useful it can be.

This new interface file will be more useful if it can reveal information
that cannot be easily obtained by looking from userspace alone.

Cheers,
Longman

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Roman Gushchin]

On Thu, Jul 27, 2017 at 01:38:55PM -0400, Waiman Long wrote:
> On 07/27/2017 12:14 PM, Roman Gushchin wrote:
> > Add a cgroup.stat interface to the base cgroup control files
> > with the following metrics:
> >
> > nr_descendants		total number of descendant cgroups
> > nr_dying_descendants	total number of dying descendant cgroups
> > max_descendant_depth	maximum descent depth below the current cgroup
> 
> Both nr_descendants and max_descendant_depth can be found easily from
> userspace by scanning the cgroup directory. nr_dying_descendants is a
> transient figure and it should be 0 most of the time. So I wonder how
> useful it can be.
> 
> This new interface file will be more useful if it can reveal information
> that cannot be easily obtained by looking from userspace alone.

A cgroup can be in dying state for substantional amount of time
(imagine a memcg which holds a page mlocked by a process in an other cgroup).

The total amount of cgroups, as well as tree depth include dying cgroups,
which are not visible from the userspace.

The main purpose of this interface is to have a reasonable way to check if
there are no leaks/refcounting issues, which prevents cgroups from being
completely freed.

Thanks!

Roman

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Roman Gushchin]

On Thu, Jul 27, 2017 at 01:38:55PM -0400, Waiman Long wrote:
> On 07/27/2017 12:14 PM, Roman Gushchin wrote:
> > Add a cgroup.stat interface to the base cgroup control files
> > with the following metrics:
> >
> > nr_descendants		total number of descendant cgroups
> > nr_dying_descendants	total number of dying descendant cgroups
> > max_descendant_depth	maximum descent depth below the current cgroup
> 
> Both nr_descendants and max_descendant_depth can be found easily from
> userspace by scanning the cgroup directory. nr_dying_descendants is a
> transient figure and it should be 0 most of the time. So I wonder how
> useful it can be.
> 
> This new interface file will be more useful if it can reveal information
> that cannot be easily obtained by looking from userspace alone.

A cgroup can be in dying state for substantional amount of time
(imagine a memcg which holds a page mlocked by a process in an other cgroup).

The total amount of cgroups, as well as tree depth include dying cgroups,
which are not visible from the userspace.

The main purpose of this interface is to have a reasonable way to check if
there are no leaks/refcounting issues, which prevents cgroups from being
completely freed.

Thanks!

Roman

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Waiman Long]

On 07/27/2017 01:46 PM, Roman Gushchin wrote:
> On Thu, Jul 27, 2017 at 01:38:55PM -0400, Waiman Long wrote:
>> On 07/27/2017 12:14 PM, Roman Gushchin wrote:
>>> Add a cgroup.stat interface to the base cgroup control files
>>> with the following metrics:
>>>
>>> nr_descendants		total number of descendant cgroups
>>> nr_dying_descendants	total number of dying descendant cgroups
>>> max_descendant_depth	maximum descent depth below the current cgroup
>> Both nr_descendants and max_descendant_depth can be found easily from
>> userspace by scanning the cgroup directory. nr_dying_descendants is a
>> transient figure and it should be 0 most of the time. So I wonder how
>> useful it can be.
>>
>> This new interface file will be more useful if it can reveal information
>> that cannot be easily obtained by looking from userspace alone.
> A cgroup can be in dying state for substantional amount of time
> (imagine a memcg which holds a page mlocked by a process in an other cgroup).
>
> The total amount of cgroups, as well as tree depth include dying cgroups,
> which are not visible from the userspace.
>
> The main purpose of this interface is to have a reasonable way to check if
> there are no leaks/refcounting issues, which prevents cgroups from being
> completely freed.
>

If that is the use case, I am OK with that. However, you may want to
document that either in the commit log or in the comment. Maybe you can
also list the full path of the offlined cgroups if it is what you are
looking for.

Cheers,
Longman

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Waiman Long]

On 07/27/2017 01:46 PM, Roman Gushchin wrote:
> On Thu, Jul 27, 2017 at 01:38:55PM -0400, Waiman Long wrote:
>> On 07/27/2017 12:14 PM, Roman Gushchin wrote:
>>> Add a cgroup.stat interface to the base cgroup control files
>>> with the following metrics:
>>>
>>> nr_descendants		total number of descendant cgroups
>>> nr_dying_descendants	total number of dying descendant cgroups
>>> max_descendant_depth	maximum descent depth below the current cgroup
>> Both nr_descendants and max_descendant_depth can be found easily from
>> userspace by scanning the cgroup directory. nr_dying_descendants is a
>> transient figure and it should be 0 most of the time. So I wonder how
>> useful it can be.
>>
>> This new interface file will be more useful if it can reveal information
>> that cannot be easily obtained by looking from userspace alone.
> A cgroup can be in dying state for substantional amount of time
> (imagine a memcg which holds a page mlocked by a process in an other cgroup).
>
> The total amount of cgroups, as well as tree depth include dying cgroups,
> which are not visible from the userspace.
>
> The main purpose of this interface is to have a reasonable way to check if
> there are no leaks/refcounting issues, which prevents cgroups from being
> completely freed.
>

If that is the use case, I am OK with that. However, you may want to
document that either in the commit log or in the comment. Maybe you can
also list the full path of the offlined cgroups if it is what you are
looking for.

Cheers,
Longman

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Roman Gushchin]

On Thu, Jul 27, 2017 at 12:22:43PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Thu, Jul 27, 2017 at 05:14:20PM +0100, Roman Gushchin wrote:
> > Add a cgroup.stat interface to the base cgroup control files
> > with the following metrics:
> > 
> > nr_descendants		total number of descendant cgroups
> > nr_dying_descendants	total number of dying descendant cgroups
> > max_descendant_depth	maximum descent depth below the current cgroup
> 
> Yeah, this'd be great to have.  Some comments below.
> 
> > +  cgroup.stat
> > +	A read-only flat-keyed file with the following entries:
> > +
> > +	  nr_descendants
> > +		Total number of descendant cgroups.
> > +
> > +	  nr_dying_descendants
> > +		Total number of dying descendant cgroups.
> 
> Can you please go into more detail on what's going on with dying
> descendants here?

Sure.
Don't we plan do describe cgroup/css lifecycle in details
in a separate section?

> > +static int cgroup_stats_show(struct seq_file *seq, void *v)
> > +{
> > +	struct cgroup_subsys_state *css;
> > +	unsigned long total = 0;
> > +	unsigned long offline = 0;
> > +	int max_level = 0;
> > +
> > +	rcu_read_lock();
> > +	css_for_each_descendant_pre(css, seq_css(seq)) {
> > +		if (css == seq_css(seq))
> > +			continue;
> > +		++total;
> 
> Let's do post increment for consistency.

Ok.

> 
> > +		if (!(css->flags & CSS_ONLINE))
> > +			++offline;
> > +		if (css->cgroup->level > max_level)
> > +			max_level = css->cgroup->level;
> > +	}
> > +	rcu_read_unlock();
> 
> I wonder whether we want to keep these counters in sync instead of
> trying to gather the number on read.  Walking all descendants can get
> expensive pretty quickly and things like nr_descendants will be useful
> for other purposes too.

Ok, assuming that I'm working on adding an ability to set limits
on cgroup hierarchy, it seems very reasonable. I'll implement this
and post the updated path as a part of a patchset.

Thanks!

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Roman Gushchin]

On Thu, Jul 27, 2017 at 12:22:43PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Thu, Jul 27, 2017 at 05:14:20PM +0100, Roman Gushchin wrote:
> > Add a cgroup.stat interface to the base cgroup control files
> > with the following metrics:
> > 
> > nr_descendants		total number of descendant cgroups
> > nr_dying_descendants	total number of dying descendant cgroups
> > max_descendant_depth	maximum descent depth below the current cgroup
> 
> Yeah, this'd be great to have.  Some comments below.
> 
> > +  cgroup.stat
> > +	A read-only flat-keyed file with the following entries:
> > +
> > +	  nr_descendants
> > +		Total number of descendant cgroups.
> > +
> > +	  nr_dying_descendants
> > +		Total number of dying descendant cgroups.
> 
> Can you please go into more detail on what's going on with dying
> descendants here?

Sure.
Don't we plan do describe cgroup/css lifecycle in details
in a separate section?

> > +static int cgroup_stats_show(struct seq_file *seq, void *v)
> > +{
> > +	struct cgroup_subsys_state *css;
> > +	unsigned long total = 0;
> > +	unsigned long offline = 0;
> > +	int max_level = 0;
> > +
> > +	rcu_read_lock();
> > +	css_for_each_descendant_pre(css, seq_css(seq)) {
> > +		if (css == seq_css(seq))
> > +			continue;
> > +		++total;
> 
> Let's do post increment for consistency.

Ok.

> 
> > +		if (!(css->flags & CSS_ONLINE))
> > +			++offline;
> > +		if (css->cgroup->level > max_level)
> > +			max_level = css->cgroup->level;
> > +	}
> > +	rcu_read_unlock();
> 
> I wonder whether we want to keep these counters in sync instead of
> trying to gather the number on read.  Walking all descendants can get
> expensive pretty quickly and things like nr_descendants will be useful
> for other purposes too.

Ok, assuming that I'm working on adding an ability to set limits
on cgroup hierarchy, it seems very reasonable. I'll implement this
and post the updated path as a part of a patchset.

Thanks!

Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats

[Tejun Heo]

Hello,

On Fri, Jul 28, 2017 at 02:01:55PM +0100, Roman Gushchin wrote:
> > > +	  nr_dying_descendants
> > > +		Total number of dying descendant cgroups.
> > 
> > Can you please go into more detail on what's going on with dying
> > descendants here?
>
> Sure.
> Don't we plan do describe cgroup/css lifecycle in details
> in a separate section?

We should but it'd still be nice to have a short description here too.

Thanks.

-- 
tejun