2024-05-28 16:47:25

by T.J. Mercier

[permalink] [raw]
Subject: [PATCH 2/2] cgroup: Remove nr_cgrps

nr_cgrps now largely overlaps with nr_css. Use nr_css instead of
nr_cgrps for v1 so that nr_cgrps can be removed.

Signed-off-by: T.J. Mercier <[email protected]>
---
include/linux/cgroup-defs.h | 3 ---
kernel/cgroup/cgroup-v1.c | 8 ++------
kernel/cgroup/cgroup.c | 31 +++++++++++++++++++++++++------
3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index bc1dbf7652c4..dcd47a717eac 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -576,9 +576,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;
-
/*
* Number of cgroups using each controller. Includes online and zombies.
* Used only for /proc/cgroups.
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 9bad59486c46..d52dc62803c3 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -675,15 +675,11 @@ int proc_cgroupstats_show(struct seq_file *m, void *v)
* cgroup_mutex contention.
*/

- for_each_subsys(ss, i) {
- int count = cgroup_on_dfl(&ss->root->cgrp) ?
- atomic_read(&ss->root->nr_css[i]) : atomic_read(&ss->root->nr_cgrps);
-
+ for_each_subsys(ss, i)
seq_printf(m, "%s\t%d\t%d\t%d\n",
ss->legacy_name, ss->root->hierarchy_id,
- count,
+ atomic_read(&ss->root->nr_css[i]),
cgroup_ssid_enabled(i));
- }

return 0;
}
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 1bacd7cf7551..fb4510a28ea3 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1322,12 +1322,15 @@ static void cgroup_destroy_root(struct cgroup_root *root)
{
struct cgroup *cgrp = &root->cgrp;
struct cgrp_cset_link *link, *tmp_link;
+ struct cgroup_subsys *ss;
+ int ssid;

trace_cgroup_destroy_root(root);

cgroup_lock_and_drain_offline(&cgrp_dfl_root.cgrp);

- BUG_ON(atomic_read(&root->nr_cgrps));
+ for_each_subsys(ss, ssid)
+ BUG_ON(atomic_read(&root->nr_css[ssid]));
BUG_ON(!list_empty(&cgrp->self.children));

/* Rebind all subsystems back to the default hierarchy */
@@ -1874,6 +1877,7 @@ int rebind_subsystems(struct cgroup_root *dst_root, u16 ss_mask)
} else {
dcgrp->subtree_control |= 1 << ssid;
static_branch_disable(cgroup_subsys_on_dfl_key[ssid]);
+ atomic_set(&ss->root->nr_css[ssid], 1);
}

ret = cgroup_apply_control(dcgrp);
@@ -2046,7 +2050,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);

@@ -2065,6 +2068,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
LIST_HEAD(tmp_links);
struct cgroup *root_cgrp = &root->cgrp;
struct kernfs_syscall_ops *kf_sops;
+ struct cgroup_subsys *ss;
struct css_set *cset;
int i, ret;

@@ -2144,7 +2148,9 @@ 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);
+ do_each_subsys_mask(ss, i, ss_mask) {
+ BUG_ON(atomic_read(&root->nr_css[i]) != 1);
+ } while_each_subsys_mask();

ret = 0;
goto out;
@@ -5368,7 +5374,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);
@@ -5387,12 +5392,27 @@ static void css_free_rwork_fn(struct work_struct *work)
cgroup_rstat_exit(cgrp);
kfree(cgrp);
} else {
+ struct cgroup_root *root = cgrp->root;
/*
* This is root cgroup's refcnt reaching zero,
* which indicates that the root should be
* released.
*/
- cgroup_destroy_root(cgrp->root);
+
+ /*
+ * v1 root css are first onlined as v2, then rebound
+ * to v1 (without re-onlining) where their count is
+ * initialized to 1. Drop the root counters to 0
+ * before destroying v1 roots.
+ */
+ if (root != &cgrp_dfl_root) {
+ int ssid;
+
+ do_each_subsys_mask(ss, ssid, root->subsys_mask) {
+ atomic_dec(&root->nr_css[ssid]);
+ } while_each_subsys_mask();
+ }
+ cgroup_destroy_root(root);
}
}
}
@@ -5678,7 +5698,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.45.1.288.g0e0cd299f1-goog



2024-05-28 23:22:00

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] cgroup: Remove nr_cgrps

Hi Mercier,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6fbf71854e2ddea7c99397772fbbb3783bfe15b5]

url: https://github.com/intel-lab-lkp/linux/commits/T-J-Mercier/cgroup-Remove-nr_cgrps/20240529-004057
base: 6fbf71854e2ddea7c99397772fbbb3783bfe15b5
patch link: https://lore.kernel.org/r/20240528163750.2025330-1-tjmercier%40google.com
patch subject: [PATCH 2/2] cgroup: Remove nr_cgrps
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240529/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240529/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

kernel/cgroup/cgroup.c: In function 'cgroup_setup_root':
>> kernel/cgroup/cgroup.c:2071:31: warning: variable 'ss' set but not used [-Wunused-but-set-variable]
2071 | struct cgroup_subsys *ss;
| ^~


vim +/ss +2071 kernel/cgroup/cgroup.c

2065
2066 int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
2067 {
2068 LIST_HEAD(tmp_links);
2069 struct cgroup *root_cgrp = &root->cgrp;
2070 struct kernfs_syscall_ops *kf_sops;
> 2071 struct cgroup_subsys *ss;
2072 struct css_set *cset;
2073 int i, ret;
2074
2075 lockdep_assert_held(&cgroup_mutex);
2076
2077 ret = percpu_ref_init(&root_cgrp->self.refcnt, css_release,
2078 0, GFP_KERNEL);
2079 if (ret)
2080 goto out;
2081
2082 /*
2083 * We're accessing css_set_count without locking css_set_lock here,
2084 * but that's OK - it can only be increased by someone holding
2085 * cgroup_lock, and that's us. Later rebinding may disable
2086 * controllers on the default hierarchy and thus create new csets,
2087 * which can't be more than the existing ones. Allocate 2x.
2088 */
2089 ret = allocate_cgrp_cset_links(2 * css_set_count, &tmp_links);
2090 if (ret)
2091 goto cancel_ref;
2092
2093 ret = cgroup_init_root_id(root);
2094 if (ret)
2095 goto cancel_ref;
2096
2097 kf_sops = root == &cgrp_dfl_root ?
2098 &cgroup_kf_syscall_ops : &cgroup1_kf_syscall_ops;
2099
2100 root->kf_root = kernfs_create_root(kf_sops,
2101 KERNFS_ROOT_CREATE_DEACTIVATED |
2102 KERNFS_ROOT_SUPPORT_EXPORTOP |
2103 KERNFS_ROOT_SUPPORT_USER_XATTR,
2104 root_cgrp);
2105 if (IS_ERR(root->kf_root)) {
2106 ret = PTR_ERR(root->kf_root);
2107 goto exit_root_id;
2108 }
2109 root_cgrp->kn = kernfs_root_to_node(root->kf_root);
2110 WARN_ON_ONCE(cgroup_ino(root_cgrp) != 1);
2111 root_cgrp->ancestors[0] = root_cgrp;
2112
2113 ret = css_populate_dir(&root_cgrp->self);
2114 if (ret)
2115 goto destroy_root;
2116
2117 ret = cgroup_rstat_init(root_cgrp);
2118 if (ret)
2119 goto destroy_root;
2120
2121 ret = rebind_subsystems(root, ss_mask);
2122 if (ret)
2123 goto exit_stats;
2124
2125 ret = cgroup_bpf_inherit(root_cgrp);
2126 WARN_ON_ONCE(ret);
2127
2128 trace_cgroup_setup_root(root);
2129
2130 /*
2131 * There must be no failure case after here, since rebinding takes
2132 * care of subsystems' refcounts, which are explicitly dropped in
2133 * the failure exit path.
2134 */
2135 list_add_rcu(&root->root_list, &cgroup_roots);
2136 cgroup_root_count++;
2137
2138 /*
2139 * Link the root cgroup in this hierarchy into all the css_set
2140 * objects.
2141 */
2142 spin_lock_irq(&css_set_lock);
2143 hash_for_each(css_set_table, i, cset, hlist) {
2144 link_css_set(&tmp_links, cset, root_cgrp);
2145 if (css_set_populated(cset))
2146 cgroup_update_populated(root_cgrp, true);
2147 }
2148 spin_unlock_irq(&css_set_lock);
2149
2150 BUG_ON(!list_empty(&root_cgrp->self.children));
2151 do_each_subsys_mask(ss, i, ss_mask) {
2152 BUG_ON(atomic_read(&root->nr_css[i]) != 1);
2153 } while_each_subsys_mask();
2154
2155 ret = 0;
2156 goto out;
2157
2158 exit_stats:
2159 cgroup_rstat_exit(root_cgrp);
2160 destroy_root:
2161 kernfs_destroy_root(root->kf_root);
2162 root->kf_root = NULL;
2163 exit_root_id:
2164 cgroup_exit_root_id(root);
2165 cancel_ref:
2166 percpu_ref_exit(&root_cgrp->self.refcnt);
2167 out:
2168 free_cgrp_cset_links(&tmp_links);
2169 return ret;
2170 }
2171

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki