cgroup: Remove CGROUP_BUILTIN_SUBSYS_COUNT

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@bmw-carit.de>
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Li Zefan <lizefan@huawei.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Cc: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: netdev@vger.kernel.org
Cc: cgroups@vger.kernel.org

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 145901f..1916cdb 100644 (file)
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -48,7 +48,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 ced292d..1b18090 100644 (file)
--- 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.
  */
@@ -1321,7 +1321,7 @@ 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_mask))
@@ -1337,7 +1337,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 >= 0; i--) {
                        /* drop refcounts only on the ones we took */
                        unsigned long bit = 1UL << i;
 
@@ -1354,7 +1354,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 static void drop_parsed_module_refcounts(unsigned long subsys_mask)
 {
        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_mask))
@@ -4442,8 +4442,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;
        }
@@ -4457,7 +4456,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;
        }
@@ -4560,7 +4559,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 */
@@ -4623,10 +4621,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->module)
+                       continue;
+
                BUG_ON(!ss->name);
                BUG_ON(strlen(ss->name) > MAX_CGROUP_TYPE_NAMELEN);
                BUG_ON(!ss->create);
@@ -4659,9 +4660,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->module)
+                       continue;
                if (!ss->early_init)
                        cgroup_init_subsys(ss);
                if (ss->use_id)
@@ -4856,13 +4860,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->module)
+                               continue;
+
                        if (ss->fork)
                                ss->fork(child);
                }
@@ -4967,12 +4974,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->module)
+                               continue;
+
                        if (ss->exit) {
                                struct cgroup *old_cgrp =
                                        rcu_dereference_raw(cg->subsys[i])->cgroup;
@@ -5158,13 +5166,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->module)
+                               continue;
+
                        if (!strcmp(token, ss->name)) {
                                ss->disabled = 1;
                                printk(KERN_INFO "Disabling %s control group"