[PATCH] taskstats: fix CGROUPSTATS_CMD_GET for cgroup v2

[PATCH] taskstats: fix CGROUPSTATS_CMD_GET for cgroup v2

[Chengming Zhou]

We found cgroupstats_build would return -EINVAL when using netlink
CGROUPSTATS_CMD_GET interface to get stats on cgroup v2. Fix it by
supporting cgroup v2 kernfs directory in cgroupstats_build, and export
cgroup2_fs_type like we did for cgroup_fs_type.

Reported-by: Daowen Luo <luodaowen.backend@bytedance.com>
Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
 kernel/cgroup/cgroup-internal.h | 1 +
 kernel/cgroup/cgroup-v1.c       | 5 +++--
 kernel/cgroup/cgroup.c          | 3 +--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index bfbeabc17a9d..9ca05fb513c2 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -151,6 +151,7 @@ extern spinlock_t css_set_lock;
 extern struct cgroup_subsys *cgroup_subsys[];
 extern struct list_head cgroup_roots;
 extern struct file_system_type cgroup_fs_type;
+extern struct file_system_type cgroup2_fs_type;
 
 /* iterate across the hierarchies */
 #define for_each_root(root)						\
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 191c329e482a..6d9e9b553276 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -686,13 +686,14 @@ int proc_cgroupstats_show(struct seq_file *m, void *v)
 int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)
 {
 	struct kernfs_node *kn = kernfs_node_from_dentry(dentry);
+	struct file_system_type *s_type = dentry->d_sb->s_type;
 	struct cgroup *cgrp;
 	struct css_task_iter it;
 	struct task_struct *tsk;
 
 	/* it should be kernfs_node belonging to cgroupfs and is a directory */
-	if (dentry->d_sb->s_type != &cgroup_fs_type || !kn ||
-	    kernfs_type(kn) != KERNFS_DIR)
+	if ((s_type != &cgroup_fs_type && s_type != &cgroup2_fs_type) ||
+	    !kn || kernfs_type(kn) != KERNFS_DIR)
 		return -EINVAL;
 
 	mutex_lock(&cgroup_mutex);
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index dd247747ec14..0e23ae3b1e56 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -206,7 +206,6 @@ struct cgroup_namespace init_cgroup_ns = {
 	.root_cset	= &init_css_set,
 };
 
-static struct file_system_type cgroup2_fs_type;
 static struct cftype cgroup_base_files[];
 
 static int cgroup_apply_control(struct cgroup *cgrp);
@@ -2162,7 +2161,7 @@ struct file_system_type cgroup_fs_type = {
 	.fs_flags		= FS_USERNS_MOUNT,
 };
 
-static struct file_system_type cgroup2_fs_type = {
+struct file_system_type cgroup2_fs_type = {
 	.name			= "cgroup2",
 	.init_fs_context	= cgroup_init_fs_context,
 	.parameters		= cgroup2_fs_parameters,
-- 
2.11.0

Re: [PATCH] taskstats: fix CGROUPSTATS_CMD_GET for cgroup v2

[Tejun Heo]

On Thu, Sep 10, 2020 at 01:52:07PM +0800, Chengming Zhou wrote:
> We found cgroupstats_build would return -EINVAL when using netlink
> CGROUPSTATS_CMD_GET interface to get stats on cgroup v2. Fix it by
> supporting cgroup v2 kernfs directory in cgroupstats_build, and export
> cgroup2_fs_type like we did for cgroup_fs_type.
> 
> Reported-by: Daowen Luo <luodaowen.backend@bytedance.com>
> Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>

The exclusion of cgroupstats from v2 interface was intentional due to the
duplication and inconsistencies with other statistics. If you need these
numbers, please justify and add them to the appropriate cgroupfs stat file.

Thanks.

-- 
tejun

Re: [Phishing Risk] [External] Re: [PATCH] taskstats: fix CGROUPSTATS_CMD_GET for cgroup v2

[Chengming Zhou]

在 2020/9/10 下午10:32, Tejun Heo 写道:
> On Thu, Sep 10, 2020 at 01:52:07PM +0800, Chengming Zhou wrote:
>> We found cgroupstats_build would return -EINVAL when using netlink
>> CGROUPSTATS_CMD_GET interface to get stats on cgroup v2. Fix it by
>> supporting cgroup v2 kernfs directory in cgroupstats_build, and export
>> cgroup2_fs_type like we did for cgroup_fs_type.
>>
>> Reported-by: Daowen Luo <luodaowen.backend@bytedance.com>
>> Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
>> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
> The exclusion of cgroupstats from v2 interface was intentional due to the
> duplication and inconsistencies with other statistics. If you need these
> numbers, please justify and add them to the appropriate cgroupfs stat file.
>
> Thanks.
>
Thanks for your advice. The container monitor tool "cadvisor" needs
these numbers to work,

I will put them in the existing cpu.stat interface.

Thanks.