[PATCH 2/2] cgroup: Remove nr_cgrps

[PATCH 2/2] cgroup: Remove nr_cgrps

[T.J. Mercier]

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 <tjmercier@google.com>
---
 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

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

[kernel test robot]

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/202405290609.I4LQ4GZ3-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240529/202405290609.I4LQ4GZ3-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405290609.I4LQ4GZ3-lkp@intel.com/

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