Skip to contentSkip to conversation

cgroups: cleanup for dummy root

PATCH In review Li Zefan 4 participants 5 messages Nov 28, 2008 – Dec 2, 2008 2 messages not in the archive

lore
Share & tools
lore

Li Zefan

author
#0 lore raw

PATCH 1/2 cgroups: cleanup for dummy root

- 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).

---
 kernel/cgroup.c |   24 ++----------------------
 1 files changed, 2 insertions(+), 22 deletions(-)
kernel/cgroup.c +2 -22
index fe00b3b..46d9981 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 */
@@ -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);
Signature · 1 line
1.5.4.rc3
author
#1 lore raw
Quoted 8 lines · 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).

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.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw
Quoted 4 lines · Li Zefan

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

#4 lore raw
In reply to Li Zefan
Quoted 4 lines · Li Zefan

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw
Quoted 8 lines · 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).

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.