[PATCH] cgroup: Add cgroupstats numbers to cgroup.stat file

[PATCH] cgroup: Add cgroupstats numbers to cgroup.stat file

[Chengming Zhou]

In the cgroup v1, we can use netlink interface to get cgroupstats for
a cgroup. But it has been excluded from cgroup v2 interface intentionally
due to the duplication and inconsistencies with other statistics.
To make container monitor tool like "cadvisor" continue to work, we add
these cgroupstats numbers to the cgroup.stat file, and change the
admin-guide doc accordingly.

Reported-by: Daowen Luo <luodaowen.backend@bytedance.com>
Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
 Documentation/admin-guide/cgroup-v2.rst | 15 ++++++++++++++
 kernel/cgroup/cgroup.c                  | 36 +++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 6be43781ec7f..9f781edca95a 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -925,6 +925,21 @@ All cgroup core files are prefixed with "cgroup."
 		A dying cgroup can consume system resources not exceeding
 		limits, which were active at the moment of cgroup deletion.
 
+          nr_running
+                Number of tasks running.
+
+          nr_sleeping
+                Number of tasks sleeping.
+
+          nr_uninterruptible
+                Number of tasks in uninterruptible state.
+
+          nr_stopped
+                Number of tasks in stopped state.
+
+          nr_io_wait
+                Number of tasks waiting on IO.
+
   cgroup.freeze
 	A read-write single value file which exists on non-root cgroups.
 	Allowed values are "0" and "1". The default is "0".
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 0e23ae3b1e56..c6ccacaf812d 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -42,6 +42,7 @@
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/sched/task.h>
+#include <linux/delayacct.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/percpu-rwsem.h>
@@ -3499,11 +3500,46 @@ static int cgroup_events_show(struct seq_file *seq, void *v)
 static int cgroup_stat_show(struct seq_file *seq, void *v)
 {
 	struct cgroup *cgroup = seq_css(seq)->cgroup;
+	struct css_task_iter it;
+	struct task_struct *tsk;
+	u64 nr_running = 0;
+	u64 nr_sleeping = 0;
+	u64 nr_uninterruptible = 0;
+	u64 nr_stopped = 0;
+	u64 nr_io_wait = 0;
+
+	css_task_iter_start(&cgroup->self, 0, &it);
+	while ((tsk = css_task_iter_next(&it))) {
+		switch (tsk->state) {
+		case TASK_RUNNING:
+			nr_running++;
+			break;
+		case TASK_INTERRUPTIBLE:
+			nr_sleeping++;
+			break;
+		case TASK_UNINTERRUPTIBLE:
+			nr_uninterruptible++;
+			break;
+		case TASK_STOPPED:
+			nr_stopped++;
+			break;
+		default:
+			if (delayacct_is_task_waiting_on_io(tsk))
+				nr_io_wait++;
+			break;
+		}
+	}
+	css_task_iter_end(&it);
 
 	seq_printf(seq, "nr_descendants %d\n",
 		   cgroup->nr_descendants);
 	seq_printf(seq, "nr_dying_descendants %d\n",
 		   cgroup->nr_dying_descendants);
+	seq_printf(seq, "nr_running %llu\n", nr_running);
+	seq_printf(seq, "nr_sleeping %llu\n", nr_sleeping);
+	seq_printf(seq, "nr_uninterruptible %llu\n", nr_uninterruptible);
+	seq_printf(seq, "nr_stopped %llu\n", nr_stopped);
+	seq_printf(seq, "nr_io_wait %llu\n", nr_io_wait);
 
 	return 0;
 }
-- 
2.11.0

Re: [PATCH] cgroup: Add cgroupstats numbers to cgroup.stat file

[Tejun Heo]

Hello,

On Tue, Sep 15, 2020 at 11:53:49PM +0800, Chengming Zhou wrote:
> In the cgroup v1, we can use netlink interface to get cgroupstats for
> a cgroup. But it has been excluded from cgroup v2 interface intentionally
> due to the duplication and inconsistencies with other statistics.
> To make container monitor tool like "cadvisor" continue to work, we add
> these cgroupstats numbers to the cgroup.stat file, and change the
> admin-guide doc accordingly.

So, we can't add O(nr_threads) operations to cgroup.stat reads. There are
two ways forward that I can see.

* Investigate how these counters are being used. If it's used for congestion
  detection, pressure metrics are likely better indicators to use anyway. If
  the usage frequency is low enough, maybe querying from userspace should
  work?

* If the need for these per-cgroup task state counters is really
  justifiable, the counters should be maintained from scheduling event
  directly and summed up using rstat like other statistics.

Thanks.

-- 
tejun

Re: [Phishing Risk] [External] Re: [PATCH] cgroup: Add cgroupstats numbers to cgroup.stat file

[Chengming Zhou]

在 2020/9/21 下午10:50, Tejun Heo 写道:
> Hello,
>
> On Tue, Sep 15, 2020 at 11:53:49PM +0800, Chengming Zhou wrote:
>> In the cgroup v1, we can use netlink interface to get cgroupstats for
>> a cgroup. But it has been excluded from cgroup v2 interface intentionally
>> due to the duplication and inconsistencies with other statistics.
>> To make container monitor tool like "cadvisor" continue to work, we add
>> these cgroupstats numbers to the cgroup.stat file, and change the
>> admin-guide doc accordingly.
> So, we can't add O(nr_threads) operations to cgroup.stat reads. There are
> two ways forward that I can see.
>
> * Investigate how these counters are being used. If it's used for congestion
>   detection, pressure metrics are likely better indicators to use anyway. If
>   the usage frequency is low enough, maybe querying from userspace should
>   work?
>
> * If the need for these per-cgroup task state counters is really
>   justifiable, the counters should be maintained from scheduling event
>   directly and summed up using rstat like other statistics.
>
> Thanks.
>
Well, I see. These counters are being used for load monitor and debug,
pressure metrics

is good for congestion detection, but more details needed for debug when
problem happened.

And I noticed PSI has maintained these taskstats from scheduler event
already. I think we can

just export these counters gracefully.

Thanks.