Skip to contentSkip to conversation

cgroup: drop module support and cgroup_root_mutex

PATCH Superseded Tejun Heo 10 participants 23 messages Jan 17, 2014 – Jan 20, 2014 4 messages not in the archive

Subsystems (14)
Networking Network drivers Memory management VFS +10 more subsystems

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Tejun Heo

author maintainer
#0 lore raw

PATCH 1/1 cgroup: drop module support and cgroup_root_mutex

Hello,

There are only two controllers which make use of the module support - net_cls and net_prio, both of which, non-coincidentally, aren't actual resource controllers. It's highly unlikely that the actual resource controlling controllers are gonna be made modular and we aren't gonna add more non-resource controllers either, so the value of module support is strictly restricted to the two existing controllers.

Both controllers are fairly simple while module support in cgroup core adds quite a bit of complexity. Building those two controllers as module only saves about half-page in vmlinux, which really can't justify the additional complexity and (minute but existing) runtime overhead.

This patchset makes net_cls and net_prio config options bool so that they can't be built as modules and drop module support from cgroup core, which in turn facilitates further simplification leading to removal of cgroup_root_mutex by allowing iterating subsystems outside the mutexes.

This patchset contains the following six patches.

0001-cgroup-make-CONFIG_NET_CLS_CGROUP-and-CONFIG_NETPRIO.patch 0002-cgroup-drop-module-support.patch 0003-cgroup-clean-up-cgroup_subsys-names-and-initializati.patch 0004-cgroup-rename-cgroup_subsys-subsys_id-to-id.patch 0005-cgroup-update-locking-in-cgroup_show_options.patch 0006-cgroup-remove-cgroup_root_mutex.patch

0001-0002 drop cgroup module support.

0003-0004 are cleanups.

0005-0006 remove cgroup_root_mutex.

The patchset is also available in the following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-simplify

diffstat follows. Thanks.

 block/blk-cgroup.c             |    9
 block/blk-cgroup.h             |    2
 fs/bio.c                       |    2
 include/linux/cgroup.h         |   38 ---
 include/linux/cgroup_subsys.h  |   30 +--
 include/linux/hugetlb_cgroup.h |    2
 include/linux/memcontrol.h     |    2
 include/net/cls_cgroup.h       |    4
 include/net/netprio_cgroup.h   |    4
 kernel/cgroup.c                |  392 ++++++-----------------------------------
 kernel/cgroup_freezer.c        |    8
 kernel/cpuset.c                |   10 -
 kernel/events/core.c           |    8
 kernel/sched/core.c            |    6
 kernel/sched/cpuacct.c         |    6
 mm/hugetlb_cgroup.c            |    9
 mm/memcontrol.c                |   22 +-
 net/Kconfig                    |    2
 net/core/netprio_cgroup.c      |   37 ---
 net/ipv4/tcp_memcontrol.c      |    2
 net/sched/Kconfig              |    2
 net/sched/cls_cgroup.c         |   30 ---
 security/device_cgroup.c       |    8
 23 files changed, 132 insertions(+), 503 deletions(-)

--
tejun

author maintainer
#1 lore raw

PATCH 1/6 cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

net_cls and net_prio are the only cgroups which are allowed to be built as modules. The savings from allowing the two controllers to be built as modules are tiny especially given that cgroup module support itself adds quite a bit of complexity.

The following are the sizes of vmlinux with both built as module and both built as part of the kernel image with cgroup module support removed.

	text		data		bss		dec
	20292207	2411496		10784768	33488471
	20293421	2412568		10784768	33490757

The total difference is 2286 bytes. Given that none of other controllers has much chance of being made a module and that we're unlikely to add new modular controllers, the added complexity is simply not justifiable.

As a first step to drop cgroup module support, this patch changes the two config options to bool from tristate and drops module related code from the two controllers.

Signed-off-by Tejun Heo <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>
---
 net/Kconfig               |  2 +-
 net/core/netprio_cgroup.c | 32 ++------------------------------
 net/sched/Kconfig         |  2 +-
 net/sched/cls_cgroup.c    | 23 ++---------------------
 4 files changed, 6 insertions(+), 53 deletions(-)
net/Kconfig +1 -1
index d334678..fea7b88 100644
@@ -239,7 +239,7 @@ config XPS
 	default y
 
 config NETPRIO_CGROUP
-	tristate "Network priority cgroup"
+	bool "Network priority cgroup"
 	depends on CGROUPS
 	---help---
 	  Cgroup subsystem for use in assigning processes to network priorities on
net/core/netprio_cgroup.c +2 -30
index 56cbb69..8912da7 100644
@@ -283,37 +283,9 @@ static struct notifier_block netprio_device_notifier = {
 
 static int __init init_cgroup_netprio(void)
 {
-	int ret;
-
-	ret = cgroup_load_subsys(&net_prio_subsys);
-	if (ret)
-		goto out;
-
 	register_netdevice_notifier(&netprio_device_notifier);
-
-out:
-	return ret;
-}
-
-static void __exit exit_cgroup_netprio(void)
-{
-	struct netprio_map *old;
-	struct net_device *dev;
-
-	unregister_netdevice_notifier(&netprio_device_notifier);
-
-	cgroup_unload_subsys(&net_prio_subsys);
-
-	rtnl_lock();
-	for_each_netdev(&init_net, dev) {
-		old = rtnl_dereference(dev->priomap);
-		RCU_INIT_POINTER(dev->priomap, NULL);
-		if (old)
-			kfree_rcu(old, rcu);
-	}
-	rtnl_unlock();
+	return 0;
 }
 
-module_init(init_cgroup_netprio);
-module_exit(exit_cgroup_netprio);
+subsys_initcall(init_cgroup_netprio);
 MODULE_LICENSE("GPL v2");
net/sched/Kconfig +1 -1
index ad1f1d8..0a9e4d1 100644
@@ -433,7 +433,7 @@ config NET_CLS_FLOW
 	  module will be called cls_flow.
 
 config NET_CLS_CGROUP
-	tristate "Control Group Classifier"
+	bool "Control Group Classifier"
 	select NET_CLS
 	depends on CGROUPS
 	---help---
net/sched/cls_cgroup.c +2 -21
index 16006c9..2f6a9b4 100644
@@ -309,27 +309,8 @@ static struct tcf_proto_ops cls_cgroup_ops __read_mostly = {
 
 static int __init init_cgroup_cls(void)
 {
-	int ret;
-
-	ret = cgroup_load_subsys(&net_cls_subsys);
-	if (ret)
-		goto out;
-
-	ret = register_tcf_proto_ops(&cls_cgroup_ops);
-	if (ret)
-		cgroup_unload_subsys(&net_cls_subsys);
-
-out:
-	return ret;
-}
-
-static void __exit exit_cgroup_cls(void)
-{
-	unregister_tcf_proto_ops(&cls_cgroup_ops);
-
-	cgroup_unload_subsys(&net_cls_subsys);
+	return register_tcf_proto_ops(&cls_cgroup_ops);
 }
 
-module_init(init_cgroup_cls);
-module_exit(exit_cgroup_cls);
+subsys_initcall(init_cgroup_cls);
 MODULE_LICENSE("GPL");
Signature · 1 line
1.8.4.2
Acked-by on v1 1/6
#2 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

Quoted 18 lines · Tejun Heo

net_cls and net_prio are the only cgroups which are allowed to be built as modules. The savings from allowing the two controllers to be built as modules are tiny especially given that cgroup module support itself adds quite a bit of complexity.

The following are the sizes of vmlinux with both built as module and both built as part of the kernel image with cgroup module support removed.

	text		data		bss		dec
	20292207	2411496		10784768	33488471
	20293421	2412568		10784768	33490757

The total difference is 2286 bytes. Given that none of other controllers has much chance of being made a module and that we're unlikely to add new modular controllers, the added complexity is simply not justifiable.

Quoted 8 lines

As a first step to drop cgroup module support, this patch changes the two config options to bool from tristate and drops module related code from the two controllers.

Signed-off-by Tejun Heo <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>
Acked-by Neil Horman <[email protected]>
#3 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

In reply to Tejun Heo
Cc Daniel Borkmann <[email protected]>
Quoted 13 lines · Tejun Heo

net_cls and net_prio are the only cgroups which are allowed to be built as modules. The savings from allowing the two controllers to be built as modules are tiny especially given that cgroup module support itself adds quite a bit of complexity.

The following are the sizes of vmlinux with both built as module and both built as part of the kernel image with cgroup module support removed.

	text		data		bss		dec
	20292207	2411496		10784768	33488471
	20293421	2412568		10784768	33490757
Quoted 9 lines

The total difference is 2286 bytes. Given that none of other controllers has much chance of being made a module and that we're unlikely to add new modular controllers, the added complexity is simply not justifiable.

As a first step to drop cgroup module support, this patch changes the two config options to bool from tristate and drops module related code from the two controllers.

I sugguested Daniel to do this for net_cls, and the change has been in net-next.

https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=fe1217c4f3f7d7cbf8efdd8dd5fdc7204a1d65a8

I was planning to remove module support after that change goes into upstream. :)

Quoted 11 lines
Signed-off-by Tejun Heo <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>
---
 net/Kconfig               |  2 +-
 net/core/netprio_cgroup.c | 32 ++------------------------------
 net/sched/Kconfig         |  2 +-
 net/sched/cls_cgroup.c    | 23 ++---------------------
 4 files changed, 6 insertions(+), 53 deletions(-)

The modular version of task_netprioidx() in include/net/netprio_cgroup.h can be removed.

#4 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

Quoted 26 lines · Li Zefan
Cc Daniel Borkmann <[email protected]>
Quoted 13 lines · Tejun Heo

net_cls and net_prio are the only cgroups which are allowed to be built as modules. The savings from allowing the two controllers to be built as modules are tiny especially given that cgroup module support itself adds quite a bit of complexity.

The following are the sizes of vmlinux with both built as module and both built as part of the kernel image with cgroup module support removed.

	text		data		bss		dec
	20292207	2411496		10784768	33488471
	20293421	2412568		10784768	33490757
Quoted 9 lines

The total difference is 2286 bytes. Given that none of other controllers has much chance of being made a module and that we're unlikely to add new modular controllers, the added complexity is simply not justifiable.

As a first step to drop cgroup module support, this patch changes the two config options to bool from tristate and drops module related code from the two controllers.

Quoted 7 lines

I sugguested Daniel to do this for net_cls, and the change has been in net-next.

https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=fe1217c4f3f7d7cbf8efdd8dd5fdc7204a1d65a8

I was planning to remove module support after that change goes into upstream. :)

I am fine with that, thanks Li.

Quoted 15 lines
Quoted 11 lines
Signed-off-by Tejun Heo <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>
---
  net/Kconfig               |  2 +-
  net/core/netprio_cgroup.c | 32 ++------------------------------
  net/sched/Kconfig         |  2 +-
  net/sched/cls_cgroup.c    | 23 ++---------------------
  4 files changed, 6 insertions(+), 53 deletions(-)

The modular version of task_netprioidx() in include/net/netprio_cgroup.h can be removed.

author maintainer
#5 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

In reply to Li Zefan

Hey,

Quoted 26 lines · Li Zefan
Cc Daniel Borkmann <[email protected]>
Quoted 13 lines · Tejun Heo

net_cls and net_prio are the only cgroups which are allowed to be built as modules. The savings from allowing the two controllers to be built as modules are tiny especially given that cgroup module support itself adds quite a bit of complexity.

The following are the sizes of vmlinux with both built as module and both built as part of the kernel image with cgroup module support removed.

	text		data		bss		dec
	20292207	2411496		10784768	33488471
	20293421	2412568		10784768	33490757
Quoted 9 lines

The total difference is 2286 bytes. Given that none of other controllers has much chance of being made a module and that we're unlikely to add new modular controllers, the added complexity is simply not justifiable.

As a first step to drop cgroup module support, this patch changes the two config options to bool from tristate and drops module related code from the two controllers.

Quoted 7 lines

I sugguested Daniel to do this for net_cls, and the change has been in net-next.

https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=fe1217c4f3f7d7cbf8efdd8dd5fdc7204a1d65a8

I was planning to remove module support after that change goes into upstream. :)

Ooh, cool. :)

Unless there's gonna be another rc, I think it's already a bit too late for 3.14 anyway. I'll drop the net_cls part and rebase the changes on top of rc1 later on.

Thanks.

Signature · 1 line
tejun
#6 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

Quoted 34 lines · Tejun Heo

Hey,

Quoted 31 lines · Li Zefan
Cc Daniel Borkmann <[email protected]>
Quoted 13 lines · Tejun Heo

net_cls and net_prio are the only cgroups which are allowed to be built as modules. The savings from allowing the two controllers to be built as modules are tiny especially given that cgroup module support itself adds quite a bit of complexity.

The following are the sizes of vmlinux with both built as module and both built as part of the kernel image with cgroup module support removed.

	text		data		bss		dec
	20292207	2411496		10784768	33488471
	20293421	2412568		10784768	33490757
Quoted 9 lines

The total difference is 2286 bytes. Given that none of other controllers has much chance of being made a module and that we're unlikely to add new modular controllers, the added complexity is simply not justifiable.

As a first step to drop cgroup module support, this patch changes the two config options to bool from tristate and drops module related code from the two controllers.

I sugguested Daniel to do this for net_cls, and the change has been in net-next.

https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=fe1217c4f3f7d7cbf8efdd8dd5fdc7204a1d65a8

Quoted 8 lines
Quoted 2 lines

I was planning to remove module support after that change goes into upstream. :)

Ooh, cool. :)

Unless there's gonna be another rc, I think it's already a bit too late for 3.14 anyway. I'll drop the net_cls part and rebase the changes on top of rc1 later on.

I think that's 1 patch of your series, right?

I think this one could still go through net-next if you want, that would avoid a merge conflict.

Best,

Daniel

author maintainer
#7 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

Quoted 5 lines · Daniel Borkmann
Quoted 3 lines

Unless there's gonna be another rc, I think it's already a bit too late for 3.14 anyway. I'll drop the net_cls part and rebase the changes on top of rc1 later on.

I think that's 1 patch of your series, right?

They overlap.

Quoted 2 lines

I think this one could still go through net-next if you want, that would avoid a merge conflict.

If there's gonna be another rc, I'll prolly just cherry-pick the commit from net-next and then rebase the rest on top of it. If there isn't gonna be another rc, I'm just gonna base everything on v3.14-rc1 which would include the patch from net-next.

Thanks.

Signature · 1 line
tejun
#8 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

Quoted 9 lines · Tejun Heo
Quoted 5 lines · Daniel Borkmann
Quoted 3 lines

Unless there's gonna be another rc, I think it's already a bit too late for 3.14 anyway. I'll drop the net_cls part and rebase the changes on top of rc1 later on.

I think that's 1 patch of your series, right?

They overlap.

Quoted 7 lines
Quoted 2 lines

I think this one could still go through net-next if you want, that would avoid a merge conflict.

If there's gonna be another rc, I'll prolly just cherry-pick the commit from net-next and then rebase the rest on top of it. If there isn't gonna be another rc, I'm just gonna base everything on v3.14-rc1 which would include the patch from net-next.

Ok, sounds good.

Thanks Tejun.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

Quoted 5 lines · Daniel Borkmann
Quoted 3 lines

Unless there's gonna be another rc, I think it's already a bit too late for 3.14 anyway. I'll drop the net_cls part and rebase the changes on top of rc1 later on.

I think that's 1 patch of your series, right?

They overlap.

Quoted 2 lines

I think this one could still go through net-next if you want, that would avoid a merge conflict.

If there's gonna be another rc, I'll prolly just cherry-pick the commit from net-next and then rebase the rest on top of it. If there isn't gonna be another rc, I'm just gonna base everything on v3.14-rc1 which would include the patch from net-next.

Thanks.

Signature · 1 line
tejun
maintainer Acked-by on v1 1/6
#11 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

In reply to Tejun Heo

From: Tejun Heo <[email protected]>
Date: Fri, 17 Jan 2014 13:11:52 -0500

Quoted 18 lines

net_cls and net_prio are the only cgroups which are allowed to be built as modules. The savings from allowing the two controllers to be built as modules are tiny especially given that cgroup module support itself adds quite a bit of complexity.

The following are the sizes of vmlinux with both built as module and both built as part of the kernel image with cgroup module support removed.

	text		data		bss		dec
	20292207	2411496		10784768	33488471
	20293421	2412568		10784768	33490757

The total difference is 2286 bytes. Given that none of other controllers has much chance of being made a module and that we're unlikely to add new modular controllers, the added complexity is simply not justifiable.

Quoted 5 lines

As a first step to drop cgroup module support, this patch changes the two config options to bool from tristate and drops module related code from the two controllers.

Signed-off-by Tejun Heo <[email protected]>
Acked-by David S. Miller <davem-fT/PcQaiUtIeIZ0/[email protected]>

A message that is not in the archive (sent off-list or not yet received): 1389982317-22225-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Acked-by
#13 lore raw

Re: [PATCH 1/6] cgroup: make CONFIG_NET_CLS_CGROUP and CONFIG_NETPRIO_CGROUP bool instead of tristate

From: Tejun Heo <[email protected]>
Date: Fri, 17 Jan 2014 13:11:52 -0500

Quoted 18 lines

net_cls and net_prio are the only cgroups which are allowed to be built as modules. The savings from allowing the two controllers to be built as modules are tiny especially given that cgroup module support itself adds quite a bit of complexity.

The following are the sizes of vmlinux with both built as module and both built as part of the kernel image with cgroup module support removed.

	text		data		bss		dec
	20292207	2411496		10784768	33488471
	20293421	2412568		10784768	33490757

The total difference is 2286 bytes. Given that none of other controllers has much chance of being made a module and that we're unlikely to add new modular controllers, the added complexity is simply not justifiable.

Quoted 5 lines

As a first step to drop cgroup module support, this patch changes the two config options to bool from tristate and drops module related code from the two controllers.

Signed-off-by Tejun Heo <[email protected]>
Acked-by David S. Miller <davem-fT/PcQaiUtIeIZ0/[email protected]>
author maintainer
#14 lore raw

PATCH 2/6 cgroup: drop module support

In reply to Tejun Heo

With module supported dropped from net_cls and net_prio, no controller is using cgroup module support. None of actual resource controllers can be built as a module and we aren't gonna add new controllers which don't control resources. This patch drops module support from cgroup.

* cgroup_[un]load_subsys() and cgroup_subsys->module removed.

* As there's no point in distinguishing IS_BUILTIN() and IS_MODULE(), cgroup_subsys.h now uses IS_ENABLED() directly.

* enum cgroup_subsys_id now exactly matches the list of enabled controllers as ordered in cgroup_subsys.h.

* cgroup_subsys[] is now a contiguously occupied array. Size specification is no longer necessary and dropped.

* for_each_builtin_subsys() is removed and for_each_subsys() is updated to not require any locking.

* module ref handling is removed from rebind_subsystems().

* Module related comments dropped.

Signed-off-by Tejun Heo <[email protected]>
---
 block/blk-cgroup.c            |   1 -
 include/linux/cgroup.h        |  29 +----
 include/linux/cgroup_subsys.h |  24 ++--
 kernel/cgroup.c               | 287 +++---------------------------------------
 net/core/netprio_cgroup.c     |   1 -
 net/sched/cls_cgroup.c        |   1 -
 6 files changed, 33 insertions(+), 310 deletions(-)
block/blk-cgroup.c +0 -1
index 4e491d9..660d419 100644
@@ -914,7 +914,6 @@ struct cgroup_subsys blkio_subsys = {
 	.can_attach = blkcg_can_attach,
 	.subsys_id = blkio_subsys_id,
 	.base_cftypes = blkcg_files,
-	.module = THIS_MODULE,
 };
 EXPORT_SYMBOL_GPL(blkio_subsys);
include/linux/cgroup.h +4 -25
index 5c09759..d842a73 100644
@@ -37,28 +37,13 @@ extern void cgroup_post_fork(struct task_struct *p);
 extern void cgroup_exit(struct task_struct *p, int run_callbacks);
 extern int cgroupstats_build(struct cgroupstats *stats,
 				struct dentry *dentry);
-extern int cgroup_load_subsys(struct cgroup_subsys *ss);
-extern void cgroup_unload_subsys(struct cgroup_subsys *ss);
 
 extern int proc_cgroup_show(struct seq_file *, void *);
 
-/*
- * Define the enumeration of all cgroup subsystems.
- *
- * We define ids for builtin subsystems and then modular ones.
- */
+/* define the enumeration of all cgroup subsystems */
 #define SUBSYS(_x) _x ## _subsys_id,
 enum cgroup_subsys_id {
-#define IS_SUBSYS_ENABLED(option) IS_BUILTIN(option)
 #include <linux/cgroup_subsys.h>
-#undef IS_SUBSYS_ENABLED
-	CGROUP_BUILTIN_SUBSYS_COUNT,
-
-	__CGROUP_SUBSYS_TEMP_PLACEHOLDER = CGROUP_BUILTIN_SUBSYS_COUNT - 1,
-
-#define IS_SUBSYS_ENABLED(option) IS_MODULE(option)
-#include <linux/cgroup_subsys.h>
-#undef IS_SUBSYS_ENABLED
 	CGROUP_SUBSYS_COUNT,
 };
 #undef SUBSYS

@@ -370,10 +355,9 @@ struct css_set {
 	struct list_head cgrp_links;
 
 	/*
-	 * Set of subsystem states, one for each subsystem. This array
-	 * is immutable after creation apart from the init_css_set
-	 * during subsystem registration (at boot time) and modular subsystem
-	 * loading/unloading.
+	 * Set of subsystem states, one for each subsystem. This array is
+	 * immutable after creation apart from the init_css_set during
+	 * subsystem registration (at boot time).
 	 */
 	struct cgroup_subsys_state *subsys[CGROUP_SUBSYS_COUNT];
 

@@ -620,15 +604,10 @@ struct cgroup_subsys {
 	/* base cftypes, automatically [de]registered with subsys itself */
 	struct cftype *base_cftypes;
 	struct cftype_set base_cftset;
-
-	/* should be defined only by modular subsystems */
-	struct module *module;
 };
 
 #define SUBSYS(_x) extern struct cgroup_subsys _x ## _subsys;
-#define IS_SUBSYS_ENABLED(option) IS_BUILTIN(option)
 #include <linux/cgroup_subsys.h>
-#undef IS_SUBSYS_ENABLED
 #undef SUBSYS
 
 /**
include/linux/cgroup_subsys.h +12 -12
index b613ffd..cc4cafe 100644
@@ -3,51 +3,51 @@
  *
  * DO NOT ADD ANY SUBSYSTEM WITHOUT EXPLICIT ACKS FROM CGROUP MAINTAINERS.
  */
-#if IS_SUBSYS_ENABLED(CONFIG_CPUSETS)
+#if IS_ENABLED(CONFIG_CPUSETS)
 SUBSYS(cpuset)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_DEBUG)
+#if IS_ENABLED(CONFIG_CGROUP_DEBUG)
 SUBSYS(debug)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_SCHED)
+#if IS_ENABLED(CONFIG_CGROUP_SCHED)
 SUBSYS(cpu_cgroup)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_CPUACCT)
+#if IS_ENABLED(CONFIG_CGROUP_CPUACCT)
 SUBSYS(cpuacct)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_MEMCG)
+#if IS_ENABLED(CONFIG_MEMCG)
 SUBSYS(mem_cgroup)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_DEVICE)
+#if IS_ENABLED(CONFIG_CGROUP_DEVICE)
 SUBSYS(devices)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_FREEZER)
+#if IS_ENABLED(CONFIG_CGROUP_FREEZER)
 SUBSYS(freezer)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_NET_CLS_CGROUP)
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
 SUBSYS(net_cls)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_BLK_CGROUP)
+#if IS_ENABLED(CONFIG_BLK_CGROUP)
 SUBSYS(blkio)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_PERF)
+#if IS_ENABLED(CONFIG_CGROUP_PERF)
 SUBSYS(perf)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_NETPRIO_CGROUP)
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 SUBSYS(net_prio)
 #endif
 
-#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_HUGETLB)
+#if IS_ENABLED(CONFIG_CGROUP_HUGETLB)
 SUBSYS(hugetlb)
 #endif
 /*
kernel/cgroup.c +17 -270

387 lines not on this page · raw message

net/core/netprio_cgroup.c +0 -1
index 8912da7..a34df14 100644
@@ -252,7 +252,6 @@ struct cgroup_subsys net_prio_subsys = {
 	.attach		= net_prio_attach,
 	.subsys_id	= net_prio_subsys_id,
 	.base_cftypes	= ss_files,
-	.module		= THIS_MODULE,
 };
 
 static int netprio_device_event(struct notifier_block *unused,
net/sched/cls_cgroup.c +0 -1
index 2f6a9b4..706ec7b 100644
@@ -111,7 +111,6 @@ struct cgroup_subsys net_cls_subsys = {
 	.attach		= cgrp_attach,
 	.subsys_id	= net_cls_subsys_id,
 	.base_cftypes	= ss_files,
-	.module		= THIS_MODULE,
 };
 
 struct cls_cgroup_head {
Signature · 1 line
1.8.4.2
author maintainer
#15 lore raw

PATCH 3/6 cgroup: clean up cgroup_subsys names and initialization

In reply to Tejun Heo

cgroup_subsys is a bit messier than it needs to be.

* The name of a subsys can be different from its internal identifier defined in cgroup_subsys.h. Most subsystems use the matching name but three - cpu, memory and perf_event - use different ones.

* cgroup_subsys_id enums are postfixed with _subsys_id and each cgroup_subsys is postfixed with _subsys. cgroup.h is widely included throughout various subsystems, it doesn't and shouldn't have claim on such generic names which don't have any qualifier indicating that they belong to cgroup.

* cgroup_subsys->subsys_id should always equal the matching cgroup_subsys_id enum; however, we require each controller to initialize it and then BUG if they don't match, which is a bit silly.

This patch cleans up cgroup_subsys names and initialization by doing the followings.

* cgroup_subsys_id enums are now postfixed with _cgrp_id, and each cgroup_subsys with _cgrp_subsys.

* With the above, renaming subsys identifiers to match the userland visible names doesn't cause any naming conflicts. All non-matching identifiers are renamed to match the official names.

cpu_cgroup -> cpu
mem_cgroup -> memory
perf -> perf_event

* controllers no longer need to initialize ->subsys_id and ->name. They're generated in cgroup core and set automatically during boot.

* Redundant cgroup_subsys declarations removed.

* While updating BUG_ON()s in cgroup_init_early(), convert them to WARN()s. BUGging that early during boot is stupid - the kernel can't print anything, even through serial console and the trap handler doesn't even link stack frame properly for back-tracing.

This patch doesn't introduce any behavior changes.

Signed-off-by Tejun Heo <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Balbir Singh <[email protected]>
Cc KAMEZAWA Hiroyuki <[email protected]>
Cc Aristeu Rozanski <[email protected]>
Cc Serge E. Hallyn <serue-r/[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>
---
 block/blk-cgroup.c             |  8 +++-----
 block/blk-cgroup.h             |  2 +-
 fs/bio.c                       |  2 +-
 include/linux/cgroup.h         |  7 ++++---
 include/linux/cgroup_subsys.h  |  6 +++---
 include/linux/hugetlb_cgroup.h |  2 +-
 include/linux/memcontrol.h     |  2 +-
 include/net/cls_cgroup.h       |  4 ++--
 include/net/netprio_cgroup.h   |  4 ++--
 kernel/cgroup.c                | 34 ++++++++++++++++++++--------------
 kernel/cgroup_freezer.c        |  8 ++------
 kernel/cpuset.c                | 10 ++++------
 kernel/events/core.c           |  8 +++-----
 kernel/sched/core.c            |  6 ++----
 kernel/sched/cpuacct.c         |  6 ++----
 mm/hugetlb_cgroup.c            |  9 +++------
 mm/memcontrol.c                | 22 ++++++++++------------
 net/core/netprio_cgroup.c      |  4 +---
 net/ipv4/tcp_memcontrol.c      |  2 +-
 net/sched/cls_cgroup.c         |  6 ++----
 security/device_cgroup.c       |  8 ++------
 21 files changed, 70 insertions(+), 90 deletions(-)
block/blk-cgroup.c +3 -5
index 660d419..1cef07c 100644
@@ -906,16 +906,14 @@ static int blkcg_can_attach(struct cgroup_subsys_state *css,
 	return ret;
 }
 
-struct cgroup_subsys blkio_subsys = {
-	.name = "blkio",
+struct cgroup_subsys blkio_cgrp_subsys = {
 	.css_alloc = blkcg_css_alloc,
 	.css_offline = blkcg_css_offline,
 	.css_free = blkcg_css_free,
 	.can_attach = blkcg_can_attach,
-	.subsys_id = blkio_subsys_id,
 	.base_cftypes = blkcg_files,
 };
-EXPORT_SYMBOL_GPL(blkio_subsys);
+EXPORT_SYMBOL_GPL(blkio_cgrp_subsys);
 
 /**
  * blkcg_activate_policy - activate a blkcg policy on a request_queue

@@ -1105,7 +1103,7 @@ int blkcg_policy_register(struct blkcg_policy *pol)
 
 	/* everything is in place, add intf files for the new policy */
 	if (pol->cftypes)
-		WARN_ON(cgroup_add_cftypes(&blkio_subsys, pol->cftypes));
+		WARN_ON(cgroup_add_cftypes(&blkio_cgrp_subsys, pol->cftypes));
 	ret = 0;
 out_unlock:
 	mutex_unlock(&blkcg_pol_mutex);
block/blk-cgroup.h +1 -1
index 1610b22..625d66f 100644
@@ -186,7 +186,7 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css)
 
 static inline struct blkcg *task_blkcg(struct task_struct *tsk)
 {
-	return css_to_blkcg(task_css(tsk, blkio_subsys_id));
+	return css_to_blkcg(task_css(tsk, blkio_cgrp_id));
 }
 
 static inline struct blkcg *bio_blkcg(struct bio *bio)
fs/bio.c +1 -1
index 33d79a4..26a62d7 100644
@@ -2002,7 +2002,7 @@ int bio_associate_current(struct bio *bio)
 
 	/* associate blkcg if exists */
 	rcu_read_lock();
-	css = task_css(current, blkio_subsys_id);
+	css = task_css(current, blkio_cgrp_id);
 	if (css && css_tryget(css))
 		bio->bi_css = css;
 	rcu_read_unlock();
include/linux/cgroup.h +4 -3
index d842a73..cd6611e 100644
@@ -41,7 +41,7 @@ extern int cgroupstats_build(struct cgroupstats *stats,
 extern int proc_cgroup_show(struct seq_file *, void *);
 
 /* define the enumeration of all cgroup subsystems */
-#define SUBSYS(_x) _x ## _subsys_id,
+#define SUBSYS(_x) _x ## _cgrp_id,
 enum cgroup_subsys_id {
 #include <linux/cgroup_subsys.h>
 	CGROUP_SUBSYS_COUNT,

@@ -573,7 +573,6 @@ struct cgroup_subsys {
 		     struct task_struct *task);
 	void (*bind)(struct cgroup_subsys_state *root_css);
 
-	int subsys_id;
 	int disabled;
 	int early_init;
 

@@ -592,6 +591,8 @@ struct cgroup_subsys {
 	bool broken_hierarchy;
 	bool warned_broken_hierarchy;
 
+	/* the following two fields are initialized automtically during boot */
+	int subsys_id;
 #define MAX_CGROUP_TYPE_NAMELEN 32
 	const char *name;
 

@@ -606,7 +607,7 @@ struct cgroup_subsys {
 	struct cftype_set base_cftset;
 };
 
-#define SUBSYS(_x) extern struct cgroup_subsys _x ## _subsys;
+#define SUBSYS(_x) extern struct cgroup_subsys _x ## _cgrp_subsys;
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
include/linux/cgroup_subsys.h +3 -3
index cc4cafe..ce9244d 100644
@@ -12,7 +12,7 @@ SUBSYS(debug)
 #endif
 
 #if IS_ENABLED(CONFIG_CGROUP_SCHED)
-SUBSYS(cpu_cgroup)
+SUBSYS(cpu)
 #endif
 
 #if IS_ENABLED(CONFIG_CGROUP_CPUACCT)

@@ -20,7 +20,7 @@ SUBSYS(cpuacct)
 #endif
 
 #if IS_ENABLED(CONFIG_MEMCG)
-SUBSYS(mem_cgroup)
+SUBSYS(memory)
 #endif
 
 #if IS_ENABLED(CONFIG_CGROUP_DEVICE)

@@ -40,7 +40,7 @@ SUBSYS(blkio)
 #endif
 
 #if IS_ENABLED(CONFIG_CGROUP_PERF)
-SUBSYS(perf)
+SUBSYS(perf_event)
 #endif
 
 #if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
include/linux/hugetlb_cgroup.h +1 -1
index ce8217f..e54ba72 100644
@@ -48,7 +48,7 @@ int set_hugetlb_cgroup(struct page *page, struct hugetlb_cgroup *h_cg)
 
 static inline bool hugetlb_cgroup_disabled(void)
 {
-	if (hugetlb_subsys.disabled)
+	if (hugetlb_cgrp_subsys.disabled)
 		return true;
 	return false;
 }
include/linux/memcontrol.h +1 -1
index b3e7a66..65d4bf4 100644
@@ -162,7 +162,7 @@ extern int do_swap_account;
 
 static inline bool mem_cgroup_disabled(void)
 {
-	if (mem_cgroup_subsys.disabled)
+	if (memory_cgrp_subsys.disabled)
 		return true;
 	return false;
 }
include/net/cls_cgroup.h +2 -2
index 33d03b6..8807e0e 100644
@@ -35,7 +35,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
 		return 0;
 
 	rcu_read_lock();
-	classid = container_of(task_css(p, net_cls_subsys_id),
+	classid = container_of(task_css(p, net_cls_cgrp_id),
 			       struct cgroup_cls_state, css)->classid;
 	rcu_read_unlock();
 

@@ -51,7 +51,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
 		return 0;
 
 	rcu_read_lock();
-	css = task_css(p, net_cls_subsys_id);
+	css = task_css(p, net_cls_cgrp_id);
 	if (css)
 		classid = container_of(css,
 				       struct cgroup_cls_state, css)->classid;
include/net/netprio_cgroup.h +2 -2
index 099d027..058ea66 100644
@@ -35,7 +35,7 @@ static inline u32 task_netprioidx(struct task_struct *p)
 	u32 idx;
 
 	rcu_read_lock();
-	css = task_css(p, net_prio_subsys_id);
+	css = task_css(p, net_prio_cgrp_id);
 	idx = css->cgroup->id;
 	rcu_read_unlock();
 	return idx;

@@ -49,7 +49,7 @@ static inline u32 task_netprioidx(struct task_struct *p)
 	u32 idx = 0;
 
 	rcu_read_lock();
-	css = task_css(p, net_prio_subsys_id);
+	css = task_css(p, net_prio_cgrp_id);
 	if (css)
 		idx = css->cgroup->id;
 	rcu_read_unlock();
kernel/cgroup.c +20 -14
index db97ca9..aace2ee 100644
@@ -120,10 +120,18 @@ static struct workqueue_struct *cgroup_destroy_wq;
 static struct workqueue_struct *cgroup_pidlist_destroy_wq;
 
 /* generate an array of cgroup subsystem pointers */
-#define SUBSYS(_x) [_x ## _subsys_id] = &_x ## _subsys,
+#define SUBSYS(_x) [_x ## _cgrp_id] = &_x ## _cgrp_subsys,
 static struct cgroup_subsys *cgroup_subsys[] = {
 #include <linux/cgroup_subsys.h>
 };
+#undef SUBSYS
+
+/* array of cgroup subsystem names */
+#define SUBSYS(_x) [_x ## _cgrp_id] = #_x,
+static const char *cgroup_subsys_name[] = {
+#include <linux/cgroup_subsys.h>
+};
+#undef SUBSYS
 
 /*
  * The dummy hierarchy, reserved for the subsystems that are otherwise

@@ -1066,7 +1074,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 	BUG_ON(!mutex_is_locked(&cgroup_mutex));
 
 #ifdef CONFIG_CPUSETS
-	mask = ~(1UL << cpuset_subsys_id);
+	mask = ~(1UL << cpuset_cgrp_id);
 #endif
 
 	memset(opts, 0, sizeof(*opts));

@@ -4524,15 +4532,15 @@ int __init cgroup_init_early(void)
 	list_add(&init_cgrp_cset_link.cgrp_link, &init_css_set.cgrp_links);
 
 	for_each_subsys(ss, i) {
-		BUG_ON(!ss->name);
-		BUG_ON(strlen(ss->name) > MAX_CGROUP_TYPE_NAMELEN);
-		BUG_ON(!ss->css_alloc);
-		BUG_ON(!ss->css_free);
-		if (ss->subsys_id != i) {
-			printk(KERN_ERR "cgroup: Subsys %s id == %d\n",
-			       ss->name, ss->subsys_id);
-			BUG();
-		}
+		WARN(!ss->css_alloc || !ss->css_free || ss->name || ss->subsys_id,
+		     "invalid cgroup_subsys %d:%s css_alloc=%p css_free=%p name:id=%d:%s\n",
+		     i, cgroup_subsys_name[i], ss->css_alloc, ss->css_free,
+		     ss->subsys_id, ss->name);
+		WARN(strlen(cgroup_subsys_name[i]) > MAX_CGROUP_TYPE_NAMELEN,
+		     "cgroup_subsys_name %s too long\n", cgroup_subsys_name[i]);
+
+		ss->subsys_id = i;
+		ss->name = cgroup_subsys_name[i];
 
 		if (ss->early_init)
 			cgroup_init_subsys(ss);

@@ -5162,11 +5170,9 @@ static struct cftype debug_files[] =  {
 	{ }	/* terminate */
 };
 
-struct cgroup_subsys debug_subsys = {
-	.name = "debug",
+struct cgroup_subsys debug_cgrp_subsys = {
 	.css_alloc = debug_css_alloc,
 	.css_free = debug_css_free,
-	.subsys_id = debug_subsys_id,
 	.base_cftypes = debug_files,
 };
 #endif /* CONFIG_CGROUP_DEBUG */
kernel/cgroup_freezer.c +2 -6
index 6c3154e..98ea26a9 100644
@@ -52,7 +52,7 @@ static inline struct freezer *css_freezer(struct cgroup_subsys_state *css)
 
 static inline struct freezer *task_freezer(struct task_struct *task)
 {
-	return css_freezer(task_css(task, freezer_subsys_id));
+	return css_freezer(task_css(task, freezer_cgrp_id));
 }
 
 static struct freezer *parent_freezer(struct freezer *freezer)

@@ -84,8 +84,6 @@ static const char *freezer_state_strs(unsigned int state)
 	return "THAWED";
 };
 
-struct cgroup_subsys freezer_subsys;
-
 static struct cgroup_subsys_state *
 freezer_css_alloc(struct cgroup_subsys_state *parent_css)
 {

@@ -473,13 +471,11 @@ static struct cftype files[] = {
 	{ }	/* terminate */
 };
 
-struct cgroup_subsys freezer_subsys = {
-	.name		= "freezer",
+struct cgroup_subsys freezer_cgrp_subsys = {
 	.css_alloc	= freezer_css_alloc,
 	.css_online	= freezer_css_online,
 	.css_offline	= freezer_css_offline,
 	.css_free	= freezer_css_free,
-	.subsys_id	= freezer_subsys_id,
 	.attach		= freezer_attach,
 	.fork		= freezer_fork,
 	.base_cftypes	= files,
kernel/cpuset.c +4 -6
index 4410ac6..2d018c7 100644
@@ -119,7 +119,7 @@ static inline struct cpuset *css_cs(struct cgroup_subsys_state *css)
 /* Retrieve the cpuset for a task */
 static inline struct cpuset *task_cs(struct task_struct *task)
 {
-	return css_cs(task_css(task, cpuset_subsys_id));
+	return css_cs(task_css(task, cpuset_cgrp_id));
 }
 
 static inline struct cpuset *parent_cs(struct cpuset *cs)

@@ -1521,7 +1521,7 @@ static void cpuset_attach(struct cgroup_subsys_state *css,
 	struct task_struct *task;
 	struct task_struct *leader = cgroup_taskset_first(tset);
 	struct cgroup_subsys_state *oldcss = cgroup_taskset_cur_css(tset,
-							cpuset_subsys_id);
+							cpuset_cgrp_id);
 	struct cpuset *cs = css_cs(css);
 	struct cpuset *oldcs = css_cs(oldcss);
 	struct cpuset *cpus_cs = effective_cpumask_cpuset(cs);

@@ -2024,8 +2024,7 @@ static void cpuset_css_free(struct cgroup_subsys_state *css)
 	kfree(cs);
 }
 
-struct cgroup_subsys cpuset_subsys = {
-	.name = "cpuset",
+struct cgroup_subsys cpuset_cgrp_subsys = {
 	.css_alloc = cpuset_css_alloc,
 	.css_online = cpuset_css_online,
 	.css_offline = cpuset_css_offline,

@@ -2033,7 +2032,6 @@ struct cgroup_subsys cpuset_subsys = {
 	.can_attach = cpuset_can_attach,
 	.cancel_attach = cpuset_cancel_attach,
 	.attach = cpuset_attach,
-	.subsys_id = cpuset_subsys_id,
 	.base_cftypes = files,
 	.early_init = 1,
 };

@@ -2699,7 +2697,7 @@ int proc_cpuset_show(struct seq_file *m, void *unused_v)
 		goto out_free;
 
 	rcu_read_lock();
-	css = task_css(tsk, cpuset_subsys_id);
+	css = task_css(tsk, cpuset_cgrp_id);
 	retval = cgroup_path(css->cgroup, buf, PAGE_SIZE);
 	rcu_read_unlock();
 	if (retval < 0)
kernel/events/core.c +3 -5
index d724e77..1288aab 100644
@@ -341,7 +341,7 @@ struct perf_cgroup {
 static inline struct perf_cgroup *
 perf_cgroup_from_task(struct task_struct *task)
 {
-	return container_of(task_css(task, perf_subsys_id),
+	return container_of(task_css(task, perf_event_cgrp_id),
 			    struct perf_cgroup, css);
 }
 

@@ -594,7 +594,7 @@ static inline int perf_cgroup_connect(int fd, struct perf_event *event,
 
 	rcu_read_lock();
 
-	css = css_from_dir(f.file->f_dentry, &perf_subsys);
+	css = css_from_dir(f.file->f_dentry, &perf_event_cgrp_subsys);
 	if (IS_ERR(css)) {
 		ret = PTR_ERR(css);
 		goto out;

@@ -8025,9 +8025,7 @@ static void perf_cgroup_exit(struct cgroup_subsys_state *css,
 	task_function_call(task, __perf_cgroup_move, task);
 }
 
-struct cgroup_subsys perf_subsys = {
-	.name		= "perf_event",
-	.subsys_id	= perf_subsys_id,
+struct cgroup_subsys perf_event_cgrp_subsys = {
 	.css_alloc	= perf_cgroup_css_alloc,
 	.css_free	= perf_cgroup_css_free,
 	.exit		= perf_cgroup_exit,
kernel/sched/core.c +2 -4
index 7e8cbb9..3e9b81f 100644
@@ -6604,7 +6604,7 @@ void sched_move_task(struct task_struct *tsk)
 	if (unlikely(running))
 		tsk->sched_class->put_prev_task(rq, tsk);
 
-	tg = container_of(task_css_check(tsk, cpu_cgroup_subsys_id,
+	tg = container_of(task_css_check(tsk, cpu_cgrp_id,
 				lockdep_is_held(&tsk->sighand->siglock)),
 			  struct task_group, css);
 	tg = autogroup_task_group(tsk, tg);

@@ -7335,8 +7335,7 @@ static struct cftype cpu_files[] = {
 	{ }	/* terminate */
 };
 
-struct cgroup_subsys cpu_cgroup_subsys = {
-	.name		= "cpu",
+struct cgroup_subsys cpu_cgrp_subsys = {
 	.css_alloc	= cpu_cgroup_css_alloc,
 	.css_free	= cpu_cgroup_css_free,
 	.css_online	= cpu_cgroup_css_online,

@@ -7344,7 +7343,6 @@ struct cgroup_subsys cpu_cgroup_subsys = {
 	.can_attach	= cpu_cgroup_can_attach,
 	.attach		= cpu_cgroup_attach,
 	.exit		= cpu_cgroup_exit,
-	.subsys_id	= cpu_cgroup_subsys_id,
 	.base_cftypes	= cpu_files,
 	.early_init	= 1,
 };
kernel/sched/cpuacct.c +2 -4
index 622e081..c143ee3 100644
@@ -41,7 +41,7 @@ static inline struct cpuacct *css_ca(struct cgroup_subsys_state *css)
 /* return cpu accounting group to which this task belongs */
 static inline struct cpuacct *task_ca(struct task_struct *tsk)
 {
-	return css_ca(task_css(tsk, cpuacct_subsys_id));
+	return css_ca(task_css(tsk, cpuacct_cgrp_id));
 }
 
 static inline struct cpuacct *parent_ca(struct cpuacct *ca)

@@ -275,11 +275,9 @@ void cpuacct_account_field(struct task_struct *p, int index, u64 val)
 	rcu_read_unlock();
 }
 
-struct cgroup_subsys cpuacct_subsys = {
-	.name		= "cpuacct",
+struct cgroup_subsys cpuacct_cgrp_subsys = {
 	.css_alloc	= cpuacct_css_alloc,
 	.css_free	= cpuacct_css_free,
-	.subsys_id	= cpuacct_subsys_id,
 	.base_cftypes	= files,
 	.early_init	= 1,
 };
mm/hugetlb_cgroup.c +3 -6
index d747a84..33a430c 100644
@@ -30,7 +30,6 @@ struct hugetlb_cgroup {
 #define MEMFILE_IDX(val)	(((val) >> 16) & 0xffff)
 #define MEMFILE_ATTR(val)	((val) & 0xffff)
 
-struct cgroup_subsys hugetlb_subsys __read_mostly;
 static struct hugetlb_cgroup *root_h_cgroup __read_mostly;
 
 static inline

@@ -42,7 +41,7 @@ struct hugetlb_cgroup *hugetlb_cgroup_from_css(struct cgroup_subsys_state *s)
 static inline
 struct hugetlb_cgroup *hugetlb_cgroup_from_task(struct task_struct *task)
 {
-	return hugetlb_cgroup_from_css(task_css(task, hugetlb_subsys_id));
+	return hugetlb_cgroup_from_css(task_css(task, hugetlb_cgrp_id));
 }
 
 static inline bool hugetlb_cgroup_is_root(struct hugetlb_cgroup *h_cg)

@@ -358,7 +357,7 @@ static void __init __hugetlb_cgroup_file_init(int idx)
 	cft = &h->cgroup_files[4];
 	memset(cft, 0, sizeof(*cft));
 
-	WARN_ON(cgroup_add_cftypes(&hugetlb_subsys, h->cgroup_files));
+	WARN_ON(cgroup_add_cftypes(&hugetlb_cgrp_subsys, h->cgroup_files));
 
 	return;
 }

@@ -402,10 +401,8 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 	return;
 }
 
-struct cgroup_subsys hugetlb_subsys = {
-	.name = "hugetlb",
+struct cgroup_subsys hugetlb_cgrp_subsys = {
 	.css_alloc	= hugetlb_cgroup_css_alloc,
 	.css_offline	= hugetlb_cgroup_css_offline,
 	.css_free	= hugetlb_cgroup_css_free,
-	.subsys_id	= hugetlb_subsys_id,
 };
mm/memcontrol.c +10 -12
index 9252219..0adfdb1 100644
@@ -67,8 +67,8 @@
 
 #include <trace/events/vmscan.h>
 
-struct cgroup_subsys mem_cgroup_subsys __read_mostly;
-EXPORT_SYMBOL(mem_cgroup_subsys);
+struct cgroup_subsys memory_cgrp_subsys __read_mostly;
+EXPORT_SYMBOL(memory_cgrp_subsys);
 
 #define MEM_CGROUP_RECLAIM_RETRIES	5
 static struct mem_cgroup *root_mem_cgroup __read_mostly;

@@ -560,7 +560,7 @@ static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id)
 {
 	struct cgroup_subsys_state *css;
 
-	css = css_from_id(id - 1, &mem_cgroup_subsys);
+	css = css_from_id(id - 1, &memory_cgrp_subsys);
 	return mem_cgroup_from_css(css);
 }
 

@@ -1094,7 +1094,7 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
 	if (unlikely(!p))
 		return NULL;
 
-	return mem_cgroup_from_css(task_css(p, mem_cgroup_subsys_id));
+	return mem_cgroup_from_css(task_css(p, memory_cgrp_id));
 }
 
 struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm)

@@ -1706,7 +1706,7 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
 	rcu_read_lock();
 
 	mem_cgrp = memcg->css.cgroup;
-	task_cgrp = task_cgroup(p, mem_cgroup_subsys_id);
+	task_cgrp = task_cgroup(p, memory_cgrp_id);
 
 	ret = cgroup_path(task_cgrp, memcg_name, PATH_MAX);
 	if (ret < 0) {

@@ -6187,7 +6187,7 @@ static int memcg_write_event_control(struct cgroup_subsys_state *css,
 
 	ret = -EINVAL;
 	cfile_css = css_from_dir(cfile.file->f_dentry->d_parent,
-				 &mem_cgroup_subsys);
+				 &memory_cgrp_subsys);
 	if (cfile_css == css && css_tryget(css))
 		ret = 0;
 

@@ -6576,10 +6576,10 @@ mem_cgroup_css_online(struct cgroup_subsys_state *css)
 		 * unfortunate state in our controller.
 		 */
 		if (parent != root_mem_cgroup)
-			mem_cgroup_subsys.broken_hierarchy = true;
+			memory_cgrp_subsys.broken_hierarchy = true;
 	}
 
-	error = memcg_init_kmem(memcg, &mem_cgroup_subsys);
+	error = memcg_init_kmem(memcg, &memory_cgrp_subsys);
 	mutex_unlock(&memcg_create_mutex);
 	return error;
 }

@@ -7239,9 +7239,7 @@ static void mem_cgroup_bind(struct cgroup_subsys_state *root_css)
 		mem_cgroup_from_css(root_css)->use_hierarchy = true;
 }
 
-struct cgroup_subsys mem_cgroup_subsys = {
-	.name = "memory",
-	.subsys_id = mem_cgroup_subsys_id,
+struct cgroup_subsys memory_cgrp_subsys = {
 	.css_alloc = mem_cgroup_css_alloc,
 	.css_online = mem_cgroup_css_online,
 	.css_offline = mem_cgroup_css_offline,

@@ -7267,7 +7265,7 @@ __setup("swapaccount=", enable_swap_account);
 
 static void __init memsw_file_init(void)
 {
-	WARN_ON(cgroup_add_cftypes(&mem_cgroup_subsys, memsw_cgroup_files));
+	WARN_ON(cgroup_add_cftypes(&memory_cgrp_subsys, memsw_cgroup_files));
 }
 
 static void __init enable_swap_cgroup(void)
net/core/netprio_cgroup.c +1 -3
index a34df14..ece50fc 100644
@@ -244,13 +244,11 @@ static struct cftype ss_files[] = {
 	{ }	/* terminate */
 };
 
-struct cgroup_subsys net_prio_subsys = {
-	.name		= "net_prio",
+struct cgroup_subsys net_prio_cgrp_subsys = {
 	.css_alloc	= cgrp_css_alloc,
 	.css_online	= cgrp_css_online,
 	.css_free	= cgrp_css_free,
 	.attach		= net_prio_attach,
-	.subsys_id	= net_prio_subsys_id,
 	.base_cftypes	= ss_files,
 };
net/ipv4/tcp_memcontrol.c +1 -1
index 03e9154..d441c1a 100644
@@ -228,7 +228,7 @@ static struct cftype tcp_files[] = {
 
 static int __init tcp_memcontrol_init(void)
 {
-	WARN_ON(cgroup_add_cftypes(&mem_cgroup_subsys, tcp_files));
+	WARN_ON(cgroup_add_cftypes(&memory_cgrp_subsys, tcp_files));
 	return 0;
 }
 __initcall(tcp_memcontrol_init);
net/sched/cls_cgroup.c +2 -4
index 706ec7b..89ce1df 100644
@@ -30,7 +30,7 @@ static inline struct cgroup_cls_state *css_cls_state(struct cgroup_subsys_state
 
 static inline struct cgroup_cls_state *task_cls_state(struct task_struct *p)
 {
-	return css_cls_state(task_css(p, net_cls_subsys_id));
+	return css_cls_state(task_css(p, net_cls_cgrp_id));
 }
 
 static struct cgroup_subsys_state *

@@ -103,13 +103,11 @@ static struct cftype ss_files[] = {
 	{ }	/* terminate */
 };
 
-struct cgroup_subsys net_cls_subsys = {
-	.name		= "net_cls",
+struct cgroup_subsys net_cls_cgrp_subsys = {
 	.css_alloc	= cgrp_css_alloc,
 	.css_online	= cgrp_css_online,
 	.css_free	= cgrp_css_free,
 	.attach		= cgrp_attach,
-	.subsys_id	= net_cls_subsys_id,
 	.base_cftypes	= ss_files,
 };
security/device_cgroup.c +2 -6
index d3b6d2c..7f88bcd 100644
@@ -58,11 +58,9 @@ static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s)
 
 static inline struct dev_cgroup *task_devcgroup(struct task_struct *task)
 {
-	return css_to_devcgroup(task_css(task, devices_subsys_id));
+	return css_to_devcgroup(task_css(task, devices_cgrp_id));
 }
 
-struct cgroup_subsys devices_subsys;
-
 /*
  * called under devcgroup_mutex
  */

@@ -684,13 +682,11 @@ static struct cftype dev_cgroup_files[] = {
 	{ }	/* terminate */
 };
 
-struct cgroup_subsys devices_subsys = {
-	.name = "devices",
+struct cgroup_subsys devices_cgrp_subsys = {
 	.css_alloc = devcgroup_css_alloc,
 	.css_free = devcgroup_css_free,
 	.css_online = devcgroup_online,
 	.css_offline = devcgroup_offline,
-	.subsys_id = devices_subsys_id,
 	.base_cftypes = dev_cgroup_files,
 };
Signature · 1 line
1.8.4.2
Acked-by on v1 3/6
#16 lore raw

Re: [PATCH 3/6] cgroup: clean up cgroup_subsys names and initialization

Quoted 43 lines · Tejun Heo

cgroup_subsys is a bit messier than it needs to be.

* The name of a subsys can be different from its internal identifier defined in cgroup_subsys.h. Most subsystems use the matching name but three - cpu, memory and perf_event - use different ones.

* cgroup_subsys_id enums are postfixed with _subsys_id and each cgroup_subsys is postfixed with _subsys. cgroup.h is widely included throughout various subsystems, it doesn't and shouldn't have claim on such generic names which don't have any qualifier indicating that they belong to cgroup.

* cgroup_subsys->subsys_id should always equal the matching cgroup_subsys_id enum; however, we require each controller to initialize it and then BUG if they don't match, which is a bit silly.

This patch cleans up cgroup_subsys names and initialization by doing the followings.

* cgroup_subsys_id enums are now postfixed with _cgrp_id, and each cgroup_subsys with _cgrp_subsys.

* With the above, renaming subsys identifiers to match the userland visible names doesn't cause any naming conflicts. All non-matching identifiers are renamed to match the official names.

cpu_cgroup -> cpu
mem_cgroup -> memory
perf -> perf_event

* controllers no longer need to initialize ->subsys_id and ->name. They're generated in cgroup core and set automatically during boot.

* Redundant cgroup_subsys declarations removed.

* While updating BUG_ON()s in cgroup_init_early(), convert them to WARN()s. BUGging that early during boot is stupid - the kernel can't print anything, even through serial console and the trap handler doesn't even link stack frame properly for back-tracing.

This patch doesn't introduce any behavior changes.

Quoted 15 lines
Signed-off-by Tejun Heo <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Balbir Singh <[email protected]>
Cc KAMEZAWA Hiroyuki <[email protected]>
Cc Aristeu Rozanski <[email protected]>
Cc Serge E. Hallyn <serue-r/[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>

For the netprio and net_cls bits

Acked-by Neil Horman <[email protected]>

A message that is not in the archive (sent off-list or not yet received): 1389982317-22225-4-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Acked-by
#18 lore raw

Re: [PATCH 3/6] cgroup: clean up cgroup_subsys names and initialization

Quoted 43 lines · Tejun Heo

cgroup_subsys is a bit messier than it needs to be.

* The name of a subsys can be different from its internal identifier defined in cgroup_subsys.h. Most subsystems use the matching name but three - cpu, memory and perf_event - use different ones.

* cgroup_subsys_id enums are postfixed with _subsys_id and each cgroup_subsys is postfixed with _subsys. cgroup.h is widely included throughout various subsystems, it doesn't and shouldn't have claim on such generic names which don't have any qualifier indicating that they belong to cgroup.

* cgroup_subsys->subsys_id should always equal the matching cgroup_subsys_id enum; however, we require each controller to initialize it and then BUG if they don't match, which is a bit silly.

This patch cleans up cgroup_subsys names and initialization by doing the followings.

* cgroup_subsys_id enums are now postfixed with _cgrp_id, and each cgroup_subsys with _cgrp_subsys.

* With the above, renaming subsys identifiers to match the userland visible names doesn't cause any naming conflicts. All non-matching identifiers are renamed to match the official names.

cpu_cgroup -> cpu
mem_cgroup -> memory
perf -> perf_event

* controllers no longer need to initialize ->subsys_id and ->name. They're generated in cgroup core and set automatically during boot.

* Redundant cgroup_subsys declarations removed.

* While updating BUG_ON()s in cgroup_init_early(), convert them to WARN()s. BUGging that early during boot is stupid - the kernel can't print anything, even through serial console and the trap handler doesn't even link stack frame properly for back-tracing.

This patch doesn't introduce any behavior changes.

Quoted 15 lines
Signed-off-by Tejun Heo <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Balbir Singh <[email protected]>
Cc KAMEZAWA Hiroyuki <[email protected]>
Cc Aristeu Rozanski <[email protected]>
Cc Serge E. Hallyn <serue-r/[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>

For the netprio and net_cls bits

Acked-by Neil Horman <[email protected]>
Acked-by
#19 lore raw

Re: [PATCH 3/6] cgroup: clean up cgroup_subsys names and initialization

In reply to a message not in the archive
Quoted 43 lines · Tejun Heo

cgroup_subsys is a bit messier than it needs to be.

* The name of a subsys can be different from its internal identifier defined in cgroup_subsys.h. Most subsystems use the matching name but three - cpu, memory and perf_event - use different ones.

* cgroup_subsys_id enums are postfixed with _subsys_id and each cgroup_subsys is postfixed with _subsys. cgroup.h is widely included throughout various subsystems, it doesn't and shouldn't have claim on such generic names which don't have any qualifier indicating that they belong to cgroup.

* cgroup_subsys->subsys_id should always equal the matching cgroup_subsys_id enum; however, we require each controller to initialize it and then BUG if they don't match, which is a bit silly.

This patch cleans up cgroup_subsys names and initialization by doing the followings.

* cgroup_subsys_id enums are now postfixed with _cgrp_id, and each cgroup_subsys with _cgrp_subsys.

* With the above, renaming subsys identifiers to match the userland visible names doesn't cause any naming conflicts. All non-matching identifiers are renamed to match the official names.

cpu_cgroup -> cpu
mem_cgroup -> memory
perf -> perf_event

* controllers no longer need to initialize ->subsys_id and ->name. They're generated in cgroup core and set automatically during boot.

* Redundant cgroup_subsys declarations removed.

* While updating BUG_ON()s in cgroup_init_early(), convert them to WARN()s. BUGging that early during boot is stupid - the kernel can't print anything, even through serial console and the trap handler doesn't even link stack frame properly for back-tracing.

This patch doesn't introduce any behavior changes.

Quoted 15 lines
Signed-off-by Tejun Heo <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Balbir Singh <[email protected]>
Cc KAMEZAWA Hiroyuki <[email protected]>
Cc Aristeu Rozanski <[email protected]>
Cc Serge E. Hallyn <serue-r/[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>

For memcg and hugetlb

Acked-by Michal Hocko <[email protected]>

[...]

Thanks!

Signature · 2 lines
Michal Hocko
SUSE Labs
Acked-by on v1 3/6
#21 lore raw

Re: [PATCH 3/6] cgroup: clean up cgroup_subsys names and initialization

In reply to Tejun Heo
Quoted 43 lines · Tejun Heo

cgroup_subsys is a bit messier than it needs to be.

* The name of a subsys can be different from its internal identifier defined in cgroup_subsys.h. Most subsystems use the matching name but three - cpu, memory and perf_event - use different ones.

* cgroup_subsys_id enums are postfixed with _subsys_id and each cgroup_subsys is postfixed with _subsys. cgroup.h is widely included throughout various subsystems, it doesn't and shouldn't have claim on such generic names which don't have any qualifier indicating that they belong to cgroup.

* cgroup_subsys->subsys_id should always equal the matching cgroup_subsys_id enum; however, we require each controller to initialize it and then BUG if they don't match, which is a bit silly.

This patch cleans up cgroup_subsys names and initialization by doing the followings.

* cgroup_subsys_id enums are now postfixed with _cgrp_id, and each cgroup_subsys with _cgrp_subsys.

* With the above, renaming subsys identifiers to match the userland visible names doesn't cause any naming conflicts. All non-matching identifiers are renamed to match the official names.

cpu_cgroup -> cpu
mem_cgroup -> memory
perf -> perf_event

* controllers no longer need to initialize ->subsys_id and ->name. They're generated in cgroup core and set automatically during boot.

* Redundant cgroup_subsys declarations removed.

* While updating BUG_ON()s in cgroup_init_early(), convert them to WARN()s. BUGging that early during boot is stupid - the kernel can't print anything, even through serial console and the trap handler doesn't even link stack frame properly for back-tracing.

This patch doesn't introduce any behavior changes.

Quoted 15 lines
Signed-off-by Tejun Heo <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Balbir Singh <[email protected]>
Cc KAMEZAWA Hiroyuki <[email protected]>
Cc Aristeu Rozanski <[email protected]>
Cc Serge E. Hallyn <serue-r/[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Neil Horman <[email protected]>
Cc Thomas Graf <tgraf-G/[email protected]>
Cc "David S. Miller" <davem-fT/PcQaiUtIeIZ0/[email protected]>

For memcg and hugetlb

Acked-by Michal Hocko <[email protected]>

[...]

Thanks!

Signature · 2 lines
Michal Hocko
SUSE Labs
author maintainer
#22 lore raw

PATCH 4/6 cgroup: rename cgroup_subsys->subsys_id to ->id

In reply to Tejun Heo

It's no longer referenced outside cgroup core, so renaming is easy. Let's rename it for consistency & brevity.

This patch is pure rename.

Signed-off-by Tejun Heo <[email protected]>
---
 include/linux/cgroup.h |  4 ++--
 kernel/cgroup.c        | 18 +++++++++---------
 2 files changed, 11 insertions(+), 11 deletions(-)
include/linux/cgroup.h +2 -2
index cd6611e..198c7fc 100644
@@ -548,7 +548,7 @@ int cgroup_taskset_size(struct cgroup_taskset *tset);
 	     (task) = cgroup_taskset_next((tset)))			\
 		if (!(skip_css) ||					\
 		    cgroup_taskset_cur_css((tset),			\
-			(skip_css)->ss->subsys_id) != (skip_css))
+			(skip_css)->ss->id) != (skip_css))
 
 /*
  * Control Group subsystem type.

@@ -592,7 +592,7 @@ struct cgroup_subsys {
 	bool warned_broken_hierarchy;
 
 	/* the following two fields are initialized automtically during boot */
-	int subsys_id;
+	int id;
 #define MAX_CGROUP_TYPE_NAMELEN 32
 	const char *name;
kernel/cgroup.c +9 -9
index aace2ee..3b2fda0 100644
@@ -198,7 +198,7 @@ static struct cgroup_subsys_state *cgroup_css(struct cgroup *cgrp,
 					      struct cgroup_subsys *ss)
 {
 	if (ss)
-		return rcu_dereference_check(cgrp->subsys[ss->subsys_id],
+		return rcu_dereference_check(cgrp->subsys[ss->id],
 					     lockdep_is_held(&cgroup_mutex));
 	else
 		return &cgrp->dummy_css;

@@ -4002,7 +4002,7 @@ static int online_css(struct cgroup_subsys_state *css)
 	if (!ret) {
 		css->flags |= CSS_ONLINE;
 		css->cgroup->nr_css++;
-		rcu_assign_pointer(css->cgroup->subsys[ss->subsys_id], css);
+		rcu_assign_pointer(css->cgroup->subsys[ss->id], css);
 	}
 	return ret;
 }

@@ -4022,7 +4022,7 @@ static void offline_css(struct cgroup_subsys_state *css)
 
 	css->flags &= ~CSS_ONLINE;
 	css->cgroup->nr_css--;
-	RCU_INIT_POINTER(css->cgroup->subsys[ss->subsys_id], css);
+	RCU_INIT_POINTER(css->cgroup->subsys[ss->id], css);
 }
 
 /**

@@ -4053,7 +4053,7 @@ static int create_css(struct cgroup *cgrp, struct cgroup_subsys *ss)
 
 	init_css(css, ss, cgrp);
 
-	err = cgroup_populate_dir(cgrp, 1 << ss->subsys_id);
+	err = cgroup_populate_dir(cgrp, 1 << ss->id);
 	if (err)
 		goto err_free;
 

@@ -4280,7 +4280,7 @@ static void css_killed_ref_fn(struct percpu_ref *ref)
  */
 static void kill_css(struct cgroup_subsys_state *css)
 {
-	cgroup_clear_dir(css->cgroup, 1 << css->ss->subsys_id);
+	cgroup_clear_dir(css->cgroup, 1 << css->ss->id);
 
 	/*
 	 * Killing would put the base ref, but we need to keep it alive

@@ -4492,7 +4492,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 	 * pointer to this state - since the subsystem is
 	 * newly registered, all tasks and hence the
 	 * init_css_set is in the subsystem's top cgroup. */
-	init_css_set.subsys[ss->subsys_id] = css;
+	init_css_set.subsys[ss->id] = css;
 
 	need_forkexit_callback |= ss->fork || ss->exit;
 

@@ -4532,14 +4532,14 @@ int __init cgroup_init_early(void)
 	list_add(&init_cgrp_cset_link.cgrp_link, &init_css_set.cgrp_links);
 
 	for_each_subsys(ss, i) {
-		WARN(!ss->css_alloc || !ss->css_free || ss->name || ss->subsys_id,
+		WARN(!ss->css_alloc || !ss->css_free || ss->name || ss->id,
 		     "invalid cgroup_subsys %d:%s css_alloc=%p css_free=%p name:id=%d:%s\n",
 		     i, cgroup_subsys_name[i], ss->css_alloc, ss->css_free,
-		     ss->subsys_id, ss->name);
+		     ss->id, ss->name);
 		WARN(strlen(cgroup_subsys_name[i]) > MAX_CGROUP_TYPE_NAMELEN,
 		     "cgroup_subsys_name %s too long\n", cgroup_subsys_name[i]);
 
-		ss->subsys_id = i;
+		ss->id = i;
 		ss->name = cgroup_subsys_name[i];
 
 		if (ss->early_init)
Signature · 1 line
1.8.4.2
author maintainer
#23 lore raw

PATCH 5/6 cgroup: update locking in cgroup_show_options()

In reply to Tejun Heo

cgroup_show_options() grabs cgroup_root_mutex to protect the options changing while printing; however, holding root_mutex or not doesn't really make much difference for the function. subsys_mask can be atomically tested and most of the options aren't allowed to change anyway once mounted.

The only field which needs synchronization is ->release_agent_path. This patch introduces a dedicated spinlock to synchronize accesses to the field and drops cgroup_root_mutex locking from cgroup_show_options(). The next patch will remove cgroup_root_mutex.

Signed-off-by Tejun Heo <[email protected]>
---
 kernel/cgroup.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)
kernel/cgroup.c +16 -3
index 3b2fda0..2736689 100644
@@ -92,6 +92,12 @@ static DEFINE_MUTEX(cgroup_mutex);
 
 static DEFINE_MUTEX(cgroup_root_mutex);
 
+/*
+ * Protects cgroup_subsys->release_agent_path.  Modifying it also requires
+ * cgroup_mutex.  Reading requires either cgroup_mutex or this spinlock.
+ */
+static DEFINE_SPINLOCK(release_agent_path_lock);
+
 #define cgroup_assert_mutex_or_rcu_locked()				\
 	rcu_lockdep_assert(rcu_read_lock_held() ||			\
 			   lockdep_is_held(&cgroup_mutex),		\

@@ -1024,7 +1030,6 @@ static int cgroup_show_options(struct seq_file *seq, struct dentry *dentry)
 	struct cgroup_subsys *ss;
 	int ssid;
 
-	mutex_lock(&cgroup_root_mutex);
 	for_each_subsys(ss, ssid)
 		if (root->subsys_mask & (1 << ssid))
 			seq_printf(seq, ",%s", ss->name);

@@ -1034,13 +1039,16 @@ static int cgroup_show_options(struct seq_file *seq, struct dentry *dentry)
 		seq_puts(seq, ",noprefix");
 	if (root->flags & CGRP_ROOT_XATTR)
 		seq_puts(seq, ",xattr");
+
+	spin_lock(&release_agent_path_lock);
 	if (strlen(root->release_agent_path))
 		seq_printf(seq, ",release_agent=%s", root->release_agent_path);
+	spin_unlock(&release_agent_path_lock);
+
 	if (test_bit(CGRP_CPUSET_CLONE_CHILDREN, &root->top_cgroup.flags))
 		seq_puts(seq, ",clone_children");
 	if (strlen(root->name))
 		seq_printf(seq, ",name=%s", root->name);
-	mutex_unlock(&cgroup_root_mutex);
 	return 0;
 }
 

@@ -1262,8 +1270,11 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
 	if (ret)
 		goto out_unlock;
 
-	if (opts.release_agent)
+	if (opts.release_agent) {
+		spin_lock(&release_agent_path_lock);
 		strcpy(root->release_agent_path, opts.release_agent);
+		spin_unlock(&release_agent_path_lock);
+	}
  out_unlock:
 	kfree(opts.release_agent);
 	kfree(opts.name);

@@ -2172,7 +2183,9 @@ static int cgroup_release_agent_write(struct cgroup_subsys_state *css,
 	if (!cgroup_lock_live_group(css->cgroup))
 		return -ENODEV;
 	mutex_lock(&cgroup_root_mutex);
+	spin_lock(&release_agent_path_lock);
 	strcpy(css->cgroup->root->release_agent_path, buffer);
+	spin_unlock(&release_agent_path_lock);
 	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	return 0;
Signature · 1 line
1.8.4.2
author maintainer
#24 lore raw

PATCH 6/6 cgroup: remove cgroup_root_mutex

In reply to Tejun Heo

cgroup_root_mutex was added to avoid deadlock involving namespace_sem via cgroup_show_options(). It added a lot of overhead for the small purpose of it and, because it's nested under cgroup_mutex, it has very limited usefulness. The previous patch made cgroup_show_options() not use cgroup_root_mutex, so nobody needs it anymore. Remove it.

Signed-off-by Tejun Heo <[email protected]>
---
 kernel/cgroup.c | 42 +-----------------------------------------
 1 file changed, 1 insertion(+), 41 deletions(-)
kernel/cgroup.c +1 -41
index 2736689..f5dcac5 100644
@@ -70,18 +70,6 @@
 /*
  * cgroup_mutex is the master lock.  Any modification to cgroup or its
  * hierarchy must be performed while holding it.
- *
- * cgroup_root_mutex nests inside cgroup_mutex and should be held to modify
- * cgroupfs_root of any cgroup hierarchy - subsys list, flags,
- * release_agent_path and so on.  Modifying requires both cgroup_mutex and
- * cgroup_root_mutex.  Readers can acquire either of the two.  This is to
- * break the following locking order cycle.
- *
- *  A. cgroup_mutex -> cred_guard_mutex -> s_type->i_mutex_key -> namespace_sem
- *  B. namespace_sem -> cgroup_mutex
- *
- * B happens only through cgroup_show_options() and using cgroup_root_mutex
- * breaks it.
  */
 #ifdef CONFIG_PROVE_RCU
 DEFINE_MUTEX(cgroup_mutex);

@@ -90,8 +78,6 @@ EXPORT_SYMBOL_GPL(cgroup_mutex);	/* only for lockdep */
 static DEFINE_MUTEX(cgroup_mutex);
 #endif
 
-static DEFINE_MUTEX(cgroup_root_mutex);
-
 /*
  * Protects cgroup_subsys->release_agent_path.  Modifying it also requires
  * cgroup_mutex.  Reading requires either cgroup_mutex or this spinlock.

@@ -103,14 +89,6 @@ static DEFINE_SPINLOCK(release_agent_path_lock);
 			   lockdep_is_held(&cgroup_mutex),		\
 			   "cgroup_mutex or RCU read lock required");
 
-#ifdef CONFIG_LOCKDEP
-#define cgroup_assert_mutex_or_root_locked()				\
-	WARN_ON_ONCE(debug_locks && (!lockdep_is_held(&cgroup_mutex) &&	\
-				     !lockdep_is_held(&cgroup_root_mutex)))
-#else
-#define cgroup_assert_mutex_or_root_locked()	do { } while (0)
-#endif
-
 /*
  * cgroup destruction makes heavy use of work items and there can be a lot
  * of concurrent destructions.  Use a separate workqueue so that cgroup

@@ -154,11 +132,7 @@ static struct cgroup * const cgroup_dummy_top = &cgroup_dummy_root.top_cgroup;
 static LIST_HEAD(cgroup_roots);
 static int cgroup_root_count;
 
-/*
- * Hierarchy ID allocation and mapping.  It follows the same exclusion
- * rules as other root ops - both cgroup_mutex and cgroup_root_mutex for
- * writes, either for reads.
- */
+/* hierarchy ID allocation and mapping, protected by cgroup_mutex */
 static DEFINE_IDR(cgroup_hierarchy_idr);
 
 static struct cgroup_name root_cgroup_name = { .name = "/" };

@@ -963,7 +937,6 @@ static int rebind_subsystems(struct cgroupfs_root *root,
 	int i, ret;
 
 	BUG_ON(!mutex_is_locked(&cgroup_mutex));
-	BUG_ON(!mutex_is_locked(&cgroup_root_mutex));
 
 	/* Check that any added subsystems are currently free */
 	for_each_subsys(ss, i)

@@ -1236,7 +1209,6 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
 
 	mutex_lock(&cgrp->dentry->d_inode->i_mutex);
 	mutex_lock(&cgroup_mutex);
-	mutex_lock(&cgroup_root_mutex);
 
 	/* See what subsystems are wanted */
 	ret = parse_cgroupfs_options(data, &opts);

@@ -1278,7 +1250,6 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
  out_unlock:
 	kfree(opts.release_agent);
 	kfree(opts.name);
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
 	return ret;

@@ -1321,7 +1292,6 @@ static int cgroup_init_root_id(struct cgroupfs_root *root, int start, int end)
 	int id;
 
 	lockdep_assert_held(&cgroup_mutex);
-	lockdep_assert_held(&cgroup_root_mutex);
 
 	id = idr_alloc_cyclic(&cgroup_hierarchy_idr, root, start, end,
 			      GFP_KERNEL);

@@ -1335,7 +1305,6 @@ static int cgroup_init_root_id(struct cgroupfs_root *root, int start, int end)
 static void cgroup_exit_root_id(struct cgroupfs_root *root)
 {
 	lockdep_assert_held(&cgroup_mutex);
-	lockdep_assert_held(&cgroup_root_mutex);
 
 	if (root->hierarchy_id) {
 		idr_remove(&cgroup_hierarchy_idr, root->hierarchy_id);

@@ -1514,7 +1483,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 
 		mutex_lock(&inode->i_mutex);
 		mutex_lock(&cgroup_mutex);
-		mutex_lock(&cgroup_root_mutex);
 
 		root_cgrp->id = idr_alloc(&root->cgroup_idr, root_cgrp,
 					   0, 1, GFP_KERNEL);

@@ -1587,7 +1555,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		BUG_ON(!list_empty(&root_cgrp->children));
 		BUG_ON(root->number_of_cgroups != 1);
 
-		mutex_unlock(&cgroup_root_mutex);
 		mutex_unlock(&cgroup_mutex);
 		mutex_unlock(&inode->i_mutex);
 	} else {

@@ -1618,7 +1585,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 	revert_creds(cred);
  unlock_drop:
 	cgroup_exit_root_id(root);
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&inode->i_mutex);
  drop_new_super:

@@ -1642,7 +1608,6 @@ static void cgroup_kill_sb(struct super_block *sb) {
 
 	mutex_lock(&cgrp->dentry->d_inode->i_mutex);
 	mutex_lock(&cgroup_mutex);
-	mutex_lock(&cgroup_root_mutex);
 
 	/* Rebind all subsystems back to the default hierarchy */
 	if (root->flags & CGRP_ROOT_SUBSYS_BOUND) {

@@ -1671,7 +1636,6 @@ static void cgroup_kill_sb(struct super_block *sb) {
 
 	cgroup_exit_root_id(root);
 
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
 

@@ -2182,11 +2146,9 @@ static int cgroup_release_agent_write(struct cgroup_subsys_state *css,
 		return -EINVAL;
 	if (!cgroup_lock_live_group(css->cgroup))
 		return -ENODEV;
-	mutex_lock(&cgroup_root_mutex);
 	spin_lock(&release_agent_path_lock);
 	strcpy(css->cgroup->root->release_agent_path, buffer);
 	spin_unlock(&release_agent_path_lock);
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	return 0;
 }

@@ -4584,7 +4546,6 @@ int __init cgroup_init(void)
 
 	/* allocate id for the dummy hierarchy */
 	mutex_lock(&cgroup_mutex);
-	mutex_lock(&cgroup_root_mutex);
 
 	/* Add init_css_set to the hash table */
 	key = css_set_hash(init_css_set.subsys);

@@ -4596,7 +4557,6 @@ int __init cgroup_init(void)
 			0, 1, GFP_KERNEL);
 	BUG_ON(err < 0);
 
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 
 	cgroup_kobj = kobject_create_and_add("cgroup", fs_kobj);
Signature · 1 line
1.8.4.2

A message that is not in the archive (sent off-list or not yet received): 1389982317-22225-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#26 lore raw

PATCH 6/6 cgroup: remove cgroup_root_mutex

cgroup_root_mutex was added to avoid deadlock involving namespace_sem via cgroup_show_options(). It added a lot of overhead for the small purpose of it and, because it's nested under cgroup_mutex, it has very limited usefulness. The previous patch made cgroup_show_options() not use cgroup_root_mutex, so nobody needs it anymore. Remove it.

Signed-off-by Tejun Heo <[email protected]>
---
 kernel/cgroup.c | 42 +-----------------------------------------
 1 file changed, 1 insertion(+), 41 deletions(-)
kernel/cgroup.c +1 -41
index 2736689..f5dcac5 100644
@@ -70,18 +70,6 @@
 /*
  * cgroup_mutex is the master lock.  Any modification to cgroup or its
  * hierarchy must be performed while holding it.
- *
- * cgroup_root_mutex nests inside cgroup_mutex and should be held to modify
- * cgroupfs_root of any cgroup hierarchy - subsys list, flags,
- * release_agent_path and so on.  Modifying requires both cgroup_mutex and
- * cgroup_root_mutex.  Readers can acquire either of the two.  This is to
- * break the following locking order cycle.
- *
- *  A. cgroup_mutex -> cred_guard_mutex -> s_type->i_mutex_key -> namespace_sem
- *  B. namespace_sem -> cgroup_mutex
- *
- * B happens only through cgroup_show_options() and using cgroup_root_mutex
- * breaks it.
  */
 #ifdef CONFIG_PROVE_RCU
 DEFINE_MUTEX(cgroup_mutex);

@@ -90,8 +78,6 @@ EXPORT_SYMBOL_GPL(cgroup_mutex);	/* only for lockdep */
 static DEFINE_MUTEX(cgroup_mutex);
 #endif
 
-static DEFINE_MUTEX(cgroup_root_mutex);
-
 /*
  * Protects cgroup_subsys->release_agent_path.  Modifying it also requires
  * cgroup_mutex.  Reading requires either cgroup_mutex or this spinlock.

@@ -103,14 +89,6 @@ static DEFINE_SPINLOCK(release_agent_path_lock);
 			   lockdep_is_held(&cgroup_mutex),		\
 			   "cgroup_mutex or RCU read lock required");
 
-#ifdef CONFIG_LOCKDEP
-#define cgroup_assert_mutex_or_root_locked()				\
-	WARN_ON_ONCE(debug_locks && (!lockdep_is_held(&cgroup_mutex) &&	\
-				     !lockdep_is_held(&cgroup_root_mutex)))
-#else
-#define cgroup_assert_mutex_or_root_locked()	do { } while (0)
-#endif
-
 /*
  * cgroup destruction makes heavy use of work items and there can be a lot
  * of concurrent destructions.  Use a separate workqueue so that cgroup

@@ -154,11 +132,7 @@ static struct cgroup * const cgroup_dummy_top = &cgroup_dummy_root.top_cgroup;
 static LIST_HEAD(cgroup_roots);
 static int cgroup_root_count;
 
-/*
- * Hierarchy ID allocation and mapping.  It follows the same exclusion
- * rules as other root ops - both cgroup_mutex and cgroup_root_mutex for
- * writes, either for reads.
- */
+/* hierarchy ID allocation and mapping, protected by cgroup_mutex */
 static DEFINE_IDR(cgroup_hierarchy_idr);
 
 static struct cgroup_name root_cgroup_name = { .name = "/" };

@@ -963,7 +937,6 @@ static int rebind_subsystems(struct cgroupfs_root *root,
 	int i, ret;
 
 	BUG_ON(!mutex_is_locked(&cgroup_mutex));
-	BUG_ON(!mutex_is_locked(&cgroup_root_mutex));
 
 	/* Check that any added subsystems are currently free */
 	for_each_subsys(ss, i)

@@ -1236,7 +1209,6 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
 
 	mutex_lock(&cgrp->dentry->d_inode->i_mutex);
 	mutex_lock(&cgroup_mutex);
-	mutex_lock(&cgroup_root_mutex);
 
 	/* See what subsystems are wanted */
 	ret = parse_cgroupfs_options(data, &opts);

@@ -1278,7 +1250,6 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
  out_unlock:
 	kfree(opts.release_agent);
 	kfree(opts.name);
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
 	return ret;

@@ -1321,7 +1292,6 @@ static int cgroup_init_root_id(struct cgroupfs_root *root, int start, int end)
 	int id;
 
 	lockdep_assert_held(&cgroup_mutex);
-	lockdep_assert_held(&cgroup_root_mutex);
 
 	id = idr_alloc_cyclic(&cgroup_hierarchy_idr, root, start, end,
 			      GFP_KERNEL);

@@ -1335,7 +1305,6 @@ static int cgroup_init_root_id(struct cgroupfs_root *root, int start, int end)
 static void cgroup_exit_root_id(struct cgroupfs_root *root)
 {
 	lockdep_assert_held(&cgroup_mutex);
-	lockdep_assert_held(&cgroup_root_mutex);
 
 	if (root->hierarchy_id) {
 		idr_remove(&cgroup_hierarchy_idr, root->hierarchy_id);

@@ -1514,7 +1483,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 
 		mutex_lock(&inode->i_mutex);
 		mutex_lock(&cgroup_mutex);
-		mutex_lock(&cgroup_root_mutex);
 
 		root_cgrp->id = idr_alloc(&root->cgroup_idr, root_cgrp,
 					   0, 1, GFP_KERNEL);

@@ -1587,7 +1555,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		BUG_ON(!list_empty(&root_cgrp->children));
 		BUG_ON(root->number_of_cgroups != 1);
 
-		mutex_unlock(&cgroup_root_mutex);
 		mutex_unlock(&cgroup_mutex);
 		mutex_unlock(&inode->i_mutex);
 	} else {

@@ -1618,7 +1585,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 	revert_creds(cred);
  unlock_drop:
 	cgroup_exit_root_id(root);
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&inode->i_mutex);
  drop_new_super:

@@ -1642,7 +1608,6 @@ static void cgroup_kill_sb(struct super_block *sb) {
 
 	mutex_lock(&cgrp->dentry->d_inode->i_mutex);
 	mutex_lock(&cgroup_mutex);
-	mutex_lock(&cgroup_root_mutex);
 
 	/* Rebind all subsystems back to the default hierarchy */
 	if (root->flags & CGRP_ROOT_SUBSYS_BOUND) {

@@ -1671,7 +1636,6 @@ static void cgroup_kill_sb(struct super_block *sb) {
 
 	cgroup_exit_root_id(root);
 
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
 

@@ -2182,11 +2146,9 @@ static int cgroup_release_agent_write(struct cgroup_subsys_state *css,
 		return -EINVAL;
 	if (!cgroup_lock_live_group(css->cgroup))
 		return -ENODEV;
-	mutex_lock(&cgroup_root_mutex);
 	spin_lock(&release_agent_path_lock);
 	strcpy(css->cgroup->root->release_agent_path, buffer);
 	spin_unlock(&release_agent_path_lock);
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 	return 0;
 }

@@ -4584,7 +4546,6 @@ int __init cgroup_init(void)
 
 	/* allocate id for the dummy hierarchy */
 	mutex_lock(&cgroup_mutex);
-	mutex_lock(&cgroup_root_mutex);
 
 	/* Add init_css_set to the hash table */
 	key = css_set_hash(init_css_set.subsys);

@@ -4596,7 +4557,6 @@ int __init cgroup_init(void)
 			0, 1, GFP_KERNEL);
 	BUG_ON(err < 0);
 
-	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
 
 	cgroup_kobj = kobject_create_and_add("cgroup", fs_kobj);
Signature · 1 line
1.8.4.2