cgroups: make root_list contains active hierarchies only

Don't link rootnode to the root list, so root_list contains active hierarchies only as the comment indicates. And rename for_each_root() to for_each_active_root().

Also remove redundant check in cgroup_kill_sb().

Signed-off-by Li Zefan <[email protected]>

---
 kernel/cgroup.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

kernel/cgroup.c +7 -13

index fe00b3b..33ba756 100644
@@ -84,7 +84,7 @@ struct cgroupfs_root {
       /* Tracks how many cgroups are currently defined in hierarchy.*/
       int number_of_cgroups;
 
-       /* A list running through the mounted hierarchies */
+       /* A list running through the active hierarchies */
       struct list_head root_list;
 
       /* Hierarchy-specific flags */

@@ -149,8 +149,8 @@ static int notify_on_release(const struct cgroup *cgrp)
 #define for_each_subsys(_root, _ss) \
 list_for_each_entry(_ss, &_root->subsys_list, sibling)
 
-/* for_each_root() allows you to iterate across the active hierarchies */
-#define for_each_root(_root) \
+/* for_each_active_root() allows you to iterate across the active hierarchies */
+#define for_each_active_root(_root) \
 list_for_each_entry(_root, &roots, root_list)
 
 /* the list of cgroups eligible for automatic release. Protected by

@@ -1114,10 +1114,9 @@ static void cgroup_kill_sb(struct super_block *sb) {
       }
       write_unlock(&css_set_lock);
 
-       if (!list_empty(&root->root_list)) {
-               list_del(&root->root_list);
-               root_count--;
-       }
+       list_del(&root->root_list);
+       root_count--;
+
       mutex_unlock(&cgroup_mutex);
 
       kfree(root);

@@ -2561,7 +2560,6 @@ int __init cgroup_init_early(void)
       INIT_HLIST_NODE(&init_css_set.hlist);
       css_set_count = 1;
       init_cgroup_root(&rootnode);
-       list_add(&rootnode.root_list, &roots);
       root_count = 1;
       init_task.cgroups = &init_css_set;
 

@@ -2668,15 +2666,12 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 
       mutex_lock(&cgroup_mutex);
 
-       for_each_root(root) {
+       for_each_active_root(root) {
               struct cgroup_subsys *ss;
               struct cgroup *cgrp;
               int subsys_id;
               int count = 0;
 
-               /* Skip this hierarchy if it has no active subsystems */
-               if (!root->actual_subsys_bits)
-                       continue;
               seq_printf(m, "%lu:", root->subsys_bits);
               for_each_subsys(root, ss)
                       seq_printf(m, "%s%s", count++ ? "," : "", ss->name);
--

1.5.4.rc3

Don't link rootnode to the root list, so root_list contains active hierarchies only as the comment indicates. And rename for_each_root() to for_each_active_root().

Also remove redundant check in cgroup_kill_sb().

Signed-off-by Li Zefan <[email protected]>

---
 kernel/cgroup.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

kernel/cgroup.c +7 -13

index fe00b3b..33ba756 100644
@@ -84,7 +84,7 @@ struct cgroupfs_root {
       /* Tracks how many cgroups are currently defined in hierarchy.*/
       int number_of_cgroups;
 
-       /* A list running through the mounted hierarchies */
+       /* A list running through the active hierarchies */
       struct list_head root_list;
 
       /* Hierarchy-specific flags */

@@ -149,8 +149,8 @@ static int notify_on_release(const struct cgroup *cgrp)
 #define for_each_subsys(_root, _ss) \
 list_for_each_entry(_ss, &_root->subsys_list, sibling)
 
-/* for_each_root() allows you to iterate across the active hierarchies */
-#define for_each_root(_root) \
+/* for_each_active_root() allows you to iterate across the active hierarchies */
+#define for_each_active_root(_root) \
 list_for_each_entry(_root, &roots, root_list)
 
 /* the list of cgroups eligible for automatic release. Protected by

@@ -1114,10 +1114,9 @@ static void cgroup_kill_sb(struct super_block *sb) {
       }
       write_unlock(&css_set_lock);
 
-       if (!list_empty(&root->root_list)) {
-               list_del(&root->root_list);
-               root_count--;
-       }
+       list_del(&root->root_list);
+       root_count--;
+
       mutex_unlock(&cgroup_mutex);
 
       kfree(root);

@@ -2561,7 +2560,6 @@ int __init cgroup_init_early(void)
       INIT_HLIST_NODE(&init_css_set.hlist);
       css_set_count = 1;
       init_cgroup_root(&rootnode);
-       list_add(&rootnode.root_list, &roots);
       root_count = 1;
       init_task.cgroups = &init_css_set;
 

@@ -2668,15 +2666,12 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 
       mutex_lock(&cgroup_mutex);
 
-       for_each_root(root) {
+       for_each_active_root(root) {
               struct cgroup_subsys *ss;
               struct cgroup *cgrp;
               int subsys_id;
               int count = 0;
 
-               /* Skip this hierarchy if it has no active subsystems */
-               if (!root->actual_subsys_bits)
-                       continue;
               seq_printf(m, "%lu:", root->subsys_bits);
               for_each_subsys(root, ss)
                       seq_printf(m, "%s%s", count++ ? "," : "", ss->name);
--

1.5.4.rc3

Quoted 3 lines · Li Zefan

Don't link rootnode to the root list, so root_list contains active hierarchies only as the comment indicates. And rename for_each_root() to for_each_active_root().

Why is that change an improvement? If you're concerned about the comment's accuracy then it would be better to just change it to say "A list running through all hierarchies (including the inactive subsystem hierarchy)"?

Also, with this change, "root_count" is no longer the length of the "roots" list, but instead is length + 1, which doesn't seem good.

Paul

Quoted 74 lines

Also remove redundant check in cgroup_kill_sb().

Signed-off-by Li Zefan <[email protected]>

---
 kernel/cgroup.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

kernel/cgroup.c +7 -13

index fe00b3b..33ba756 100644
@@ -84,7 +84,7 @@ struct cgroupfs_root {
       /* Tracks how many cgroups are currently defined in hierarchy.*/
       int number_of_cgroups;
 
-       /* A list running through the mounted hierarchies */
+       /* A list running through the active hierarchies */
       struct list_head root_list;
 
       /* Hierarchy-specific flags */

@@ -149,8 +149,8 @@ static int notify_on_release(const struct cgroup *cgrp)
 #define for_each_subsys(_root, _ss) \
 list_for_each_entry(_ss, &_root->subsys_list, sibling)
 
-/* for_each_root() allows you to iterate across the active hierarchies */
-#define for_each_root(_root) \
+/* for_each_active_root() allows you to iterate across the active hierarchies */
+#define for_each_active_root(_root) \
 list_for_each_entry(_root, &roots, root_list)
 
 /* the list of cgroups eligible for automatic release. Protected by

@@ -1114,10 +1114,9 @@ static void cgroup_kill_sb(struct super_block *sb) {
       }
       write_unlock(&css_set_lock);
 
-       if (!list_empty(&root->root_list)) {
-               list_del(&root->root_list);
-               root_count--;
-       }
+       list_del(&root->root_list);
+       root_count--;
+
       mutex_unlock(&cgroup_mutex);
 
       kfree(root);

@@ -2561,7 +2560,6 @@ int __init cgroup_init_early(void)
       INIT_HLIST_NODE(&init_css_set.hlist);
       css_set_count = 1;
       init_cgroup_root(&rootnode);
-       list_add(&rootnode.root_list, &roots);
       root_count = 1;
       init_task.cgroups = &init_css_set;
 

@@ -2668,15 +2666,12 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 
       mutex_lock(&cgroup_mutex);
 
-       for_each_root(root) {
+       for_each_active_root(root) {
               struct cgroup_subsys *ss;
               struct cgroup *cgrp;
               int subsys_id;
               int count = 0;
 
-               /* Skip this hierarchy if it has no active subsystems */
-               if (!root->actual_subsys_bits)
-                       continue;
               seq_printf(m, "%lu:", root->subsys_bits);
               for_each_subsys(root, ss)
                       seq_printf(m, "%s%s", count++ ? "," : "", ss->name);
--

1.5.4.rc3

Quoted 3 lines · Li Zefan

Don't link rootnode to the root list, so root_list contains active hierarchies only as the comment indicates. And rename for_each_root() to for_each_active_root().

Why is that change an improvement? If you're concerned about the comment's accuracy then it would be better to just change it to say "A list running through all hierarchies (including the inactive subsystem hierarchy)"?

Also, with this change, "root_count" is no longer the length of the "roots" list, but instead is length + 1, which doesn't seem good.

Paul

Quoted 74 lines

Also remove redundant check in cgroup_kill_sb().

Signed-off-by Li Zefan <[email protected]>

---
 kernel/cgroup.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

kernel/cgroup.c +7 -13

index fe00b3b..33ba756 100644
@@ -84,7 +84,7 @@ struct cgroupfs_root {
       /* Tracks how many cgroups are currently defined in hierarchy.*/
       int number_of_cgroups;
 
-       /* A list running through the mounted hierarchies */
+       /* A list running through the active hierarchies */
       struct list_head root_list;
 
       /* Hierarchy-specific flags */

@@ -149,8 +149,8 @@ static int notify_on_release(const struct cgroup *cgrp)
 #define for_each_subsys(_root, _ss) \
 list_for_each_entry(_ss, &_root->subsys_list, sibling)
 
-/* for_each_root() allows you to iterate across the active hierarchies */
-#define for_each_root(_root) \
+/* for_each_active_root() allows you to iterate across the active hierarchies */
+#define for_each_active_root(_root) \
 list_for_each_entry(_root, &roots, root_list)
 
 /* the list of cgroups eligible for automatic release. Protected by

@@ -1114,10 +1114,9 @@ static void cgroup_kill_sb(struct super_block *sb) {
       }
       write_unlock(&css_set_lock);
 
-       if (!list_empty(&root->root_list)) {
-               list_del(&root->root_list);
-               root_count--;
-       }
+       list_del(&root->root_list);
+       root_count--;
+
       mutex_unlock(&cgroup_mutex);
 
       kfree(root);

@@ -2561,7 +2560,6 @@ int __init cgroup_init_early(void)
       INIT_HLIST_NODE(&init_css_set.hlist);
       css_set_count = 1;
       init_cgroup_root(&rootnode);
-       list_add(&rootnode.root_list, &roots);
       root_count = 1;
       init_task.cgroups = &init_css_set;
 

@@ -2668,15 +2666,12 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 
       mutex_lock(&cgroup_mutex);
 
-       for_each_root(root) {
+       for_each_active_root(root) {
               struct cgroup_subsys *ss;
               struct cgroup *cgrp;
               int subsys_id;
               int count = 0;
 
-               /* Skip this hierarchy if it has no active subsystems */
-               if (!root->actual_subsys_bits)
-                       continue;
               seq_printf(m, "%lu:", root->subsys_bits);
               for_each_subsys(root, ss)
                       seq_printf(m, "%s%s", count++ ? "," : "", ss->name);
--

1.5.4.rc3