[PATCH 1/2] cgroups: cleanup for dummy root

[PATCH 1/2] cgroups: cleanup for dummy root

[Li Zefan]

- Don't link rootnode to the root list, so root_list contains active
  hierarchies only as the comment indicates.

- Don't link css_sets to the dummy cgroup, because we never want to
  run through the tasks in that dummpy cgroup (which means run
  through all the tasks in the system).

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/cgroup.c |   24 ++----------------------
 1 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index fe00b3b..46d9981 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -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 */
@@ -184,7 +184,6 @@ struct cg_cgroup_link {
  */
 
 static struct css_set init_css_set;
-static struct cg_cgroup_link init_css_set_link;
 
 /* css_set_lock protects the list of css_set objects, and the
  * chain of tasks off each css_set.  Nests outside task->alloc_lock
@@ -456,15 +455,6 @@ static struct css_set *find_css_set(
 			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);
-	}
 
 	BUG_ON(!list_empty(&tmp_cg_links));
 
@@ -2561,16 +2551,9 @@ 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;
+	root_count = 0;
 	init_task.cgroups = &init_css_set;
 
-	init_css_set_link.cg = &init_css_set;
-	list_add(&init_css_set_link.cgrp_link_list,
-		 &rootnode.top_cgroup.css_sets);
-	list_add(&init_css_set_link.cg_link_list,
-		 &init_css_set.cg_links);
-
 	for (i = 0; i < CSS_SET_TABLE_SIZE; i++)
 		INIT_HLIST_HEAD(&css_set_table[i]);
 
@@ -2674,9 +2657,6 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 		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

Re: [PATCH 1/2] cgroups: cleanup for dummy root

[Li Zefan]

Li Zefan wrote:
> - Don't link rootnode to the root list, so root_list contains active
>   hierarchies only as the comment indicates.
> 
> - Don't link css_sets to the dummy cgroup, because we never want to
>   run through the tasks in that dummpy cgroup (which means run
>   through all the tasks in the system).
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Hmmm...Sorry, this patch is wrong, when a subsystem is inactive thus in
rootnode, it may still iterate tasks in dummy top cgroup, though currently
no subsystem does so.

Re: [PATCH 1/2] cgroups: cleanup for dummy root

[Paul Menage]

On Fri, Nov 28, 2008 at 1:03 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
>
> Hmmm...Sorry, this patch is wrong, when a subsystem is inactive thus in
> rootnode, it may still iterate tasks in dummy top cgroup, though currently
> no subsystem does so.

Right - iterating across the tasks in a cgroup for an unmounted
subsystem is a completely valid thing to do. I suppose it might make
sense to optimize that to just scan the tasklist and thread lists in
that case, to avoid the overhead of maintaining the cg links for the
unmounted subsystems, but you'd have to change the cgroup iterator
first, before you stop maintaining the links. Given that I suspect the
main overhead of maintaining the links is taking css_set_lock, I'm not
sure how much it hurts to be keeping the links for the inactive
hierarchies too and keeping the code simpler.

Paul