[PATCH v0 0/5] cgroup: Assign subsystem IDs during compile time

[PATCH v0 0/5] cgroup: Assign subsystem IDs during compile time

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Hi,

The patch #1 and #2 are there to be able to introduce (#3, #4) the 
jump labels in task_cls_classid() and task_netprioidx(). The jump
labels are needed to know when it is safe to access the controller. 
For example not safe means the module is not yet loaded.

All those patches are just preparation for the center piece (#5) 
of these series. This one will remove the dynamic subsystem ID
generation and falls back to compile time generated IDs. 

This is the first result from the discussion around on the
"cgroup cls & netprio 'cleanups'" patches.

This patches are against net-next

cheers,
daniel

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: netdev@vger.kernel.org
Cc: cgroups@vger.kernel.org

Daniel Wagner (5):
  cgroup: Use empty task_cls_classid() when !CONFIG_NET_CLS(_MODULE)
  cgroup: Move sock_update_classid() decleration to cls_cgroup.h
  cgroup: Protect access to task_cls_classid() when built as module
  cgroup: Protect access to task_netprioidx() when built as module
  cgroup: Assign subsystem IDs during compile time

 include/linux/cgroup.h        | 20 +++++++++++++-------
 include/linux/cgroup_subsys.h | 24 ++++++++++++------------
 include/net/cls_cgroup.h      | 42 +++++++++++++++++++++++++++++-------------
 include/net/netprio_cgroup.h  | 23 +++++++++++------------
 include/net/sock.h            |  8 --------
 kernel/cgroup.c               | 31 +++++++++----------------------
 net/core/netprio_cgroup.c     | 17 ++++++-----------
 net/core/sock.c               | 12 ++++++------
 net/sched/cls_cgroup.c        | 18 +++++-------------
 9 files changed, 91 insertions(+), 104 deletions(-)

-- 
1.7.12.rc1.16.g05a20c8

[PATCH v0 1/5] cgroup: Use empty task_cls_classid() when !CONFIG_NET_CLS(_MODULE)

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>

Signed-off-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Cc: Jamal Hadi Salim <jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org>
Cc: John Fastabend <john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 include/net/cls_cgroup.h | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index a4dc5b0..e2fe2b9 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -24,7 +24,8 @@ struct cgroup_cls_state
 	u32 classid;
 };
 
-#ifdef CONFIG_NET_CLS_CGROUP
+#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
+
 static inline u32 task_cls_classid(struct task_struct *p)
 {
 	int classid;
@@ -39,7 +40,9 @@ static inline u32 task_cls_classid(struct task_struct *p)
 
 	return classid;
 }
-#else
+
+#elif IS_MODULE(CONFIG_NET_CLS_CGROUP)
+
 extern int net_cls_subsys_id;
 
 static inline u32 task_cls_classid(struct task_struct *p)
@@ -60,11 +63,16 @@ static inline u32 task_cls_classid(struct task_struct *p)
 
 	return classid;
 }
-#endif
+
 #else
+
 static inline u32 task_cls_classid(struct task_struct *p)
 {
 	return 0;
 }
-#endif
+
+#endif /* CONFIG_NET_CLS_CGROUP */
+
+#endif /* CONFIG_CGROURPS */
+
 #endif  /* _NET_CLS_CGROUP_H */
-- 
1.7.12.rc1.16.g05a20c8

[PATCH v0 2/5] cgroup: Move sock_update_classid() decleration to cls_cgroup.h

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

The only user of sock_update_classid() is net/socket.c which
happens to include cls_cgroup.h direclty.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: netdev@vger.kernel.org
Cc: cgroups@vger.kernel.org
---
 include/net/cls_cgroup.h | 8 ++++++++
 include/net/sock.h       | 8 --------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index e2fe2b9..401672c 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -24,6 +24,8 @@ struct cgroup_cls_state
 	u32 classid;
 };
 
+extern void sock_update_classid(struct sock *sk);
+
 #if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
 
 static inline u32 task_cls_classid(struct task_struct *p)
@@ -73,6 +75,12 @@ static inline u32 task_cls_classid(struct task_struct *p)
 
 #endif /* CONFIG_NET_CLS_CGROUP */
 
+#else /* !CONFIG_CGROUPS */
+
+static inline void sock_update_classid(struct sock *sk)
+{
+}
+
 #endif /* CONFIG_CGROURPS */
 
 #endif  /* _NET_CLS_CGROUP_H */
diff --git a/include/net/sock.h b/include/net/sock.h
index 72132ae..160a680 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1486,14 +1486,6 @@ extern void *sock_kmalloc(struct sock *sk, int size,
 extern void sock_kfree_s(struct sock *sk, void *mem, int size);
 extern void sk_send_sigurg(struct sock *sk);
 
-#ifdef CONFIG_CGROUPS
-extern void sock_update_classid(struct sock *sk);
-#else
-static inline void sock_update_classid(struct sock *sk)
-{
-}
-#endif
-
 /*
  * Functions to fill in entries in struct proto_ops when a protocol
  * does not implement a particular function.
-- 
1.7.12.rc1.16.g05a20c8

[PATCH v0 3/5] cgroup: Protect access to task_cls_classid() when built as module

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

The module version of task_cls_classid() checks if net_cls_sbusys_id
is valid to indentify when it is okay to access the controller.

Instead relying on the subusys_id to be set, make it explicit
with a jump label.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: netdev@vger.kernel.org
Cc: cgroups@vger.kernel.org
---
 include/net/cls_cgroup.h | 8 +++++++-
 net/core/sock.c          | 5 +++++
 net/sched/cls_cgroup.c   | 5 +++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index 401672c..5b91220 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -16,6 +16,7 @@
 #include <linux/cgroup.h>
 #include <linux/hardirq.h>
 #include <linux/rcupdate.h>
+#include <linux/jump_label.h>
 
 #ifdef CONFIG_CGROUPS
 struct cgroup_cls_state
@@ -24,6 +25,11 @@ struct cgroup_cls_state
 	u32 classid;
 };
 
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
+extern struct static_key cgroup_cls_enabled;
+#define clscg_enabled static_key_false(&cgroup_cls_enabled)
+#endif
+
 extern void sock_update_classid(struct sock *sk);
 
 #if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
@@ -52,7 +58,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
 	int id;
 	u32 classid = 0;
 
-	if (in_interrupt())
+	if (!clscg_enabled || in_interrupt())
 		return 0;
 
 	rcu_read_lock();
diff --git a/net/core/sock.c b/net/core/sock.c
index 8f67ced..8d3a400 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -327,6 +327,11 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
 EXPORT_SYMBOL(__sk_backlog_rcv);
 
 #if defined(CONFIG_CGROUPS)
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
+struct static_key cgroup_cls_enabled = STATIC_KEY_INIT_FALSE;
+EXPORT_SYMBOL_GPL(cgroup_cls_enabled);
+#endif
+
 #if !defined(CONFIG_NET_CLS_CGROUP)
 int net_cls_subsys_id = -1;
 EXPORT_SYMBOL_GPL(net_cls_subsys_id);
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 7743ea8..f40086b 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -44,12 +44,17 @@ static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
 
 	if (cgrp->parent)
 		cs->classid = cgrp_cls_state(cgrp->parent)->classid;
+	else if (!clscg_enabled)
+                static_key_slow_inc(&cgroup_cls_enabled);
 
 	return &cs->css;
 }
 
 static void cgrp_destroy(struct cgroup *cgrp)
 {
+	if (!cgrp->parent && clscg_enabled)
+                static_key_slow_dec(&cgroup_cls_enabled);
+
 	kfree(cgrp_cls_state(cgrp));
 }
 
-- 
1.7.12.rc1.16.g05a20c8

[PATCH v0 4/5] cgroup: Protect access to task_netprioidx() when built as module

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>

The module version of task_netprioidex() checks if net_prio_subsys_id
is valid to indentify when it is okay to access the controller.

Instead relying on the net_prio_subsys_id to be set, make it explicit
with a jump label.

Signed-off-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Cc: Jamal Hadi Salim <jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org>
Cc: John Fastabend <john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 include/net/netprio_cgroup.h | 10 +++++++++-
 net/core/netprio_cgroup.c    |  6 ++++++
 net/core/sock.c              |  4 ++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index 2719dec..a0fd35a 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h
@@ -16,7 +16,7 @@
 #include <linux/cgroup.h>
 #include <linux/hardirq.h>
 #include <linux/rcupdate.h>
-
+#include <linux/jump_label.h>
 
 struct netprio_map {
 	struct rcu_head rcu;
@@ -35,6 +35,11 @@ struct cgroup_netprio_state {
 extern int net_prio_subsys_id;
 #endif
 
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
+extern struct static_key cgroup_netprio_enabled;
+#define netpriocg_enabled static_key_false(&cgroup_netprio_enabled)
+#endif
+
 extern void sock_update_netprioidx(struct sock *sk, struct task_struct *task);
 
 #if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
@@ -60,6 +65,9 @@ static inline u32 task_netprioidx(struct task_struct *p)
 	int subsys_id;
 	u32 idx = 0;
 
+	if (!netpriocg_enabled)
+		return 0;
+
 	rcu_read_lock();
 	subsys_id = rcu_dereference_index_check(net_prio_subsys_id,
 						rcu_read_lock_held());
diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index ed0c043..6fef72f 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -155,6 +155,9 @@ static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
 		goto out;
 	}
 
+	if (!netpriocg_enabled && !cgrp->parent)
+                static_key_slow_inc(&cgroup_netprio_enabled);
+
 	ret = update_netdev_tables();
 	if (ret < 0) {
 		put_prioidx(cs->prioidx);
@@ -173,6 +176,9 @@ static void cgrp_destroy(struct cgroup *cgrp)
 	struct net_device *dev;
 	struct netprio_map *map;
 
+        if (netpriocg_enabled && !cgrp->parent)
+                static_key_slow_dec(&cgroup_netprio_enabled);
+
 	cs = cgrp_netprio_state(cgrp);
 	rtnl_lock();
 	for_each_netdev(&init_net, dev) {
diff --git a/net/core/sock.c b/net/core/sock.c
index 8d3a400..d00da68 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -331,6 +331,10 @@ EXPORT_SYMBOL(__sk_backlog_rcv);
 struct static_key cgroup_cls_enabled = STATIC_KEY_INIT_FALSE;
 EXPORT_SYMBOL_GPL(cgroup_cls_enabled);
 #endif
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
+struct static_key cgroup_netprio_enabled = STATIC_KEY_INIT_FALSE;
+EXPORT_SYMBOL_GPL(cgroup_netprio_enabled);
+#endif
 
 #if !defined(CONFIG_NET_CLS_CGROUP)
 int net_cls_subsys_id = -1;
-- 
1.7.12.rc1.16.g05a20c8

[PATCH v0 5/5] cgroup: Assign subsystem IDs during compile time

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>

We are able to safe some space when we assign the subsystem
IDs at compile time. Instead of allocating per cgroup
cgroup->subsys[CGROUP_SUBSYS_COUNT] where CGROUP_SUBSYS_COUNT is
always 64, we allocate 12 + 1 at max (at this point there are 12
subsystem).

The enum is created by passing in cgroup_subsys.h twice and
redefine the IS_SUBSYS_ENABLED. In the first pass, we just select
the builtin subsystem and in the second pass only the module
subsystems. The enums regions are separated by
CGROUP_BUILTIN_SUBSYS_COUNT which is the reason why we loose one
enum.

That also means we need to update all iterators over the module
subsystem ids to start one later (or stop one earlier).

task_cls_classid() and task_netprioidx() (when built as
module) are protected by a jump label and therefore we can
simply replace the subsystem index lookup with the enum.

Signed-off-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Cc: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Cc: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Cc: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Cc: Jamal Hadi Salim <jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org>
Cc: John Fastabend <john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Kamezawa Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 include/linux/cgroup.h        | 20 +++++++++++++-------
 include/linux/cgroup_subsys.h | 24 ++++++++++++------------
 include/net/cls_cgroup.h      | 12 +++---------
 include/net/netprio_cgroup.h  | 17 ++++-------------
 kernel/cgroup.c               | 31 +++++++++----------------------
 net/core/netprio_cgroup.c     | 11 -----------
 net/core/sock.c               |  9 ---------
 net/sched/cls_cgroup.c        | 13 -------------
 8 files changed, 41 insertions(+), 96 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index c90eaa8..995739f 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -46,16 +46,20 @@ extern const struct file_operations proc_cgroup_operations;
 /* Define the enumeration of all builtin 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,
+
+#define IS_SUBSYS_ENABLED(option) IS_MODULE(option)
 #include <linux/cgroup_subsys.h>
-	CGROUP_BUILTIN_SUBSYS_COUNT
+#undef IS_SUBSYS_ENABLED
+
+	CGROUP_SUBSYS_COUNT
 };
 #undef SUBSYS
-/*
- * This define indicates the maximum number of subsystems that can be loaded
- * at once. We limit to this many since cgroupfs_root has subsys_bits to keep
- * track of all of them.
- */
-#define CGROUP_SUBSYS_COUNT (BITS_PER_BYTE*sizeof(unsigned long))
 
 /* Per-subsystem/per-cgroup state maintained by the system. */
 struct cgroup_subsys_state {
@@ -521,7 +525,9 @@ struct cgroup_subsys {
 };
 
 #define SUBSYS(_x) extern struct cgroup_subsys _x ## _subsys;
+#define IS_SUBSYS_ENABLED(option) IS_ENABLED(option)
 #include <linux/cgroup_subsys.h>
+#undef IS_SUBSYS_ENABLED
 #undef SUBSYS
 
 static inline struct cgroup_subsys_state *cgroup_subsys_state(
diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
index dfae957..f204a7a 100644
--- a/include/linux/cgroup_subsys.h
+++ b/include/linux/cgroup_subsys.h
@@ -7,73 +7,73 @@
 
 /* */
 
-#ifdef CONFIG_CPUSETS
+#if IS_SUBSYS_ENABLED(CONFIG_CPUSETS)
 SUBSYS(cpuset)
 #endif
 
 /* */
 
-#ifdef CONFIG_CGROUP_DEBUG
+#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_DEBUG)
 SUBSYS(debug)
 #endif
 
 /* */
 
-#ifdef CONFIG_CGROUP_SCHED
+#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_SCHED)
 SUBSYS(cpu_cgroup)
 #endif
 
 /* */
 
-#ifdef CONFIG_CGROUP_CPUACCT
+#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_CPUACCT)
 SUBSYS(cpuacct)
 #endif
 
 /* */
 
-#ifdef CONFIG_MEMCG
+#if IS_SUBSYS_ENABLED(CONFIG_MEMCG)
 SUBSYS(mem_cgroup)
 #endif
 
 /* */
 
-#ifdef CONFIG_CGROUP_DEVICE
+#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_DEVICE)
 SUBSYS(devices)
 #endif
 
 /* */
 
-#ifdef CONFIG_CGROUP_FREEZER
+#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_FREEZER)
 SUBSYS(freezer)
 #endif
 
 /* */
 
-#ifdef CONFIG_NET_CLS_CGROUP
+#if IS_SUBSYS_ENABLED(CONFIG_NET_CLS_CGROUP)
 SUBSYS(net_cls)
 #endif
 
 /* */
 
-#ifdef CONFIG_BLK_CGROUP
+#if IS_SUBSYS_ENABLED(CONFIG_BLK_CGROUP)
 SUBSYS(blkio)
 #endif
 
 /* */
 
-#ifdef CONFIG_CGROUP_PERF
+#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_PERF)
 SUBSYS(perf)
 #endif
 
 /* */
 
-#ifdef CONFIG_NETPRIO_CGROUP
+#if IS_SUBSYS_ENABLED(CONFIG_NETPRIO_CGROUP)
 SUBSYS(net_prio)
 #endif
 
 /* */
 
-#ifdef CONFIG_CGROUP_HUGETLB
+#if IS_SUBSYS_ENABLED(CONFIG_CGROUP_HUGETLB)
 SUBSYS(hugetlb)
 #endif
 
diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index 5b91220..c82af60 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -51,22 +51,16 @@ static inline u32 task_cls_classid(struct task_struct *p)
 
 #elif IS_MODULE(CONFIG_NET_CLS_CGROUP)
 
-extern int net_cls_subsys_id;
-
 static inline u32 task_cls_classid(struct task_struct *p)
 {
-	int id;
-	u32 classid = 0;
+	u32 classid;
 
 	if (!clscg_enabled || in_interrupt())
 		return 0;
 
 	rcu_read_lock();
-	id = rcu_dereference_index_check(net_cls_subsys_id,
-					 rcu_read_lock_held());
-	if (id >= 0)
-		classid = container_of(task_subsys_state(p, id),
-				       struct cgroup_cls_state, css)->classid;
+	classid = container_of(task_subsys_state(p, net_cls_subsys_id),
+			       struct cgroup_cls_state, css)->classid;
 	rcu_read_unlock();
 
 	return classid;
diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index a0fd35a..5b790dc 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h
@@ -31,10 +31,6 @@ struct cgroup_netprio_state {
 	u32 prioidx;
 };
 
-#ifndef CONFIG_NETPRIO_CGROUP
-extern int net_prio_subsys_id;
-#endif
-
 #if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 extern struct static_key cgroup_netprio_enabled;
 #define netpriocg_enabled static_key_false(&cgroup_netprio_enabled)
@@ -62,20 +58,15 @@ static inline u32 task_netprioidx(struct task_struct *p)
 static inline u32 task_netprioidx(struct task_struct *p)
 {
 	struct cgroup_netprio_state *state;
-	int subsys_id;
-	u32 idx = 0;
+	u32 idx;
 
 	if (!netpriocg_enabled)
 		return 0;
 
 	rcu_read_lock();
-	subsys_id = rcu_dereference_index_check(net_prio_subsys_id,
-						rcu_read_lock_held());
-	if (subsys_id >= 0) {
-		state = container_of(task_subsys_state(p, subsys_id),
-				     struct cgroup_netprio_state, css);
-		idx = state->prioidx;
-	}
+	state = container_of(task_subsys_state(p, net_prio_subsys_id),
+			     struct cgroup_netprio_state, css);
+	idx = state->prioidx;
 	rcu_read_unlock();
 	return idx;
 }
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 7981850..aa629ce 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -93,9 +93,12 @@ static DEFINE_MUTEX(cgroup_root_mutex);
  * cgroup_mutex.
  */
 #define SUBSYS(_x) &_x ## _subsys,
+#define IS_SUBSYS_ENABLED(option) IS_BUILTIN(option)
 static struct cgroup_subsys *subsys[CGROUP_SUBSYS_COUNT] = {
 #include <linux/cgroup_subsys.h>
 };
+#undef IS_SUBSYS_ENABLED
+#undef SUBSYS
 
 #define MAX_CGROUP_ROOT_NAMELEN 64
 
@@ -1307,7 +1310,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 		 * raced with a module_delete call, and to the user this is
 		 * essentially a "subsystem doesn't exist" case.
 		 */
-		for (i--; i >= CGROUP_BUILTIN_SUBSYS_COUNT; i--) {
+		for (i--; i > CGROUP_BUILTIN_SUBSYS_COUNT; i--) {
 			/* drop refcounts only on the ones we took */
 			unsigned long bit = 1UL << i;
 
@@ -1324,7 +1327,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 static void drop_parsed_module_refcounts(unsigned long subsys_bits)
 {
 	int i;
-	for (i = CGROUP_BUILTIN_SUBSYS_COUNT; i < CGROUP_SUBSYS_COUNT; i++) {
+	for (i = CGROUP_BUILTIN_SUBSYS_COUNT + 1; i < CGROUP_SUBSYS_COUNT; i++) {
 		unsigned long bit = 1UL << i;
 
 		if (!(bit & subsys_bits))
@@ -4322,7 +4325,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	 */
 	if (ss->module == NULL) {
 		/* a few sanity checks */
-		BUG_ON(ss->subsys_id >= CGROUP_BUILTIN_SUBSYS_COUNT);
+		BUG_ON(ss->subsys_id > CGROUP_BUILTIN_SUBSYS_COUNT);
 		BUG_ON(subsys[ss->subsys_id] != ss);
 		return 0;
 	}
@@ -4330,24 +4333,8 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	/* init base cftset */
 	cgroup_init_cftsets(ss);
 
-	/*
-	 * need to register a subsys id before anything else - for example,
-	 * init_cgroup_css needs it.
-	 */
 	mutex_lock(&cgroup_mutex);
-	/* find the first empty slot in the array */
-	for (i = CGROUP_BUILTIN_SUBSYS_COUNT; i < CGROUP_SUBSYS_COUNT; i++) {
-		if (subsys[i] == NULL)
-			break;
-	}
-	if (i == CGROUP_SUBSYS_COUNT) {
-		/* maximum number of subsystems already registered! */
-		mutex_unlock(&cgroup_mutex);
-		return -EBUSY;
-	}
-	/* assign ourselves the subsys_id */
-	ss->subsys_id = i;
-	subsys[i] = ss;
+	subsys[ss->subsys_id] = ss;
 
 	/*
 	 * no ss->create seems to need anything important in the ss struct, so
@@ -4356,7 +4343,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	css = ss->create(dummytop);
 	if (IS_ERR(css)) {
 		/* failure case - need to deassign the subsys[] slot. */
-		subsys[i] = NULL;
+		subsys[ss->subsys_id] = NULL;
 		mutex_unlock(&cgroup_mutex);
 		return PTR_ERR(css);
 	}
@@ -4372,7 +4359,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 		if (ret) {
 			dummytop->subsys[ss->subsys_id] = NULL;
 			ss->destroy(dummytop);
-			subsys[i] = NULL;
+			subsys[ss->subsys_id] = NULL;
 			mutex_unlock(&cgroup_mutex);
 			return ret;
 		}
diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index 6fef72f..bf82a11 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -348,9 +348,7 @@ struct cgroup_subsys net_prio_subsys = {
 	.create		= cgrp_create,
 	.destroy	= cgrp_destroy,
 	.attach		= net_prio_attach,
-#ifdef CONFIG_NETPRIO_CGROUP
 	.subsys_id	= net_prio_subsys_id,
-#endif
 	.base_cftypes	= ss_files,
 	.module		= THIS_MODULE
 };
@@ -388,10 +386,6 @@ static int __init init_cgroup_netprio(void)
 	ret = cgroup_load_subsys(&net_prio_subsys);
 	if (ret)
 		goto out;
-#ifndef CONFIG_NETPRIO_CGROUP
-	smp_wmb();
-	net_prio_subsys_id = net_prio_subsys.subsys_id;
-#endif
 
 	register_netdevice_notifier(&netprio_device_notifier);
 
@@ -408,11 +402,6 @@ static void __exit exit_cgroup_netprio(void)
 
 	cgroup_unload_subsys(&net_prio_subsys);
 
-#ifndef CONFIG_NETPRIO_CGROUP
-	net_prio_subsys_id = -1;
-	synchronize_rcu();
-#endif
-
 	rtnl_lock();
 	for_each_netdev(&init_net, dev) {
 		old = rtnl_dereference(dev->priomap);
diff --git a/net/core/sock.c b/net/core/sock.c
index d00da68..c2ce9bc 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -335,15 +335,6 @@ EXPORT_SYMBOL_GPL(cgroup_cls_enabled);
 struct static_key cgroup_netprio_enabled = STATIC_KEY_INIT_FALSE;
 EXPORT_SYMBOL_GPL(cgroup_netprio_enabled);
 #endif
-
-#if !defined(CONFIG_NET_CLS_CGROUP)
-int net_cls_subsys_id = -1;
-EXPORT_SYMBOL_GPL(net_cls_subsys_id);
-#endif
-#if !defined(CONFIG_NETPRIO_CGROUP)
-int net_prio_subsys_id = -1;
-EXPORT_SYMBOL_GPL(net_prio_subsys_id);
-#endif
 #endif
 
 static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index f40086b..5d4b46d 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -82,9 +82,7 @@ struct cgroup_subsys net_cls_subsys = {
 	.name		= "net_cls",
 	.create		= cgrp_create,
 	.destroy	= cgrp_destroy,
-#ifdef CONFIG_NET_CLS_CGROUP
 	.subsys_id	= net_cls_subsys_id,
-#endif
 	.base_cftypes	= ss_files,
 	.module		= THIS_MODULE,
 };
@@ -288,12 +286,6 @@ static int __init init_cgroup_cls(void)
 	if (ret)
 		goto out;
 
-#ifndef CONFIG_NET_CLS_CGROUP
-	/* We can't use rcu_assign_pointer because this is an int. */
-	smp_wmb();
-	net_cls_subsys_id = net_cls_subsys.subsys_id;
-#endif
-
 	ret = register_tcf_proto_ops(&cls_cgroup_ops);
 	if (ret)
 		cgroup_unload_subsys(&net_cls_subsys);
@@ -306,11 +298,6 @@ static void __exit exit_cgroup_cls(void)
 {
 	unregister_tcf_proto_ops(&cls_cgroup_ops);
 
-#ifndef CONFIG_NET_CLS_CGROUP
-	net_cls_subsys_id = -1;
-	synchronize_rcu();
-#endif
-
 	cgroup_unload_subsys(&net_cls_subsys);
 }
 
-- 
1.7.12.rc1.16.g05a20c8

Re: [PATCH v0 0/5] cgroup: Assign subsystem IDs during compile time

[Neil Horman]

On Thu, Aug 16, 2012 at 04:12:11PM +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
> 
> Hi,
> 
> The patch #1 and #2 are there to be able to introduce (#3, #4) the 
> jump labels in task_cls_classid() and task_netprioidx(). The jump
> labels are needed to know when it is safe to access the controller. 
> For example not safe means the module is not yet loaded.
> 
> All those patches are just preparation for the center piece (#5) 
> of these series. This one will remove the dynamic subsystem ID
> generation and falls back to compile time generated IDs. 
> 
> This is the first result from the discussion around on the
> "cgroup cls & netprio 'cleanups'" patches.
> 
> This patches are against net-next
> 
> cheers,
> daniel
> 
> Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Cc: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
> Cc: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> Cc: Jamal Hadi Salim <jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org>
> Cc: John Fastabend <john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Kamezawa Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
> Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Cc: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> 
> Daniel Wagner (5):
>   cgroup: Use empty task_cls_classid() when !CONFIG_NET_CLS(_MODULE)
>   cgroup: Move sock_update_classid() decleration to cls_cgroup.h
>   cgroup: Protect access to task_cls_classid() when built as module
>   cgroup: Protect access to task_netprioidx() when built as module
>   cgroup: Assign subsystem IDs during compile time
> 
>  include/linux/cgroup.h        | 20 +++++++++++++-------
>  include/linux/cgroup_subsys.h | 24 ++++++++++++------------
>  include/net/cls_cgroup.h      | 42 +++++++++++++++++++++++++++++-------------
>  include/net/netprio_cgroup.h  | 23 +++++++++++------------
>  include/net/sock.h            |  8 --------
>  kernel/cgroup.c               | 31 +++++++++----------------------
>  net/core/netprio_cgroup.c     | 17 ++++++-----------
>  net/core/sock.c               | 12 ++++++------
>  net/sched/cls_cgroup.c        | 18 +++++-------------
>  9 files changed, 91 insertions(+), 104 deletions(-)
> 
> -- 
> 1.7.12.rc1.16.g05a20c8
> 
> 


The series seems reasonable.  I presume you've testing building and running both
net_prio and net_cls as modules and monolithically?
Neil

Re: [PATCH v0 0/5] cgroup: Assign subsystem IDs during compile time

[Daniel Wagner]

Hi Neil,

On 08/16/2012 09:05 PM, Neil Horman wrote:
>> Daniel Wagner (5):
>>    cgroup: Use empty task_cls_classid() when !CONFIG_NET_CLS(_MODULE)
>>    cgroup: Move sock_update_classid() decleration to cls_cgroup.h
>>    cgroup: Protect access to task_cls_classid() when built as module
>>    cgroup: Protect access to task_netprioidx() when built as module
>>    cgroup: Assign subsystem IDs during compile time
> 
> The series seems reasonable.  I presume you've testing building and running both
> net_prio and net_cls as modules and monolithically?
> Neil

Yep, I spend a good bit of time rebuilding and testing all patches in
the different configuration. I hope I really got the small semantic
differences between net_cls and net_prio regarding the loading and
unloading correct. So please have a close look at the jump label
patches (#3 and #4). 

BTW, I have a few mores on top of these patches, e.g. merging the
builtin and module version of task_cls_classid()/task_netprioidx()
implementation together and getting rid of the many ifdefs in the
header. But let's first get this part reviewed.

thanks,
daniel

Re: [PATCH v0 0/5] cgroup: Assign subsystem IDs during compile time

[John Fastabend]

On 8/16/2012 1:25 PM, Daniel Wagner wrote:
> Hi Neil,
>
> On 08/16/2012 09:05 PM, Neil Horman wrote:
>>> Daniel Wagner (5):
>>>     cgroup: Use empty task_cls_classid() when !CONFIG_NET_CLS(_MODULE)
>>>     cgroup: Move sock_update_classid() decleration to cls_cgroup.h
>>>     cgroup: Protect access to task_cls_classid() when built as module
>>>     cgroup: Protect access to task_netprioidx() when built as module
>>>     cgroup: Assign subsystem IDs during compile time
>>
>> The series seems reasonable.  I presume you've testing building and running both
>> net_prio and net_cls as modules and monolithically?
>> Neil
>
> Yep, I spend a good bit of time rebuilding and testing all patches in
> the different configuration. I hope I really got the small semantic
> differences between net_cls and net_prio regarding the loading and
> unloading correct. So please have a close look at the jump label
> patches (#3 and #4).
>
> BTW, I have a few mores on top of these patches, e.g. merging the
> builtin and module version of task_cls_classid()/task_netprioidx()
> implementation together and getting rid of the many ifdefs in the
> header. But let's first get this part reviewed.
>
> thanks,
> daniel
>

Also I have another series for netprio against net-next to clean up
some of the locking as suggested by Al Viro. I'll wait to submit those
until after this series has been reviewed/applied.

In the meantime I'll apply these to my local tree and test these.

Re: [PATCH v0 5/5] cgroup: Assign subsystem IDs during compile time

[Tejun Heo]

On Thu, Aug 16, 2012 at 04:12:16PM +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
> 
> We are able to safe some space when we assign the subsystem
> IDs at compile time. Instead of allocating per cgroup
> cgroup->subsys[CGROUP_SUBSYS_COUNT] where CGROUP_SUBSYS_COUNT is
> always 64, we allocate 12 + 1 at max (at this point there are 12
> subsystem).

So, IIUC, this is effectively removing the capability to implement
modularized controller which isn't known at kernel compile time.  Am I
right?

I don't think that's a bad idea but if we're doing that, can't we make
things even simpler?  Do we need to distinguish in-kernel and module
at all?

Li, what do you think about this?

Thanks.

-- 
tejun

Re: [PATCH v0 5/5] cgroup: Assign subsystem IDs during compile time

[Li Zefan]

On 2012/8/17 7:20, Tejun Heo wrote:
> On Thu, Aug 16, 2012 at 04:12:16PM +0200, Daniel Wagner wrote:
>> From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
>>
>> We are able to safe some space when we assign the subsystem
>> IDs at compile time. Instead of allocating per cgroup
>> cgroup->subsys[CGROUP_SUBSYS_COUNT] where CGROUP_SUBSYS_COUNT is
>> always 64, we allocate 12 + 1 at max (at this point there are 12
>> subsystem).
> 
> So, IIUC, this is effectively removing the capability to implement
> modularized controller which isn't known at kernel compile time.  Am I
> right?
> 

I think so.

> I don't think that's a bad idea but if we're doing that, can't we make
> things even simpler?  Do we need to distinguish in-kernel and module
> at all?
> 
> Li, what do you think about this?
> 

I'm definitely all for simplicity, but I'm not sure if we can do better in
simplifying the code for modularized cgroup subsystem. (I guess you didn't
mean to remove this feature?)

Re: [PATCH v0 3/5] cgroup: Protect access to task_cls_classid() when built as module

[Li Zefan]

> +#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
> +extern struct static_key cgroup_cls_enabled;
> +#define clscg_enabled static_key_false(&cgroup_cls_enabled)
> +#endif
> +

If it's built-in, clscg_enabled is always true (after we call cgroup_subsys_init at
boot), so we don't need jump label at all.

>  extern void sock_update_classid(struct sock *sk);
>  
>  #if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
> @@ -52,7 +58,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
>  	int id;
>  	u32 classid = 0;
>  
> -	if (in_interrupt())
> +	if (!clscg_enabled || in_interrupt())
>  		return 0;
>  
>  	rcu_read_lock();
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8f67ced..8d3a400 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -327,6 +327,11 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
>  EXPORT_SYMBOL(__sk_backlog_rcv);
>  
>  #if defined(CONFIG_CGROUPS)
> +#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
> +struct static_key cgroup_cls_enabled = STATIC_KEY_INIT_FALSE;
> +EXPORT_SYMBOL_GPL(cgroup_cls_enabled);
> +#endif
> +
>  #if !defined(CONFIG_NET_CLS_CGROUP)
>  int net_cls_subsys_id = -1;
>  EXPORT_SYMBOL_GPL(net_cls_subsys_id);
> diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
> index 7743ea8..f40086b 100644
> --- a/net/sched/cls_cgroup.c
> +++ b/net/sched/cls_cgroup.c
> @@ -44,12 +44,17 @@ static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
>  
>  	if (cgrp->parent)
>  		cs->classid = cgrp_cls_state(cgrp->parent)->classid;
> +	else if (!clscg_enabled)
> +                static_key_slow_inc(&cgroup_cls_enabled);

It's not necessary to check if !clsg_enabled.

>  
>  	return &cs->css;
>  }
>  
>  static void cgrp_destroy(struct cgroup *cgrp)
>  {
> +	if (!cgrp->parent && clscg_enabled)
> +                static_key_slow_dec(&cgroup_cls_enabled);
> +

ditto.

>  	kfree(cgrp_cls_state(cgrp));
>  }
>  
> 

Re: [PATCH v0 3/5] cgroup: Protect access to task_cls_classid() when built as module

[Daniel Wagner]

On 17.08.2012 09:35, Li Zefan wrote:
>> +#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
>> +extern struct static_key cgroup_cls_enabled;
>> +#define clscg_enabled static_key_false(&cgroup_cls_enabled)
>> +#endif
>> +
> 
> If it's built-in, clscg_enabled is always true (after we call cgroup_subsys_init at
> boot), so we don't need jump label at all.

Okay. I'll update the patches so that only the module built contain the
jump labels.

daniel

Re: [PATCH v0 5/5] cgroup: Assign subsystem IDs during compile time

[Daniel Wagner]

On 17.08.2012 09:25, Li Zefan wrote:
> On 2012/8/17 7:20, Tejun Heo wrote:
>> On Thu, Aug 16, 2012 at 04:12:16PM +0200, Daniel Wagner wrote:
>>> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>>>
>>> We are able to safe some space when we assign the subsystem
>>> IDs at compile time. Instead of allocating per cgroup
>>> cgroup->subsys[CGROUP_SUBSYS_COUNT] where CGROUP_SUBSYS_COUNT is
>>> always 64, we allocate 12 + 1 at max (at this point there are 12
>>> subsystem).
>>
>> So, IIUC, this is effectively removing the capability to implement
>> modularized controller which isn't known at kernel compile time.  Am I
>> right?
>>
>
> I think so.

I am preparing an updated version which does not need the extra 1 
pointer. Some more preprocessor magic involved :)

>> I don't think that's a bad idea but if we're doing that, can't we make
>> things even simpler?  Do we need to distinguish in-kernel and module
>> at all?
>>
>> Li, what do you think about this?
>>
>
> I'm definitely all for simplicity, but I'm not sure if we can do better in
> simplifying the code for modularized cgroup subsystem. (I guess you didn't
> mean to remove this feature?)

The new version should also be simpler to review because I don't have to 
touch the loops everywhere.

daniel

Re: [PATCH v0 5/5] cgroup: Assign subsystem IDs during compile time

[Tejun Heo]

Hello, Daniel, Li.

On Fri, Aug 17, 2012 at 10:48:18AM +0200, Daniel Wagner wrote:
> >I think so.
> 
> I am preparing an updated version which does not need the extra 1
> pointer. Some more preprocessor magic involved :)

Please try to refrain from macros as much as possible.  Let's try to
keep things C. :)

> >I'm definitely all for simplicity, but I'm not sure if we can do better in
> >simplifying the code for modularized cgroup subsystem. (I guess you didn't
> >mean to remove this feature?)
> 
> The new version should also be simpler to review because I don't
> have to touch the loops everywhere.

e.g. Why does the proposed code have different variants of
task_cls_classid() for builtin and modular cases?  Why not just use
the same code path if the ID is always static anyway?  Also, longer
term, maybe we can unify root cgroup initialization for built-in and
modular cases?

Thanks.

-- 
tejun