cgroup: remove cgroup->actual_subsys_mask
authorTejun Heo <tj@kernel.org>
Mon, 24 Jun 2013 22:21:47 +0000 (15:21 -0700)
committerTejun Heo <tj@kernel.org>
Mon, 24 Jun 2013 22:21:47 +0000 (15:21 -0700)
cgroup curiously has two subsystem masks, ->subsys_mask and
->actual_subsys_mask.  The latter only exists because the new target
subsys_mask is passed into rebind_subsystems() via @root>subsys_mask.
rebind_subsystems() needs to know what the current mask is to decide
how to reach the target mask so ->actual_subsys_mask is used as the
temp location to remember the current state.

Adding a temporary field to a permanent data structure is rather silly
and can be misleading.  Update rebind_subsystems() to take @added_mask
and @removed_mask instead and remove @root->actual_subsys_mask.

This patch shouldn't introduce any behavior changes.

v2: Comment and description updated as suggested by Li.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Li Zefan <lizefan@huawei.com>
include/linux/cgroup.h
kernel/cgroup.c

index ab27001a2c4a5f5cf94106dd1ea24c5405de1798..4c1eceb8c439e536843ab28ce2385f1e755fc343 100644 (file)
@@ -286,18 +286,12 @@ enum {
 struct cgroupfs_root {
        struct super_block *sb;
 
-       /*
-        * The bitmask of subsystems intended to be attached to this
-        * hierarchy
-        */
+       /* The bitmask of subsystems attached to this hierarchy */
        unsigned long subsys_mask;
 
        /* Unique id for this hierarchy. */
        int hierarchy_id;
 
-       /* The bitmask of subsystems currently attached to this hierarchy */
-       unsigned long actual_subsys_mask;
-
        /* A list running through the attached subsystems */
        struct list_head subsys_list;
 
index 8f296b83b6a327ae516f4a8617f544bcfef277e5..67fc953c816a9a233f9a969e8272089565a1c8c8 100644 (file)
@@ -986,17 +986,14 @@ static void cgroup_d_remove_dir(struct dentry *dentry)
  * returns an error, no reference counts are touched.
  */
 static int rebind_subsystems(struct cgroupfs_root *root,
-                             unsigned long final_subsys_mask)
+                            unsigned long added_mask, unsigned removed_mask)
 {
-       unsigned long added_mask, removed_mask;
        struct cgroup *cgrp = &root->top_cgroup;
        int i;
 
        BUG_ON(!mutex_is_locked(&cgroup_mutex));
        BUG_ON(!mutex_is_locked(&cgroup_root_mutex));
 
-       removed_mask = root->actual_subsys_mask & ~final_subsys_mask;
-       added_mask = final_subsys_mask & ~root->actual_subsys_mask;
        /* Check that any added subsystems are currently free */
        for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
                unsigned long bit = 1UL << i;
@@ -1032,27 +1029,33 @@ static int rebind_subsystems(struct cgroupfs_root *root,
                        BUG_ON(cgrp->subsys[i]);
                        BUG_ON(!cgroup_dummy_top->subsys[i]);
                        BUG_ON(cgroup_dummy_top->subsys[i]->cgroup != cgroup_dummy_top);
+
                        cgrp->subsys[i] = cgroup_dummy_top->subsys[i];
                        cgrp->subsys[i]->cgroup = cgrp;
                        list_move(&ss->sibling, &root->subsys_list);
                        ss->root = root;
                        if (ss->bind)
                                ss->bind(cgrp);
+
                        /* refcount was already taken, and we're keeping it */
+                       root->subsys_mask |= bit;
                } else if (bit & removed_mask) {
                        /* We're removing this subsystem */
                        BUG_ON(ss == NULL);
                        BUG_ON(cgrp->subsys[i] != cgroup_dummy_top->subsys[i]);
                        BUG_ON(cgrp->subsys[i]->cgroup != cgrp);
+
                        if (ss->bind)
                                ss->bind(cgroup_dummy_top);
                        cgroup_dummy_top->subsys[i]->cgroup = cgroup_dummy_top;
                        cgrp->subsys[i] = NULL;
                        cgroup_subsys[i]->root = &cgroup_dummy_root;
                        list_move(&ss->sibling, &cgroup_dummy_root.subsys_list);
+
                        /* subsystem is now free - drop reference on module */
                        module_put(ss->module);
-               } else if (bit & final_subsys_mask) {
+                       root->subsys_mask &= ~bit;
+               } else if (bit & root->subsys_mask) {
                        /* Subsystem state should already exist */
                        BUG_ON(ss == NULL);
                        BUG_ON(!cgrp->subsys[i]);
@@ -1069,7 +1072,6 @@ static int rebind_subsystems(struct cgroupfs_root *root,
                        BUG_ON(cgrp->subsys[i]);
                }
        }
-       root->subsys_mask = root->actual_subsys_mask = final_subsys_mask;
 
        return 0;
 }
@@ -1343,7 +1345,7 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
        if (ret)
                goto out_unlock;
 
-       if (opts.subsys_mask != root->actual_subsys_mask || opts.release_agent)
+       if (opts.subsys_mask != root->subsys_mask || opts.release_agent)
                pr_warning("cgroup: option changes via remount are deprecated (pid=%d comm=%s)\n",
                           task_tgid_nr(current), current->comm);
 
@@ -1365,7 +1367,7 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
         */
        cgroup_clear_directory(cgrp->dentry, false, removed_mask);
 
-       ret = rebind_subsystems(root, opts.subsys_mask);
+       ret = rebind_subsystems(root, added_mask, removed_mask);
        if (ret) {
                /* rebind_subsystems failed, re-populate the removed files */
                cgroup_populate_dir(cgrp, false, removed_mask);
@@ -1634,7 +1636,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
                if (ret)
                        goto unlock_drop;
 
-               ret = rebind_subsystems(root, root->subsys_mask);
+               ret = rebind_subsystems(root, root->subsys_mask, 0);
                if (ret == -EBUSY) {
                        free_cgrp_cset_links(&tmp_links);
                        goto unlock_drop;
@@ -1727,7 +1729,7 @@ static void cgroup_kill_sb(struct super_block *sb) {
        mutex_lock(&cgroup_root_mutex);
 
        /* Rebind all subsystems back to the default hierarchy */
-       ret = rebind_subsystems(root, 0);
+       ret = rebind_subsystems(root, 0, root->subsys_mask);
        /* Shouldn't be able to fail ... */
        BUG_ON(ret);