cgroups: introduce link_css_set() to remove duplicate code

Add a common function link_css_set() to link a css_set to a cgroup.

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

---
 kernel/cgroup.c |   62 +++++++++++++++++++++---------------------------------
 1 files changed, 24 insertions(+), 38 deletions(-)

kernel/cgroup.c +24 -39

index 4e50e97..1442077 100644
@@ -385,6 +385,19 @@ static int allocate_cg_links(int count, struct list_head *tmp)
       return 0;
 }
 
+static void link_css_set(struct list_head *tmp_cg_links,
+                        struct css_set *cg, struct cgroup *cgrp)
+{
+       struct cg_cgroup_link *link;
+
+       BUG_ON(list_empty(tmp_cg_links));
+       link = list_first_entry(tmp_cg_links, struct cg_cgroup_link,
+                               cgrp_link_list);
+       link->cg = cg;
+       list_move(&link->cgrp_link_list, &cgrp->css_sets);
+       list_add(&link->cg_link_list, &cg->cg_links);
+}
+
 /*
 * find_css_set() takes an existing cgroup group and a
 * cgroup object, and returns a css_set object that's

@@ -400,7 +413,6 @@ static struct css_set *find_css_set(
       int i;
 
       struct list_head tmp_cg_links;
-       struct cg_cgroup_link *link;
 
       struct hlist_head *hhead;
 

@@ -445,26 +457,11 @@ static struct css_set *find_css_set(
                * only do it for the first subsystem in each
                * hierarchy
                */
-               if (ss->root->subsys_list.next == &ss->sibling) {
-                       BUG_ON(list_empty(&tmp_cg_links));
-                       link = list_entry(tmp_cg_links.next,
-                                         struct cg_cgroup_link,
-                                         cgrp_link_list);
-                       list_del(&link->cgrp_link_list);
-                       list_add(&link->cgrp_link_list, &cgrp->css_sets);
-                       link->cg = res;
-                       list_add(&link->cg_link_list, &res->cg_links);
-               }
-       }
-       if (list_empty(&rootnode.subsys_list)) {
-               link = list_entry(tmp_cg_links.next,
-                                 struct cg_cgroup_link,
-                                 cgrp_link_list);
-               list_del(&link->cgrp_link_list);
-               list_add(&link->cgrp_link_list, &dummytop->css_sets);
-               link->cg = res;
-               list_add(&link->cg_link_list, &res->cg_links);
+               if (ss->root->subsys_list.next == &ss->sibling)
+                       link_css_set(&tmp_cg_links, res, cgrp);
       }
+       if (list_empty(&rootnode.subsys_list))
+               link_css_set(&tmp_cg_links, res, dummytop);
 
       BUG_ON(!list_empty(&tmp_cg_links));
 

@@ -992,7 +989,7 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
               root = NULL;
       } else {
               /* New superblock */
-               struct cgroup *cgrp = &root->top_cgroup;
+               struct cgroup *root_cgrp = &root->top_cgroup;
               struct inode *inode;
               int i;
 

@@ -1033,7 +1030,7 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
               list_add(&root->root_list, &roots);
               root_count++;
 
-               sb->s_root->d_fsdata = &root->top_cgroup;
+               sb->s_root->d_fsdata = root_cgrp;
               root->top_cgroup.dentry = sb->s_root;
 
               /* Link the top cgroup in this hierarchy into all

@@ -1044,29 +1041,18 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
                       struct hlist_node *node;
                       struct css_set *cg;
 
-                       hlist_for_each_entry(cg, node, hhead, hlist) {
-                               struct cg_cgroup_link *link;
-
-                               BUG_ON(list_empty(&tmp_cg_links));
-                               link = list_entry(tmp_cg_links.next,
-                                                 struct cg_cgroup_link,
-                                                 cgrp_link_list);
-                               list_del(&link->cgrp_link_list);
-                               link->cg = cg;
-                               list_add(&link->cgrp_link_list,
-                                        &root->top_cgroup.css_sets);
-                               list_add(&link->cg_link_list, &cg->cg_links);
-                       }
+                       hlist_for_each_entry(cg, node, hhead, hlist)
+                               link_css_set(&tmp_cg_links, cg, root_cgrp);
               }
               write_unlock(&css_set_lock);
 
               free_cg_links(&tmp_cg_links);
 
-               BUG_ON(!list_empty(&cgrp->sibling));
-               BUG_ON(!list_empty(&cgrp->children));
+               BUG_ON(!list_empty(&root_cgrp->sibling));
+               BUG_ON(!list_empty(&root_cgrp->children));
               BUG_ON(root->number_of_cgroups != 1);
 
-               cgroup_populate_dir(cgrp);
+               cgroup_populate_dir(root_cgrp);
               mutex_unlock(&inode->i_mutex);
               mutex_unlock(&cgroup_mutex);
       }
--

1.5.4.rc3

Add a common function link_css_set() to link a css_set to a cgroup.

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

---
 kernel/cgroup.c |   62 +++++++++++++++++++++---------------------------------
 1 files changed, 24 insertions(+), 38 deletions(-)

kernel/cgroup.c +24 -39

index 4e50e97..1442077 100644
@@ -385,6 +385,19 @@ static int allocate_cg_links(int count, struct list_head *tmp)
       return 0;
 }
 
+static void link_css_set(struct list_head *tmp_cg_links,
+                        struct css_set *cg, struct cgroup *cgrp)
+{
+       struct cg_cgroup_link *link;
+
+       BUG_ON(list_empty(tmp_cg_links));
+       link = list_first_entry(tmp_cg_links, struct cg_cgroup_link,
+                               cgrp_link_list);
+       link->cg = cg;
+       list_move(&link->cgrp_link_list, &cgrp->css_sets);
+       list_add(&link->cg_link_list, &cg->cg_links);
+}
+
 /*
 * find_css_set() takes an existing cgroup group and a
 * cgroup object, and returns a css_set object that's

@@ -400,7 +413,6 @@ static struct css_set *find_css_set(
       int i;
 
       struct list_head tmp_cg_links;
-       struct cg_cgroup_link *link;
 
       struct hlist_head *hhead;
 

@@ -445,26 +457,11 @@ static struct css_set *find_css_set(
                * only do it for the first subsystem in each
                * hierarchy
                */
-               if (ss->root->subsys_list.next == &ss->sibling) {
-                       BUG_ON(list_empty(&tmp_cg_links));
-                       link = list_entry(tmp_cg_links.next,
-                                         struct cg_cgroup_link,
-                                         cgrp_link_list);
-                       list_del(&link->cgrp_link_list);
-                       list_add(&link->cgrp_link_list, &cgrp->css_sets);
-                       link->cg = res;
-                       list_add(&link->cg_link_list, &res->cg_links);
-               }
-       }
-       if (list_empty(&rootnode.subsys_list)) {
-               link = list_entry(tmp_cg_links.next,
-                                 struct cg_cgroup_link,
-                                 cgrp_link_list);
-               list_del(&link->cgrp_link_list);
-               list_add(&link->cgrp_link_list, &dummytop->css_sets);
-               link->cg = res;
-               list_add(&link->cg_link_list, &res->cg_links);
+               if (ss->root->subsys_list.next == &ss->sibling)
+                       link_css_set(&tmp_cg_links, res, cgrp);
       }
+       if (list_empty(&rootnode.subsys_list))
+               link_css_set(&tmp_cg_links, res, dummytop);
 
       BUG_ON(!list_empty(&tmp_cg_links));
 

@@ -992,7 +989,7 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
               root = NULL;
       } else {
               /* New superblock */
-               struct cgroup *cgrp = &root->top_cgroup;
+               struct cgroup *root_cgrp = &root->top_cgroup;
               struct inode *inode;
               int i;
 

@@ -1033,7 +1030,7 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
               list_add(&root->root_list, &roots);
               root_count++;
 
-               sb->s_root->d_fsdata = &root->top_cgroup;
+               sb->s_root->d_fsdata = root_cgrp;
               root->top_cgroup.dentry = sb->s_root;
 
               /* Link the top cgroup in this hierarchy into all

@@ -1044,29 +1041,18 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
                       struct hlist_node *node;
                       struct css_set *cg;
 
-                       hlist_for_each_entry(cg, node, hhead, hlist) {
-                               struct cg_cgroup_link *link;
-
-                               BUG_ON(list_empty(&tmp_cg_links));
-                               link = list_entry(tmp_cg_links.next,
-                                                 struct cg_cgroup_link,
-                                                 cgrp_link_list);
-                               list_del(&link->cgrp_link_list);
-                               link->cg = cg;
-                               list_add(&link->cgrp_link_list,
-                                        &root->top_cgroup.css_sets);
-                               list_add(&link->cg_link_list, &cg->cg_links);
-                       }
+                       hlist_for_each_entry(cg, node, hhead, hlist)
+                               link_css_set(&tmp_cg_links, cg, root_cgrp);
               }
               write_unlock(&css_set_lock);
 
               free_cg_links(&tmp_cg_links);
 
-               BUG_ON(!list_empty(&cgrp->sibling));
-               BUG_ON(!list_empty(&cgrp->children));
+               BUG_ON(!list_empty(&root_cgrp->sibling));
+               BUG_ON(!list_empty(&root_cgrp->children));
               BUG_ON(root->number_of_cgroups != 1);
 
-               cgroup_populate_dir(cgrp);
+               cgroup_populate_dir(root_cgrp);
               mutex_unlock(&inode->i_mutex);
               mutex_unlock(&cgroup_mutex);
       }
--

1.5.4.rc3

Quoted 3 lines · Li Zefan

Add a common function link_css_set() to link a css_set to a cgroup.

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

Overall the change looks like an improvement, but:

- the new function could do with comments about the semantics of its parameters, particularly tmp_cg_links.

- why are you renaming cgrp -> root_cgrp in cgroup_get_sb()? That seems like unnecessary churn.

How about:

/**
* link_css_set - a helper function to link a css_set to a cgroup
* @ tmp_cg_links: cg_cgroup_link objects allocated by allocate_cg_links()
* @ cg: the css_set to be linked
* @ cgrp: the destination cgroup
*/

Because we are linking all the css_sets to the root cgroup, so I think root_cgrp helps readability.

How about:

/**
* link_css_set - a helper function to link a css_set to a cgroup
* @ tmp_cg_links: cg_cgroup_link objects allocated by allocate_cg_links()
* @ cg: the css_set to be linked
* @ cgrp: the destination cgroup
*/

Because we are linking all the css_sets to the root cgroup, so I think root_cgrp helps readability.

Quoted 3 lines · Li Zefan

Add a common function link_css_set() to link a css_set to a cgroup.

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

Overall the change looks like an improvement, but:

- the new function could do with comments about the semantics of its parameters, particularly tmp_cg_links.

- why are you renaming cgrp -> root_cgrp in cgroup_get_sb()? That seems like unnecessary churn.