Re: [RFC PATCH -mm 2/2] cgroup: fold struct cg_cgroup_link into struct css_set

Previous thread: [PATCH] x86: efi vendor_reserved memory type by Cliff Wickman on Thursday, September 11, 2008 - 7:13 am. (1 message)

Next thread: [RFC PATCH -mm 1/2] cgroup: introduce hierarchy_id by Lai Jiangshan on Thursday, September 11, 2008 - 7:23 am. (1 message)
From: Lai Jiangshan
Date: Thursday, September 11, 2008 - 7:23 am

one struct cg_cgroup_link per link is very waste.
This way need to allocate struct cg_cgroup_link for
(css_set_count * hierarchy_count) times. And it will bring fragments
for memory allocation.

This patch fold struct cg_cgroup_link into struct css_set, use
cgroup_link[hierarchy_id] for every hierarchy's cgroup's link.

This patch removes lots of line of code. remove struct cg_cgroup_link
and corresponding code.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 4f281c5..f4ec055 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -173,18 +173,18 @@ struct css_set {
 	struct list_head tasks;
 
 	/*
-	 * List of cg_cgroup_link objects on link chains from
-	 * cgroups referenced from this css_set. Protected by
-	 * css_set_lock
-	 */
-	struct list_head cg_links;
-
-	/*
 	 * Set of subsystem states, one for each subsystem. This array
 	 * is immutable after creation apart from the init_css_set
 	 * during subsystem registration (at boot time).
 	 */
 	struct cgroup_subsys_state *subsys[CGROUP_SUBSYS_COUNT];
+
+	/*
+	 * List running through cgroup_link[hierarchy_id] associated with a
+	 * cgroup of the corresponding hierarchy, anchored on
+	 * cgroup->css_sets. Protected by css_set_lock.
+	 */
+	struct list_head cgroup_link[CGROUP_SUBSYS_COUNT];
 };
 
 /*
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 4bd5560..6c833d1 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -227,21 +227,6 @@ static void cgroup_release_agent(struct work_struct *work);
 static DECLARE_WORK(release_agent_work, cgroup_release_agent);
 static void check_for_release(struct cgroup *cgrp);
 
-/* Link structure for associating css_set objects with cgroups */
-struct cg_cgroup_link {
-	/*
-	 * List running through cg_cgroup_links associated with a
-	 * cgroup, anchored on cgroup->css_sets
-	 */
-	struct list_head cgrp_link_list;
-	/*
-	 * List running through cg_cgroup_links pointing at ...
From: Lai Jiangshan
Date: Thursday, September 11, 2008 - 7:26 am

sorry for not send to Andrew Morton and send to Paul Menage tiwce.

add [To: Andrew Morton <akpm@linux-foundation.org>]

	Thanks, Lai


--

From: Paul Menage
Date: Thursday, September 11, 2008 - 10:50 am

Correct - but in the common case, hierarchy_count==1. So it's 7
pointers (two list_heads and a pointer in the cg_cgroup_link, and a
list_head in the css_set) per css_set. Each additional hierarchy
introduces 5 pointers per css_set with the new cg_cgroup_link. (So
overall, 2 + 5*H)

We're up to 9 cgroup subsystems in -mm right now, so your approach
consumes 18 pointers (9 list heads) per css_set, regardless of how
many hierarchies are mounted.

So I think that the current solution saves memory when there are fewer
than four hierarchies mounted. As the number of cgroup subsystems
increases, the break-even point will increase. I expect that the most
common number of mounted hierarchies will be 0 (but there's only one

Yes, the code is definitely simpler after your patch. It's pretty much
how it was *before* I introduced the cg_cgroup_link structures to
allow an arbitrary number of hierarchies without bloating the css_set
structure. I'm not convinced that we want to go back to the original
way.

Paul
--

From: Lai Jiangshan
Date: Sunday, September 14, 2008 - 7:01 pm

I didn't know this history. Thank you.

Lai

--

Previous thread: [PATCH] x86: efi vendor_reserved memory type by Cliff Wickman on Thursday, September 11, 2008 - 7:13 am. (1 message)

Next thread: [RFC PATCH -mm 1/2] cgroup: introduce hierarchy_id by Lai Jiangshan on Thursday, September 11, 2008 - 7:23 am. (1 message)