* [PATCH v3 1/8] cgroup: net_cls: Move sock_update_classid() declaration to cls_cgroup.h
2012-09-11 16:26 [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
@ 2012-09-11 16:26 ` Daniel Wagner
2012-09-11 16:26 ` [PATCH v3 2/8] cgroup: net_cls: Do not define task_cls_classid() when not selected Daniel Wagner
` (6 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Daniel Wagner @ 2012-09-11 16:26 UTC (permalink / raw)
To: netdev, cgroups
Cc: Daniel Wagner, Gao feng, Jamal Hadi Salim, John Fastabend,
Li Zefan, Neil Horman, Tejun Heo
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 directly.
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 | 6 ++++++
include/net/sock.h | 8 --------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index a4dc5b0..e88527a 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);
+
#ifdef CONFIG_NET_CLS_CGROUP
static inline u32 task_cls_classid(struct task_struct *p)
{
@@ -62,6 +64,10 @@ static inline u32 task_cls_classid(struct task_struct *p)
}
#endif
#else
+static inline void sock_update_classid(struct sock *sk)
+{
+}
+
static inline u32 task_cls_classid(struct task_struct *p)
{
return 0;
diff --git a/include/net/sock.h b/include/net/sock.h
index 84bdaec..868ca7d 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1495,14 +1495,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.315.g682ce8b
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 2/8] cgroup: net_cls: Do not define task_cls_classid() when not selected
2012-09-11 16:26 [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
2012-09-11 16:26 ` [PATCH v3 1/8] cgroup: net_cls: Move sock_update_classid() declaration to cls_cgroup.h Daniel Wagner
@ 2012-09-11 16:26 ` Daniel Wagner
2012-09-11 16:26 ` [PATCH v3 3/8] cgroup: net_prio: Do not define task_netpioidx() " Daniel Wagner
` (5 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Daniel Wagner @ 2012-09-11 16:26 UTC (permalink / raw)
To: netdev, cgroups
Cc: Daniel Wagner, Gao feng, Jamal Hadi Salim, John Fastabend,
Li Zefan, Neil Horman, Tejun Heo
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
task_cls_classid() should not be defined in case the configuration is
CONFIG_NET_CLS_CGROUP=n. The reason is that in a following patch the
net_cls_subsys_id will only be defined if CONFIG_NET_CLS_CGROUP!=n.
When net_cls is not built at all a callee should only get an empty
task_cls_classid() without any references to net_cls_subsys_id.
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 | 11 ++++++-----
net/core/sock.c | 2 ++
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index e88527a..9bd5db9 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -17,7 +17,7 @@
#include <linux/hardirq.h>
#include <linux/rcupdate.h>
-#ifdef CONFIG_CGROUPS
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
struct cgroup_cls_state
{
struct cgroup_subsys_state css;
@@ -26,7 +26,7 @@ struct cgroup_cls_state
extern void sock_update_classid(struct sock *sk);
-#ifdef CONFIG_NET_CLS_CGROUP
+#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
static inline u32 task_cls_classid(struct task_struct *p)
{
int classid;
@@ -41,7 +41,8 @@ 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)
@@ -63,7 +64,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
return classid;
}
#endif
-#else
+#else /* !CGROUP_NET_CLS_CGROUP */
static inline void sock_update_classid(struct sock *sk)
{
}
@@ -72,5 +73,5 @@ static inline u32 task_cls_classid(struct task_struct *p)
{
return 0;
}
-#endif
+#endif /* CGROUP_NET_CLS_CGROUP */
#endif /* _NET_CLS_CGROUP_H */
diff --git a/net/core/sock.c b/net/core/sock.c
index d765156..a2af5ef 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1223,6 +1223,7 @@ static void sk_prot_free(struct proto *prot, struct sock *sk)
}
#ifdef CONFIG_CGROUPS
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
void sock_update_classid(struct sock *sk)
{
u32 classid;
@@ -1234,6 +1235,7 @@ void sock_update_classid(struct sock *sk)
sk->sk_classid = classid;
}
EXPORT_SYMBOL(sock_update_classid);
+#endif
void sock_update_netprioidx(struct sock *sk, struct task_struct *task)
{
--
1.7.12.315.g682ce8b
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 3/8] cgroup: net_prio: Do not define task_netpioidx() when not selected
2012-09-11 16:26 [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
2012-09-11 16:26 ` [PATCH v3 1/8] cgroup: net_cls: Move sock_update_classid() declaration to cls_cgroup.h Daniel Wagner
2012-09-11 16:26 ` [PATCH v3 2/8] cgroup: net_cls: Do not define task_cls_classid() when not selected Daniel Wagner
@ 2012-09-11 16:26 ` Daniel Wagner
[not found] ` <1347380774-9546-4-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
[not found] ` <1347380774-9546-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
` (4 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Daniel Wagner @ 2012-09-11 16:26 UTC (permalink / raw)
To: netdev, cgroups
Cc: Daniel Wagner, Gao feng, Jamal Hadi Salim, John Fastabend,
Li Zefan, Neil Horman, Tejun Heo
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
task_netprioidx() should not be defined in case the configuration is
CONFIG_NETPRIO_CGROUP=n. The reason is that in a following patch the
net_prio_subsys_id will only be defined if CONFIG_NETPRIO_CGROUP!=n.
When net_prio is not built at all any callee should only get an empty
task_netprioidx() without any references to net_prio_subsys_id.
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/netprio_cgroup.h | 12 +++++-------
net/core/sock.c | 2 ++
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index 2719dec..b202de8 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h
@@ -18,14 +18,13 @@
#include <linux/rcupdate.h>
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
struct netprio_map {
struct rcu_head rcu;
u32 priomap_len;
u32 priomap[];
};
-#ifdef CONFIG_CGROUPS
-
struct cgroup_netprio_state {
struct cgroup_subsys_state css;
u32 prioidx;
@@ -71,18 +70,17 @@ static inline u32 task_netprioidx(struct task_struct *p)
rcu_read_unlock();
return idx;
}
+#endif
-#else
+#else /* !CONFIG_NETPRIO_CGROUP */
static inline u32 task_netprioidx(struct task_struct *p)
{
return 0;
}
-#endif /* CONFIG_NETPRIO_CGROUP */
-
-#else
#define sock_update_netprioidx(sk, task)
-#endif
+
+#endif /* CONFIG_NETPRIO_CGROUP */
#endif /* _NET_CLS_CGROUP_H */
diff --git a/net/core/sock.c b/net/core/sock.c
index a2af5ef..a1f1c02 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1237,6 +1237,7 @@ void sock_update_classid(struct sock *sk)
EXPORT_SYMBOL(sock_update_classid);
#endif
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
void sock_update_netprioidx(struct sock *sk, struct task_struct *task)
{
if (in_interrupt())
@@ -1246,6 +1247,7 @@ void sock_update_netprioidx(struct sock *sk, struct task_struct *task)
}
EXPORT_SYMBOL_GPL(sock_update_netprioidx);
#endif
+#endif
/**
* sk_alloc - All socket objects are allocated here
--
1.7.12.315.g682ce8b
^ permalink raw reply related [flat|nested] 22+ messages in thread[parent not found: <1347380774-9546-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>]
* [PATCH v3 4/8] cgroup: Remove CGROUP_BUILTIN_SUBSYS_COUNT
[not found] ` <1347380774-9546-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
@ 2012-09-11 16:26 ` Daniel Wagner
[not found] ` <1347380774-9546-5-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-09-11 16:26 ` [PATCH v3 6/8] cgroup: Do not depend on a given order when populating the subsys array Daniel Wagner
1 sibling, 1 reply; 22+ messages in thread
From: Daniel Wagner @ 2012-09-11 16:26 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA
Cc: Daniel Wagner, Gao feng, Jamal Hadi Salim, John Fastabend,
Li Zefan, Neil Horman, Tejun Heo
From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
CGROUP_BUILTIN_SUBSYS_COUNT is used as start index or stop index when
looping over the subsys array looking either at the builtin or the
module subsystems. Since all the builtin subsystems have an id which
is lower then CGROUP_BUILTIN_SUBSYS_COUNT we know that any module will
have an id larger than CGROUP_BUILTIN_SUBSYS_COUNT. In short the ids
are sorted.
We are about to change id assignment to happen only at compile time
later in this series. That means we can't rely on the above trick
since all ids will always be defined at compile time. Furthermore,
ordering the builtin subsystems and the module subsystems is not
really necessary.
So we need a different way to know which subsystem is a builtin or a
module one. We can use the subsys[]->module pointer for this. Any
place where we need to know if a subsys is module we just check for
the pointer. If it is NULL then the subsystem is a builtin one.
With this we are able to drop the CGROUP_BUILTIN_SUBSYS_COUNT
enum. Though we need to introduce a temporary placeholder so that we
don't get a compilation error when only CONFIG_CGROUP is selected and
no single controller. An empty enum definition is not valid. Later in
this series we are able to remove the placeholder again.
And with this change we get a fix for this:
kernel/cgroup.c: In function ‘cgroup_load_subsys’:
kernel/cgroup.c:4326:38: warning: array subscript is below array bounds [-Warray-bounds]
when CONFIG_CGROUP=y and no built in controller was enabled.
Signed-off-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@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/linux/cgroup.h | 2 +-
kernel/cgroup.c | 75 +++++++++++++++++++++++++++++++-------------------
2 files changed, 48 insertions(+), 29 deletions(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index c90eaa8..2140f91 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -47,7 +47,7 @@ extern const struct file_operations proc_cgroup_operations;
#define SUBSYS(_x) _x ## _subsys_id,
enum cgroup_subsys_id {
#include <linux/cgroup_subsys.h>
- CGROUP_BUILTIN_SUBSYS_COUNT
+ __CGROUP_TEMPORARY_PLACEHOLDER
};
#undef SUBSYS
/*
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 7981850..40f881b 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -88,7 +88,7 @@ static DEFINE_MUTEX(cgroup_root_mutex);
/*
* Generate an array of cgroup subsystem pointers. At boot time, this is
- * populated up to CGROUP_BUILTIN_SUBSYS_COUNT, and modular subsystems are
+ * populated with the built in subsystems, and modular subsystems are
* registered after that. The mutable section of this array is protected by
* cgroup_mutex.
*/
@@ -1291,11 +1291,13 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
* take duplicate reference counts on a subsystem that's already used,
* but rebind_subsystems handles this case.
*/
- for (i = CGROUP_BUILTIN_SUBSYS_COUNT; i < CGROUP_SUBSYS_COUNT; i++) {
+ for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
unsigned long bit = 1UL << i;
if (!(bit & opts->subsys_bits))
continue;
+ if (!subsys[i]->module)
+ continue;
if (!try_module_get(subsys[i]->module)) {
module_pin_failed = true;
break;
@@ -1307,12 +1309,14 @@ 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 >= 0; i--) {
/* drop refcounts only on the ones we took */
unsigned long bit = 1UL << i;
if (!(bit & opts->subsys_bits))
continue;
+ if (!subsys[i]->module)
+ continue;
module_put(subsys[i]->module);
}
return -ENOENT;
@@ -1324,11 +1328,13 @@ 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 = 0; i < CGROUP_SUBSYS_COUNT; i++) {
unsigned long bit = 1UL << i;
if (!(bit & subsys_bits))
continue;
+ if (!subsys[i]->module)
+ continue;
module_put(subsys[i]->module);
}
}
@@ -1401,6 +1407,7 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp)
mutex_init(&cgrp->pidlist_mutex);
INIT_LIST_HEAD(&cgrp->event_list);
spin_lock_init(&cgrp->event_list_lock);
+ memset(cgrp->subsys, 0, sizeof(cgrp->subsys));
}
static void init_cgroup_root(struct cgroupfs_root *root)
@@ -4321,8 +4328,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
* since cgroup_init_subsys will have already taken care of it.
*/
if (ss->module == NULL) {
- /* a few sanity checks */
- BUG_ON(ss->subsys_id >= CGROUP_BUILTIN_SUBSYS_COUNT);
+ /* a sanity check */
BUG_ON(subsys[ss->subsys_id] != ss);
return 0;
}
@@ -4336,7 +4342,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
*/
mutex_lock(&cgroup_mutex);
/* find the first empty slot in the array */
- for (i = CGROUP_BUILTIN_SUBSYS_COUNT; i < CGROUP_SUBSYS_COUNT; i++) {
+ for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
if (subsys[i] == NULL)
break;
}
@@ -4439,7 +4445,6 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss)
mutex_lock(&cgroup_mutex);
/* deassign the subsys_id */
- BUG_ON(ss->subsys_id < CGROUP_BUILTIN_SUBSYS_COUNT);
subsys[ss->subsys_id] = NULL;
/* remove subsystem from rootnode's list of subsystems */
@@ -4502,10 +4507,13 @@ int __init cgroup_init_early(void)
for (i = 0; i < CSS_SET_TABLE_SIZE; i++)
INIT_HLIST_HEAD(&css_set_table[i]);
- /* at bootup time, we don't worry about modular subsystems */
- for (i = 0; i < CGROUP_BUILTIN_SUBSYS_COUNT; i++) {
+ for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
+ /* at bootup time, we don't worry about modular subsystems */
+ if (!ss || (ss && ss->module))
+ continue;
+
BUG_ON(!ss->name);
BUG_ON(strlen(ss->name) > MAX_CGROUP_TYPE_NAMELEN);
BUG_ON(!ss->create);
@@ -4538,9 +4546,12 @@ int __init cgroup_init(void)
if (err)
return err;
- /* at bootup time, we don't worry about modular subsystems */
- for (i = 0; i < CGROUP_BUILTIN_SUBSYS_COUNT; i++) {
+ for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
+
+ /* at bootup time, we don't worry about modular subsystems */
+ if (!ss || (ss && ss->module))
+ continue;
if (!ss->early_init)
cgroup_init_subsys(ss);
if (ss->use_id)
@@ -4735,13 +4746,16 @@ void cgroup_fork_callbacks(struct task_struct *child)
{
if (need_forkexit_callback) {
int i;
- /*
- * forkexit callbacks are only supported for builtin
- * subsystems, and the builtin section of the subsys array is
- * immutable, so we don't need to lock the subsys array here.
- */
- for (i = 0; i < CGROUP_BUILTIN_SUBSYS_COUNT; i++) {
+ for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
+
+ /*
+ * forkexit callbacks are only supported for
+ * builtin subsystems.
+ */
+ if (!ss || (ss && ss->module))
+ continue;
+
if (ss->fork)
ss->fork(child);
}
@@ -4846,12 +4860,13 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks)
tsk->cgroups = &init_css_set;
if (run_callbacks && need_forkexit_callback) {
- /*
- * modular subsystems can't use callbacks, so no need to lock
- * the subsys array
- */
- for (i = 0; i < CGROUP_BUILTIN_SUBSYS_COUNT; i++) {
+ for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
+
+ /* modular subsystems can't use callbacks */
+ if (!ss || (ss && ss->module))
+ continue;
+
if (ss->exit) {
struct cgroup *old_cgrp =
rcu_dereference_raw(cg->subsys[i])->cgroup;
@@ -5037,13 +5052,17 @@ static int __init cgroup_disable(char *str)
while ((token = strsep(&str, ",")) != NULL) {
if (!*token)
continue;
- /*
- * cgroup_disable, being at boot time, can't know about module
- * subsystems, so we don't worry about them.
- */
- for (i = 0; i < CGROUP_BUILTIN_SUBSYS_COUNT; i++) {
+ for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
+ /*
+ * cgroup_disable, being at boot time, can't
+ * know about module subsystems, so we don't
+ * worry about them.
+ */
+ if (!ss || (ss && ss->module))
+ continue;
+
if (!strcmp(token, ss->name)) {
ss->disabled = 1;
printk(KERN_INFO "Disabling %s control group"
--
1.7.12.315.g682ce8b
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 6/8] cgroup: Do not depend on a given order when populating the subsys array
[not found] ` <1347380774-9546-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-09-11 16:26 ` [PATCH v3 4/8] cgroup: Remove CGROUP_BUILTIN_SUBSYS_COUNT Daniel Wagner
@ 2012-09-11 16:26 ` Daniel Wagner
1 sibling, 0 replies; 22+ messages in thread
From: Daniel Wagner @ 2012-09-11 16:26 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA
Cc: Daniel Wagner, Gao feng, Jamal Hadi Salim, John Fastabend,
Li Zefan, Neil Horman, Tejun Heo
From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
The *_subsys_id will be used as index to access the subsys. Therefore
we need to care we populate the subsystem at the correct position by
using designated initialization.
With this change we are able to interleave builtin and modules in the subsys
array.
Signed-off-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@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
---
kernel/cgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 1fec640..5aacbf2 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -92,7 +92,7 @@ static DEFINE_MUTEX(cgroup_root_mutex);
* registered after that. The mutable section of this array is protected by
* cgroup_mutex.
*/
-#define SUBSYS(_x) &_x ## _subsys,
+#define SUBSYS(_x) [_x ## _subsys_id] = &_x ## _subsys,
#define IS_SUBSYS_ENABLED(option) IS_BUILTIN(option)
static struct cgroup_subsys *subsys[CGROUP_SUBSYS_COUNT] = {
#include <linux/cgroup_subsys.h>
--
1.7.12.315.g682ce8b
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 5/8] cgroup: Wrap subsystem selection macro
2012-09-11 16:26 [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
` (3 preceding siblings ...)
[not found] ` <1347380774-9546-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
@ 2012-09-11 16:26 ` Daniel Wagner
2012-09-11 16:26 ` [PATCH v3 7/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
` (2 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Daniel Wagner @ 2012-09-11 16:26 UTC (permalink / raw)
To: netdev, cgroups
Cc: Daniel Wagner, Gao feng, Jamal Hadi Salim, John Fastabend,
Li Zefan, Neil Horman, Tejun Heo
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
Before we are able to define all subsystem ids at compile time we need
a more fine grained control what gets defined when we include
cgroup_subsys.h. For example we define the enums for the subsystems or
to declare for struct cgroup_subsys (builtin subsystem) by including
cgroup_subsys.h and defining SUBSYS accordingly.
Currently, the decision if a subsys is used is defined inside the
header by testing if CONFIG_*=y is true. By moving this test outside
of cgroup_subsys.h we are able to control it on the include level.
This is done by introducing IS_SUBSYS_ENABLED which then is defined
according the task, e.g. is CONFIG_*=y or CONFIG_*=m.
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/linux/cgroup.h | 4 ++++
include/linux/cgroup_subsys.h | 24 ++++++++++++------------
kernel/cgroup.c | 1 +
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 2140f91..412dcbe 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -45,10 +45,12 @@ extern const struct file_operations proc_cgroup_operations;
/* Define the enumeration of all builtin cgroup subsystems */
#define SUBSYS(_x) _x ## _subsys_id,
+#define IS_SUBSYS_ENABLED(option) IS_BUILTIN(option)
enum cgroup_subsys_id {
#include <linux/cgroup_subsys.h>
__CGROUP_TEMPORARY_PLACEHOLDER
};
+#undef IS_SUBSYS_ENABLED
#undef SUBSYS
/*
* This define indicates the maximum number of subsystems that can be loaded
@@ -521,7 +523,9 @@ struct cgroup_subsys {
};
#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
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/kernel/cgroup.c b/kernel/cgroup.c
index 40f881b..1fec640 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -93,6 +93,7 @@ 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>
};
--
1.7.12.315.g682ce8b
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 7/8] cgroup: Assign subsystem IDs during compile time
2012-09-11 16:26 [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
` (4 preceding siblings ...)
2012-09-11 16:26 ` [PATCH v3 5/8] cgroup: Wrap subsystem selection macro Daniel Wagner
@ 2012-09-11 16:26 ` Daniel Wagner
2012-09-11 21:01 ` Tejun Heo
[not found] ` <1347380774-9546-8-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-09-11 16:26 ` [PATCH v3 8/8] cgroup: Define CGROUP_SUBSYS_COUNT according the configuration Daniel Wagner
2012-09-11 21:11 ` [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Tejun Heo
7 siblings, 2 replies; 22+ messages in thread
From: Daniel Wagner @ 2012-09-11 16:26 UTC (permalink / raw)
To: netdev, cgroups
Cc: Daniel Wagner, David S. Miller, Andrew Morton, Eric Dumazet,
Gao feng, Glauber Costa, Jamal Hadi Salim, John Fastabend,
Kamezawa Hiroyuki, Li Zefan, Neil Horman, Tejun Heo
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
WARNING: With this change it is not possible to load external built
controllers anymore.
In case where CONFIG_NETPRIO_CGROUP=m and CONFIG_NET_CLS_CGROUP=m is
set, the type of the corresponding subsys_id should also be of type
enum. Up to now, net_prio_subsys_id and net_cls_subsys_id would be an
int in this configuration.
With switching the macro definition IS_SUBSYS_ENABLED from IS_BUILTIN
to IS_ENABLED, the subsys_id will always be enum for all
subsystems. That means we need to remove all the code which assumes
that net_prio_subsys_id and net_cls_subsys_id is of type int.
The module version of task_netprioidx() and task_cls_classid()
can't rely anymore on the trick to check the value of the id to know
when it is safe to access the subsystem. When the module is not loaded
the id is -1 and when it is loaded it will have a value larger than or
equal 0. Instead, we try to look at the pointer to the subsystem state
if the module is loaded.
Also we are able to remove the code in cgroup.c which assigns the id
during runtime.
Noteworthy is the drop of the synchronize_rcu() call in the module
exit() functions. The reason is that rebind_subsystem() will assign
subsys[*_subsys_id] a NULL pointer and call synchronize_rcu()
afterwards. After that the corresponding module exit() function will
be called. In short when we reach the module exit() function all we
don't need to take any precautions for task_netprioidx() or
task_cls_classid().
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
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
---
include/linux/cgroup.h | 2 +-
include/net/cls_cgroup.h | 14 +++++---------
include/net/netprio_cgroup.h | 14 +++-----------
kernel/cgroup.c | 22 +++-------------------
net/core/netprio_cgroup.c | 11 -----------
net/core/sock.c | 11 -----------
net/sched/cls_cgroup.c | 13 -------------
7 files changed, 12 insertions(+), 75 deletions(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 412dcbe..798c532 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -45,7 +45,7 @@ extern const struct file_operations proc_cgroup_operations;
/* Define the enumeration of all builtin cgroup subsystems */
#define SUBSYS(_x) _x ## _subsys_id,
-#define IS_SUBSYS_ENABLED(option) IS_BUILTIN(option)
+#define IS_SUBSYS_ENABLED(option) IS_ENABLED(option)
enum cgroup_subsys_id {
#include <linux/cgroup_subsys.h>
__CGROUP_TEMPORARY_PLACEHOLDER
diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index 9bd5db9..f30001a 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -42,23 +42,19 @@ static inline u32 task_cls_classid(struct task_struct *p)
return classid;
}
#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;
+ struct cgroup_cls_state *cs;
u32 classid = 0;
if (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;
+ cs = container_of(task_subsys_state(p, net_cls_subsys_id),
+ struct cgroup_cls_state, css);
+ if (cs)
+ classid = cs->classid;
rcu_read_unlock();
return classid;
diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index b202de8..a17ae52 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h
@@ -30,10 +30,6 @@ struct cgroup_netprio_state {
u32 prioidx;
};
-#ifndef CONFIG_NETPRIO_CGROUP
-extern int net_prio_subsys_id;
-#endif
-
extern void sock_update_netprioidx(struct sock *sk, struct task_struct *task);
#if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
@@ -56,17 +52,13 @@ 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;
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);
+ state = container_of(task_subsys_state(p, net_prio_subsys_id),
+ struct cgroup_netprio_state, css);
+ if (state)
idx = state->prioidx;
- }
rcu_read_unlock();
return idx;
}
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 5aacbf2..20ce7f1 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4337,24 +4337,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 = 0; 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
@@ -4363,7 +4347,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);
}
@@ -4379,7 +4363,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 c75e3f9..6bc460c 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -326,9 +326,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
};
@@ -366,10 +364,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);
@@ -386,11 +380,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 a1f1c02..dd33ad7 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -326,17 +326,6 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
}
EXPORT_SYMBOL(__sk_backlog_rcv);
-#if defined(CONFIG_CGROUPS)
-#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)
{
struct timeval tv;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 91de666..0ee9e1d 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -77,9 +77,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,
};
@@ -284,12 +282,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);
@@ -302,11 +294,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.315.g682ce8b
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 7/8] cgroup: Assign subsystem IDs during compile time
2012-09-11 16:26 ` [PATCH v3 7/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
@ 2012-09-11 21:01 ` Tejun Heo
[not found] ` <20120911210109.GZ7677-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
[not found] ` <1347380774-9546-8-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
1 sibling, 1 reply; 22+ messages in thread
From: Tejun Heo @ 2012-09-11 21:01 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev, cgroups, Daniel Wagner, David S. Miller, Andrew Morton,
Eric Dumazet, Gao feng, Glauber Costa, Jamal Hadi Salim,
John Fastabend, Kamezawa Hiroyuki, Li Zefan, Neil Horman
Hello, Daniel.
I generally like this but I still think it's too big a patch.
> diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
> index c75e3f9..6bc460c 100644
> --- a/net/core/netprio_cgroup.c
> +++ b/net/core/netprio_cgroup.c
> @@ -326,9 +326,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
> };
> @@ -366,10 +364,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);
>
> @@ -386,11 +380,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();
For example, it's not evident the above is correct and it's burried
with all other changes. Can't we just assign the fixed subsys ID to
net_prio_subsys_id in this patch? This patch would be correct without
any netprio changes, no? Please separate these changes and explain
them.
BTW, people who use barriers of any kind without explicitly explaining
what's going on need to be kicked hard in the ass. :(
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 22+ messages in thread[parent not found: <1347380774-9546-8-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>]
* Re: [PATCH v3 7/8] cgroup: Assign subsystem IDs during compile time
[not found] ` <1347380774-9546-8-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
@ 2012-09-11 21:04 ` Tejun Heo
[not found] ` <20120911210435.GA7677-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 22+ messages in thread
From: Tejun Heo @ 2012-09-11 21:04 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
Daniel Wagner, David S. Miller, Andrew Morton, Eric Dumazet,
Gao feng, Glauber Costa, Jamal Hadi Salim, John Fastabend,
Kamezawa Hiroyuki, Li Zefan, Neil Horman
Hello, Daniel.
One more thing.
On Tue, Sep 11, 2012 at 06:26:13PM +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
>
> WARNING: With this change it is not possible to load external built
> controllers anymore.
>
> In case where CONFIG_NETPRIO_CGROUP=m and CONFIG_NET_CLS_CGROUP=m is
> set, the type of the corresponding subsys_id should also be of type
> enum. Up to now, net_prio_subsys_id and net_cls_subsys_id would be an
> int in this configuration.
>
> With switching the macro definition IS_SUBSYS_ENABLED from IS_BUILTIN
> to IS_ENABLED, the subsys_id will always be enum for all
> subsystems. That means we need to remove all the code which assumes
> that net_prio_subsys_id and net_cls_subsys_id is of type int.
I don't think int or enum is the matter here. enum is an int. It's
whether the ID is allocated statically or dynamically. Can you please
update the description using those terms instead?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 8/8] cgroup: Define CGROUP_SUBSYS_COUNT according the configuration
2012-09-11 16:26 [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
` (5 preceding siblings ...)
2012-09-11 16:26 ` [PATCH v3 7/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
@ 2012-09-11 16:26 ` Daniel Wagner
2012-09-11 21:11 ` [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Tejun Heo
7 siblings, 0 replies; 22+ messages in thread
From: Daniel Wagner @ 2012-09-11 16:26 UTC (permalink / raw)
To: netdev, cgroups
Cc: Daniel Wagner, Gao feng, Jamal Hadi Salim, John Fastabend,
Li Zefan, Neil Horman, Tejun Heo
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
Since we know exactly how many subsystems exists at compile time we are
able to define CGROUP_SUBSYS_COUNT correctly. CGROUP_SUBSYS_COUNT will
be at max 12 (all controllers enabled). Depending on the architecture
we safe either 32 - 12 pointers (80 bytes) or 64 - 12 pointers (416
bytes) per cgroup.
With this change we can also remove the temporary placeholder to avoid
compilation errors.
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/linux/cgroup.h | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 798c532..5bb73a5 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -48,16 +48,10 @@ extern const struct file_operations proc_cgroup_operations;
#define IS_SUBSYS_ENABLED(option) IS_ENABLED(option)
enum cgroup_subsys_id {
#include <linux/cgroup_subsys.h>
- __CGROUP_TEMPORARY_PLACEHOLDER
+ CGROUP_SUBSYS_COUNT,
};
#undef IS_SUBSYS_ENABLED
#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 {
--
1.7.12.315.g682ce8b
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time
2012-09-11 16:26 [PATCH v3 0/8] cgroup: Assign subsystem IDs during compile time Daniel Wagner
` (6 preceding siblings ...)
2012-09-11 16:26 ` [PATCH v3 8/8] cgroup: Define CGROUP_SUBSYS_COUNT according the configuration Daniel Wagner
@ 2012-09-11 21:11 ` Tejun Heo
[not found] ` <20120911211151.GC7677-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
7 siblings, 1 reply; 22+ messages in thread
From: Tejun Heo @ 2012-09-11 21:11 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev, cgroups, Daniel Wagner, David S. Miller, Andrew Morton,
Eric Dumazet, Gao feng, Glauber Costa, Jamal Hadi Salim,
John Fastabend, Kamezawa Hiroyuki, Li Zefan, Neil Horman
Hello,
On Tue, Sep 11, 2012 at 06:26:06PM +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> Hi,
>
> In this version I tried to concentrate on the main topic of this
> series, so I removed some of the things which were not really needed
> and I have to admit the result looks much better. So I hope that will
> simplify the review for you.
Other than the few nits I like the whole series and, given that all
the changes are entangled with cgroup core, I would like to route it
through cgroup/for-3.7 if nobody objects. Daniel, can you please
rebase on top of the following branch when sending out updated
version?
git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-3.7
Li, can you please review these?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 22+ messages in thread