[PATCH 0/5] memcg: towards I/O aware memcg v7.

[PATCH 0/5] memcg: towards I/O aware memcg v7.

[KAMEZAWA Hiroyuki]

Major changes from v6 is
 a) added documentation about CSS ID.
 b) fixed typos and bugs.
 c) refleshed some comments

based on mmotm-2010-08-27

Patch brief view:
 1. changes css ID allocation in kernel/cgroup.c
 2. use ID-array in memcg.
 3. record ID to page_cgroup rather than pointer.
 4. make update_file_mapped to be RCU aware routine instead of spinlock.
 5. make update_file_mapped as general-purpose function.


Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/5] cgroup: change allocation of css ID placement

[KAMEZAWA Hiroyuki]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Now, css'id is allocated after ->create() is called. But to make use of ID
in ->create(), it should be available before ->create().

In another thinking, considering the ID is tightly coupled with "css",
it should be allocated when "css" is allocated.
This patch moves alloc_css_id() to css allocation routine. Now, only 2 subsys,
memory and blkio are using ID. (To support complicated hierarchy walk.)

ID will be used in mem cgroup's ->create(), later.

This patch adds css ID documentation which is not provided.

Note:
If someone changes rules of css allocation, ID allocation should be changed.

Changelog: 2010/09/01
 - modified cgroups.txt

Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 Documentation/cgroups/cgroups.txt |   48 ++++++++++++++++++++++++++++++++++++
 block/blk-cgroup.c                |    9 ++++++
 include/linux/cgroup.h            |   16 ++++++------
 kernel/cgroup.c                   |   50 +++++++++++---------------------------
 mm/memcontrol.c                   |    5 +++
 5 files changed, 86 insertions(+), 42 deletions(-)

Index: mmotm-0827/kernel/cgroup.c
===================================================================
--- mmotm-0827.orig/kernel/cgroup.c
+++ mmotm-0827/kernel/cgroup.c
@@ -289,9 +289,6 @@ struct cg_cgroup_link {
 static struct css_set init_css_set;
 static struct cg_cgroup_link init_css_set_link;
 
-static int cgroup_init_idr(struct cgroup_subsys *ss,
-			   struct cgroup_subsys_state *css);
-
 /* css_set_lock protects the list of css_set objects, and the
  * chain of tasks off each css_set.  Nests outside task->alloc_lock
  * due to cgroup_iter_start() */
@@ -770,9 +767,6 @@ static struct backing_dev_info cgroup_ba
 	.capabilities	= BDI_CAP_NO_ACCT_AND_WRITEBACK,
 };
 
-static int alloc_css_id(struct cgroup_subsys *ss,
-			struct cgroup *parent, struct cgroup *child);
-
 static struct inode *cgroup_new_inode(mode_t mode, struct super_block *sb)
 {
 	struct inode *inode = new_inode(sb);
@@ -3258,7 +3252,8 @@ static void init_cgroup_css(struct cgrou
 	css->cgroup = cgrp;
 	atomic_set(&css->refcnt, 1);
 	css->flags = 0;
-	css->id = NULL;
+	if (!ss->use_id)
+		css->id = NULL;
 	if (cgrp == dummytop)
 		set_bit(CSS_ROOT, &css->flags);
 	BUG_ON(cgrp->subsys[ss->subsys_id]);
@@ -3343,12 +3338,6 @@ static long cgroup_create(struct cgroup 
 			goto err_destroy;
 		}
 		init_cgroup_css(css, ss, cgrp);
-		if (ss->use_id) {
-			err = alloc_css_id(ss, parent, cgrp);
-			if (err)
-				goto err_destroy;
-		}
-		/* At error, ->destroy() callback has to free assigned ID. */
 	}
 
 	cgroup_lock_hierarchy(root);
@@ -3710,17 +3699,6 @@ int __init_or_module cgroup_load_subsys(
 
 	/* our new subsystem will be attached to the dummy hierarchy. */
 	init_cgroup_css(css, ss, dummytop);
-	/* init_idr must be after init_cgroup_css because it sets css->id. */
-	if (ss->use_id) {
-		int ret = cgroup_init_idr(ss, css);
-		if (ret) {
-			dummytop->subsys[ss->subsys_id] = NULL;
-			ss->destroy(ss, dummytop);
-			subsys[i] = NULL;
-			mutex_unlock(&cgroup_mutex);
-			return ret;
-		}
-	}
 
 	/*
 	 * Now we need to entangle the css into the existing css_sets. unlike
@@ -3889,8 +3867,6 @@ int __init cgroup_init(void)
 		struct cgroup_subsys *ss = subsys[i];
 		if (!ss->early_init)
 			cgroup_init_subsys(ss);
-		if (ss->use_id)
-			cgroup_init_idr(ss, init_css_set.subsys[ss->subsys_id]);
 	}
 
 	/* Add init_css_set to the hash table */
@@ -4604,8 +4580,8 @@ err_out:
 
 }
 
-static int __init_or_module cgroup_init_idr(struct cgroup_subsys *ss,
-					    struct cgroup_subsys_state *rootcss)
+static int cgroup_init_idr(struct cgroup_subsys *ss,
+			    struct cgroup_subsys_state *rootcss)
 {
 	struct css_id *newid;
 
@@ -4617,21 +4593,25 @@ static int __init_or_module cgroup_init_
 		return PTR_ERR(newid);
 
 	newid->stack[0] = newid->id;
-	newid->css = rootcss;
-	rootcss->id = newid;
+	rcu_assign_pointer(newid->css, rootcss);
+	rcu_assign_pointer(rootcss->id, newid);
 	return 0;
 }
 
-static int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *parent,
-			struct cgroup *child)
+int alloc_css_id(struct cgroup_subsys *ss,
+	struct cgroup *cgrp, struct cgroup_subsys_state *css)
 {
 	int subsys_id, i, depth = 0;
-	struct cgroup_subsys_state *parent_css, *child_css;
+	struct cgroup_subsys_state *parent_css;
+	struct cgroup *parent;
 	struct css_id *child_id, *parent_id;
 
+	if (cgrp == dummytop)
+		return cgroup_init_idr(ss, css);
+
+	parent = cgrp->parent;
 	subsys_id = ss->subsys_id;
 	parent_css = parent->subsys[subsys_id];
-	child_css = child->subsys[subsys_id];
 	parent_id = parent_css->id;
 	depth = parent_id->depth + 1;
 
@@ -4646,7 +4626,7 @@ static int alloc_css_id(struct cgroup_su
 	 * child_id->css pointer will be set after this cgroup is available
 	 * see cgroup_populate_dir()
 	 */
-	rcu_assign_pointer(child_css->id, child_id);
+	rcu_assign_pointer(css->id, child_id);
 
 	return 0;
 }
Index: mmotm-0827/include/linux/cgroup.h
===================================================================
--- mmotm-0827.orig/include/linux/cgroup.h
+++ mmotm-0827/include/linux/cgroup.h
@@ -588,9 +588,11 @@ static inline int cgroup_attach_task_cur
 /*
  * CSS ID is ID for cgroup_subsys_state structs under subsys. This only works
  * if cgroup_subsys.use_id == true. It can be used for looking up and scanning.
- * CSS ID is assigned at cgroup allocation (create) automatically
- * and removed when subsys calls free_css_id() function. This is because
- * the lifetime of cgroup_subsys_state is subsys's matter.
+ * CSS ID must be assigned by subsys itself at cgroup creation and deleted
+ * when subsys calls free_css_id() function. This is because the life time of
+ * of cgroup_subsys_state is subsys's matter.
+ *
+ * ID->css look up is available after cgroup's directory is populated.
  *
  * Looking up and scanning function should be called under rcu_read_lock().
  * Taking cgroup_mutex()/hierarchy_mutex() is not necessary for following calls.
@@ -598,10 +600,10 @@ static inline int cgroup_attach_task_cur
  * destroyed". The caller should check css and cgroup's status.
  */
 
-/*
- * Typically Called at ->destroy(), or somewhere the subsys frees
- * cgroup_subsys_state.
- */
+/* Should be called in ->create() by subsys itself */
+int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *newgr,
+		struct cgroup_subsys_state *css);
+/* Typically Called at ->destroy(), or somewhere the subsys frees css */
 void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css);
 
 /* Find a cgroup_subsys_state which has given ID */
Index: mmotm-0827/mm/memcontrol.c
===================================================================
--- mmotm-0827.orig/mm/memcontrol.c
+++ mmotm-0827/mm/memcontrol.c
@@ -4141,6 +4141,11 @@ mem_cgroup_create(struct cgroup_subsys *
 		if (alloc_mem_cgroup_per_zone_info(mem, node))
 			goto free_out;
 
+	error = alloc_css_id(ss, cont, &mem->css);
+	if (error)
+		goto free_out;
+	/* Here, css_id(&mem->css) works. but css_lookup(id)->mem doesn't */
+
 	/* root ? */
 	if (cont->parent == NULL) {
 		int cpu;
Index: mmotm-0827/block/blk-cgroup.c
===================================================================
--- mmotm-0827.orig/block/blk-cgroup.c
+++ mmotm-0827/block/blk-cgroup.c
@@ -958,9 +958,13 @@ blkiocg_create(struct cgroup_subsys *sub
 {
 	struct blkio_cgroup *blkcg;
 	struct cgroup *parent = cgroup->parent;
+	int ret;
 
 	if (!parent) {
 		blkcg = &blkio_root_cgroup;
+		ret = alloc_css_id(subsys, cgroup, &blkcg->css);
+		if (ret)
+			return ERR_PTR(ret);
 		goto done;
 	}
 
@@ -971,6 +975,11 @@ blkiocg_create(struct cgroup_subsys *sub
 	blkcg = kzalloc(sizeof(*blkcg), GFP_KERNEL);
 	if (!blkcg)
 		return ERR_PTR(-ENOMEM);
+	ret = alloc_css_id(subsys, cgroup, &blkcg->css);
+	if (ret) {
+		kfree(blkcg);
+		return ERR_PTR(ret);
+	}
 
 	blkcg->weight = BLKIO_WEIGHT_DEFAULT;
 done:
Index: mmotm-0827/Documentation/cgroups/cgroups.txt
===================================================================
--- mmotm-0827.orig/Documentation/cgroups/cgroups.txt
+++ mmotm-0827/Documentation/cgroups/cgroups.txt
@@ -621,6 +621,54 @@ and root cgroup. Currently this will onl
 the default hierarchy (which never has sub-cgroups) and a hierarchy
 that is being created/destroyed (and hence has no sub-cgroups).
 
+3.4 cgroup subsys state IDs.
+------------
+When subsystem sets use_id == true, an ID per [cgroup, subsys] is added
+and it will be tied to cgroup_subsys_state object.
+
+When use_id==true can use following interfaces. But please note that
+allocation/free an ID is subsystem's job because cgroup_subsys_state
+object's lifetime is subsystem's matter.
+
+unsigned short css_id(struct cgroup_subsys_state *css)
+
+Returns ID of cgroup_subsys_state
+
+unsigend short css_depth(struct cgroup_subsys_state *css)
+
+Returns the level which "css" is exisiting under hierarchy tree.
+The root cgroup's depth 0, its children are 1, children's children are
+2....
+
+int alloc_css_id(struct struct cgroup_subsys *ss, struct cgroup *newgr,
+                struct cgroup_subsys_state *css);
+
+Attach an new ID to given css under subsystem ([ss, cgroup])
+should be called in ->create() callback.
+
+void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css);
+
+Free ID attached to "css" under subsystem. Should be called before
+"css" is freed.
+
+struct cgroup_subsys_state *css_lookup(struct cgroup_subsys *ss, int id);
+
+Look up cgroup_subsys_state via ID. Should be called under rcu_read_lock().
+
+struct cgroup_subsys_state *css_get_next(struct cgroup_subsys *ss, int id,
+                struct cgroup_subsys_state *root, int *foundid);
+
+Returns ID which is under "root" i.e. under sub-directory of "root"
+cgroup's directory at considering cgroup hierarchy. The order of IDs
+returned by this function is not sorted. Please be careful.
+
+bool css_is_ancestor(struct cgroup_subsys_state *cg,
+                     const struct cgroup_subsys_state *root);
+
+Returns true if "root" and "cs" is under the same hierarchy and
+"root" can be found when you see all ->parent from "cs" until
+the root cgroup.
+
 4. Questions
 ============
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/5] memcg: more use of css ID in memcg.

[KAMEZAWA Hiroyuki]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Now, memory cgroup has an ID per cgroup and make use of it at
 - hierarchy walk,
 - swap recording.

This patch is for making more use of it. The final purpose is
to replace page_cgroup->mem_cgroup's pointer to an unsigned short.

This patch caches a pointer of memcg in an array. By this, we
don't have to call css_lookup() which requires radix-hash walk.
This saves some amount of memory footprint at lookup memcg via id.

Changelog: 20100901
 - added unregster_memcg_id() and did some clean up.
 - removed ->valid.
 - fixed mem_cgroup_num counter handling.

Changelog: 20100825
 - applied comments.

Changelog: 20100811
 - adjusted onto mmotm-2010-08-11
 - fixed RCU related parts.
 - use attach_id() callback.

Changelog: 20100804
 - fixed description in init/Kconfig

Changelog: 20100730
 - fixed rcu_read_unlock() placement.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 init/Kconfig    |   10 +++++++
 mm/memcontrol.c |   71 ++++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 61 insertions(+), 20 deletions(-)

Index: mmotm-0827/mm/memcontrol.c
===================================================================
--- mmotm-0827.orig/mm/memcontrol.c
+++ mmotm-0827/mm/memcontrol.c
@@ -294,6 +294,33 @@ static bool move_file(void)
 					&mc.to->move_charge_at_immigrate);
 }
 
+/* 0 is unused */
+static atomic_t mem_cgroup_num;
+#define NR_MEMCG_GROUPS (CONFIG_MEM_CGROUP_MAX_GROUPS + 1)
+static struct mem_cgroup *mem_cgroups[NR_MEMCG_GROUPS] __read_mostly;
+
+/* Must be called under rcu_read_lock */
+static struct mem_cgroup *id_to_memcg(unsigned short id)
+{
+	struct mem_cgroup *mem;
+	mem = rcu_dereference_check(mem_cgroups[id], rcu_read_lock_held());
+	return mem;
+}
+
+static void register_memcg_id(struct mem_cgroup *mem)
+{
+	int id = css_id(&mem->css);
+	rcu_assign_pointer(mem_cgroups[id], mem);
+}
+
+static void unregister_memcg_id(struct mem_cgroup *mem)
+{
+	int id = css_id(&mem->css);
+	rcu_assign_pointer(mem_cgroups[id], NULL);
+	/* Wait until all references goes. */
+	synchronize_rcu();
+}
+
 /*
  * Maximum loops in mem_cgroup_hierarchical_reclaim(), used for soft
  * limit reclaim to prevent infinite loops, if they ever occur.
@@ -1847,18 +1874,7 @@ static void mem_cgroup_cancel_charge(str
  * it's concern. (dropping refcnt from swap can be called against removed
  * memcg.)
  */
-static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
-{
-	struct cgroup_subsys_state *css;
 
-	/* ID 0 is unused ID */
-	if (!id)
-		return NULL;
-	css = css_lookup(&mem_cgroup_subsys, id);
-	if (!css)
-		return NULL;
-	return container_of(css, struct mem_cgroup, css);
-}
 
 struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
 {
@@ -1879,7 +1895,7 @@ struct mem_cgroup *try_get_mem_cgroup_fr
 		ent.val = page_private(page);
 		id = lookup_swap_cgroup(ent);
 		rcu_read_lock();
-		mem = mem_cgroup_lookup(id);
+		mem = id_to_memcg(id);
 		if (mem && !css_tryget(&mem->css))
 			mem = NULL;
 		rcu_read_unlock();
@@ -2231,7 +2247,7 @@ __mem_cgroup_commit_charge_swapin(struct
 
 		id = swap_cgroup_record(ent, 0);
 		rcu_read_lock();
-		memcg = mem_cgroup_lookup(id);
+		memcg = id_to_memcg(id);
 		if (memcg) {
 			/*
 			 * This recorded memcg can be obsolete one. So, avoid
@@ -2240,9 +2256,10 @@ __mem_cgroup_commit_charge_swapin(struct
 			if (!mem_cgroup_is_root(memcg))
 				res_counter_uncharge(&memcg->memsw, PAGE_SIZE);
 			mem_cgroup_swap_statistics(memcg, false);
+			rcu_read_unlock();
 			mem_cgroup_put(memcg);
-		}
-		rcu_read_unlock();
+		} else
+			rcu_read_unlock();
 	}
 	/*
 	 * At swapin, we may charge account against cgroup which has no tasks.
@@ -2495,7 +2512,7 @@ void mem_cgroup_uncharge_swap(swp_entry_
 
 	id = swap_cgroup_record(ent, 0);
 	rcu_read_lock();
-	memcg = mem_cgroup_lookup(id);
+	memcg = id_to_memcg(id);
 	if (memcg) {
 		/*
 		 * We uncharge this because swap is freed.
@@ -2504,9 +2521,10 @@ void mem_cgroup_uncharge_swap(swp_entry_
 		if (!mem_cgroup_is_root(memcg))
 			res_counter_uncharge(&memcg->memsw, PAGE_SIZE);
 		mem_cgroup_swap_statistics(memcg, false);
+		rcu_read_unlock();
 		mem_cgroup_put(memcg);
-	}
-	rcu_read_unlock();
+	} else
+		rcu_read_unlock();
 }
 
 /**
@@ -4010,6 +4028,9 @@ static struct mem_cgroup *mem_cgroup_all
 	struct mem_cgroup *mem;
 	int size = sizeof(struct mem_cgroup);
 
+	if (atomic_read(&mem_cgroup_num) == NR_MEMCG_GROUPS)
+		return NULL;
+
 	/* Can be very big if MAX_NUMNODES is very big */
 	if (size < PAGE_SIZE)
 		mem = kmalloc(size, GFP_KERNEL);
@@ -4028,6 +4049,8 @@ static struct mem_cgroup *mem_cgroup_all
 			vfree(mem);
 		mem = NULL;
 	}
+	if (mem)
+		atomic_inc(&mem_cgroup_num);
 	return mem;
 }
 
@@ -4049,6 +4072,7 @@ static void __mem_cgroup_free(struct mem
 	mem_cgroup_remove_from_trees(mem);
 	free_css_id(&mem_cgroup_subsys, &mem->css);
 
+	atomic_dec(&mem_cgroup_num);
 	for_each_node_state(node, N_POSSIBLE)
 		free_mem_cgroup_per_zone_info(mem, node);
 
@@ -4059,6 +4083,12 @@ static void __mem_cgroup_free(struct mem
 		vfree(mem);
 }
 
+static void mem_cgroup_free(struct mem_cgroup *mem)
+{
+	unregister_memcg_id(mem);
+	__mem_cgroup_free(mem);
+}
+
 static void mem_cgroup_get(struct mem_cgroup *mem)
 {
 	atomic_inc(&mem->refcnt);
@@ -4068,7 +4098,7 @@ static void __mem_cgroup_put(struct mem_
 {
 	if (atomic_sub_and_test(count, &mem->refcnt)) {
 		struct mem_cgroup *parent = parent_mem_cgroup(mem);
-		__mem_cgroup_free(mem);
+		mem_cgroup_free(mem);
 		if (parent)
 			mem_cgroup_put(parent);
 	}
@@ -4189,9 +4219,10 @@ mem_cgroup_create(struct cgroup_subsys *
 	atomic_set(&mem->refcnt, 1);
 	mem->move_charge_at_immigrate = 0;
 	mutex_init(&mem->thresholds_lock);
+	register_memcg_id(mem);
 	return &mem->css;
 free_out:
-	__mem_cgroup_free(mem);
+	mem_cgroup_free(mem);
 	root_mem_cgroup = NULL;
 	return ERR_PTR(error);
 }
Index: mmotm-0827/init/Kconfig
===================================================================
--- mmotm-0827.orig/init/Kconfig
+++ mmotm-0827/init/Kconfig
@@ -612,6 +612,16 @@ config CGROUP_MEM_RES_CTLR_SWAP
 	  Now, memory usage of swap_cgroup is 2 bytes per entry. If swap page
 	  size is 4096bytes, 512k per 1Gbytes of swap.
 
+config MEM_CGROUP_MAX_GROUPS
+	int "Maximum number of memory cgroups on a system"
+	range 1 65535
+	default 8192 if 64BIT
+	default 2048 if 32BIT
+	help
+	  Memory cgroup has limitation of the number of groups created.
+	  Please select your favorite value. The more you allow, the more
+	  memory(a pointer per group) will be consumed.
+
 menuconfig CGROUP_SCHED
 	bool "Group CPU scheduler"
 	depends on EXPERIMENTAL && CGROUPS

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/5] memcg: use ID in page cgroup

[KAMEZAWA Hiroyuki]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Now, addresses of memory cgroup can be calculated by their ID without complex.
This patch relplaces pc->mem_cgroup from a pointer to a unsigned short.
On 64bit architecture, this offers us more 6bytes room per page_cgroup.
Use 2bytes for blkio-cgroup's page tracking. More 4bytes will be used for
some light-weight concurrent access.

We may able to move this id onto flags field but ...go step by step.

Changelog: 20100824
 - fixed comments, and typo.
Changelog: 20100811
 - using new rcu APIs, as rcu_dereference_check() etc.
Changelog: 20100804
 - added comments to page_cgroup.h
Changelog: 20100730
 - fixed some garbage added by debug code in early stage

Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 include/linux/page_cgroup.h |    6 ++++
 mm/memcontrol.c             |   53 ++++++++++++++++++++++++++++----------------
 mm/page_cgroup.c            |    2 -
 3 files changed, 40 insertions(+), 21 deletions(-)

Index: mmotm-0827/include/linux/page_cgroup.h
===================================================================
--- mmotm-0827.orig/include/linux/page_cgroup.h
+++ mmotm-0827/include/linux/page_cgroup.h
@@ -9,10 +9,14 @@
  * page_cgroup helps us identify information about the cgroup
  * All page cgroups are allocated at boot or memory hotplug event,
  * then the page cgroup for pfn always exists.
+ *
+ * TODO: It seems ID for cgroup can be packed into "flags". But there will
+ * be race between assigning ID <-> set/clear flags. Please be careful.
  */
 struct page_cgroup {
 	unsigned long flags;
-	struct mem_cgroup *mem_cgroup;
+	unsigned short mem_cgroup;	/* ID of assigned memory cgroup */
+	unsigned short blk_cgroup;	/* Not Used..but will be. */
 	struct page *page;
 	struct list_head lru;		/* per cgroup LRU list */
 };
Index: mmotm-0827/mm/page_cgroup.c
===================================================================
--- mmotm-0827.orig/mm/page_cgroup.c
+++ mmotm-0827/mm/page_cgroup.c
@@ -15,7 +15,7 @@ static void __meminit
 __init_page_cgroup(struct page_cgroup *pc, unsigned long pfn)
 {
 	pc->flags = 0;
-	pc->mem_cgroup = NULL;
+	pc->mem_cgroup = 0;
 	pc->page = pfn_to_page(pfn);
 	INIT_LIST_HEAD(&pc->lru);
 }
Index: mmotm-0827/mm/memcontrol.c
===================================================================
--- mmotm-0827.orig/mm/memcontrol.c
+++ mmotm-0827/mm/memcontrol.c
@@ -299,11 +299,15 @@ static atomic_t mem_cgroup_num;
 #define NR_MEMCG_GROUPS (CONFIG_MEM_CGROUP_MAX_GROUPS + 1)
 static struct mem_cgroup *mem_cgroups[NR_MEMCG_GROUPS] __read_mostly;
 
-/* Must be called under rcu_read_lock */
-static struct mem_cgroup *id_to_memcg(unsigned short id)
+/*
+ * Must be called under rcu_read_lock, Set safe==true if you're sure
+ * you're in safe condition...under lock_page_cgroup() etc.
+ */
+static struct mem_cgroup *id_to_memcg(unsigned short id, bool safe)
 {
 	struct mem_cgroup *mem;
-	mem = rcu_dereference_check(mem_cgroups[id], rcu_read_lock_held());
+	mem = rcu_dereference_check(mem_cgroups[id],
+				rcu_read_lock_held() || safe);
 	return mem;
 }
 
@@ -384,7 +388,12 @@ struct cgroup_subsys_state *mem_cgroup_c
 static struct mem_cgroup_per_zone *
 page_cgroup_zoneinfo(struct page_cgroup *pc)
 {
-	struct mem_cgroup *mem = pc->mem_cgroup;
+	/*
+	 * The caller should guarantee this "pc" is under lock. In typical
+	 * case, this function is called by lru function with zone->lru_lock.
+	 * It is a safe access.
+	 */
+	struct mem_cgroup *mem = id_to_memcg(pc->mem_cgroup, true);
 	int nid = page_cgroup_nid(pc);
 	int zid = page_cgroup_zid(pc);
 
@@ -726,6 +735,11 @@ static inline bool mem_cgroup_is_root(st
 	return (mem == root_mem_cgroup);
 }
 
+static inline bool mem_cgroup_is_rootid(unsigned short id)
+{
+	return (id == 1);
+}
+
 /*
  * Following LRU functions are allowed to be used without PCG_LOCK.
  * Operations are called by routine of global LRU independently from memcg.
@@ -758,7 +772,7 @@ void mem_cgroup_del_lru_list(struct page
 	 */
 	mz = page_cgroup_zoneinfo(pc);
 	MEM_CGROUP_ZSTAT(mz, lru) -= 1;
-	if (mem_cgroup_is_root(pc->mem_cgroup))
+	if (mem_cgroup_is_rootid(pc->mem_cgroup))
 		return;
 	VM_BUG_ON(list_empty(&pc->lru));
 	list_del_init(&pc->lru);
@@ -785,7 +799,7 @@ void mem_cgroup_rotate_lru_list(struct p
 	 */
 	smp_rmb();
 	/* unused or root page is not rotated. */
-	if (!PageCgroupUsed(pc) || mem_cgroup_is_root(pc->mem_cgroup))
+	if (!PageCgroupUsed(pc) || mem_cgroup_is_rootid(pc->mem_cgroup))
 		return;
 	mz = page_cgroup_zoneinfo(pc);
 	list_move(&pc->lru, &mz->lists[lru]);
@@ -811,7 +825,7 @@ void mem_cgroup_add_lru_list(struct page
 	mz = page_cgroup_zoneinfo(pc);
 	MEM_CGROUP_ZSTAT(mz, lru) += 1;
 	SetPageCgroupAcctLRU(pc);
-	if (mem_cgroup_is_root(pc->mem_cgroup))
+	if (mem_cgroup_is_rootid(pc->mem_cgroup))
 		return;
 	list_add(&pc->lru, &mz->lists[lru]);
 }
@@ -1500,7 +1514,7 @@ void mem_cgroup_update_file_mapped(struc
 		return;
 
 	lock_page_cgroup(pc);
-	mem = pc->mem_cgroup;
+	mem = id_to_memcg(pc->mem_cgroup, true);
 	if (!mem || !PageCgroupUsed(pc))
 		goto done;
 
@@ -1888,14 +1902,14 @@ struct mem_cgroup *try_get_mem_cgroup_fr
 	pc = lookup_page_cgroup(page);
 	lock_page_cgroup(pc);
 	if (PageCgroupUsed(pc)) {
-		mem = pc->mem_cgroup;
+		mem = id_to_memcg(pc->mem_cgroup, true);
 		if (mem && !css_tryget(&mem->css))
 			mem = NULL;
 	} else if (PageSwapCache(page)) {
 		ent.val = page_private(page);
 		id = lookup_swap_cgroup(ent);
 		rcu_read_lock();
-		mem = id_to_memcg(id);
+		mem = id_to_memcg(id, false);
 		if (mem && !css_tryget(&mem->css))
 			mem = NULL;
 		rcu_read_unlock();
@@ -1924,7 +1938,7 @@ static void __mem_cgroup_commit_charge(s
 		return;
 	}
 
-	pc->mem_cgroup = mem;
+	pc->mem_cgroup = css_id(&mem->css);
 	/*
 	 * We access a page_cgroup asynchronously without lock_page_cgroup().
 	 * Especially when a page_cgroup is taken from a page, pc->mem_cgroup
@@ -1982,7 +1996,7 @@ static void __mem_cgroup_move_account(st
 	VM_BUG_ON(PageLRU(pc->page));
 	VM_BUG_ON(!PageCgroupLocked(pc));
 	VM_BUG_ON(!PageCgroupUsed(pc));
-	VM_BUG_ON(pc->mem_cgroup != from);
+	VM_BUG_ON(id_to_memcg(pc->mem_cgroup, true) != from);
 
 	if (PageCgroupFileMapped(pc)) {
 		/* Update mapped_file data for mem_cgroup */
@@ -1997,7 +2011,7 @@ static void __mem_cgroup_move_account(st
 		mem_cgroup_cancel_charge(from);
 
 	/* caller should have done css_get */
-	pc->mem_cgroup = to;
+	pc->mem_cgroup = css_id(&to->css);
 	mem_cgroup_charge_statistics(to, pc, true);
 	/*
 	 * We charges against "to" which may not have any tasks. Then, "to"
@@ -2017,7 +2031,7 @@ static int mem_cgroup_move_account(struc
 {
 	int ret = -EINVAL;
 	lock_page_cgroup(pc);
-	if (PageCgroupUsed(pc) && pc->mem_cgroup == from) {
+	if (PageCgroupUsed(pc) && id_to_memcg(pc->mem_cgroup, true) == from) {
 		__mem_cgroup_move_account(pc, from, to, uncharge);
 		ret = 0;
 	}
@@ -2247,7 +2261,7 @@ __mem_cgroup_commit_charge_swapin(struct
 
 		id = swap_cgroup_record(ent, 0);
 		rcu_read_lock();
-		memcg = id_to_memcg(id);
+		memcg = id_to_memcg(id, false);
 		if (memcg) {
 			/*
 			 * This recorded memcg can be obsolete one. So, avoid
@@ -2357,7 +2371,7 @@ __mem_cgroup_uncharge_common(struct page
 
 	lock_page_cgroup(pc);
 
-	mem = pc->mem_cgroup;
+	mem = id_to_memcg(pc->mem_cgroup, true);
 
 	if (!PageCgroupUsed(pc))
 		goto unlock_out;
@@ -2512,7 +2526,7 @@ void mem_cgroup_uncharge_swap(swp_entry_
 
 	id = swap_cgroup_record(ent, 0);
 	rcu_read_lock();
-	memcg = id_to_memcg(id);
+	memcg = id_to_memcg(id, false);
 	if (memcg) {
 		/*
 		 * We uncharge this because swap is freed.
@@ -2603,7 +2617,7 @@ int mem_cgroup_prepare_migration(struct 
 	pc = lookup_page_cgroup(page);
 	lock_page_cgroup(pc);
 	if (PageCgroupUsed(pc)) {
-		mem = pc->mem_cgroup;
+		mem = id_to_memcg(pc->mem_cgroup, true);
 		css_get(&mem->css);
 		/*
 		 * At migrating an anonymous page, its mapcount goes down
@@ -4436,7 +4450,8 @@ static int is_target_pte_for_mc(struct v
 		 * mem_cgroup_move_account() checks the pc is valid or not under
 		 * the lock.
 		 */
-		if (PageCgroupUsed(pc) && pc->mem_cgroup == mc.from) {
+		if (PageCgroupUsed(pc) &&
+			id_to_memcg(pc->mem_cgroup, true) == mc.from) {
 			ret = MC_TARGET_PAGE;
 			if (target)
 				target->page = page;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 4/5] memcg: light weight file mapped update lock system

[KAMEZAWA Hiroyuki]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

At accounting file events per memory cgroup, we need to find memory cgroup
via page_cgroup->mem_cgroup. Now, we use lock_page_cgroup().

But, considering the context which page-cgroup for files are accessed,
we can use alternative light-weight mutual execusion in the most case.

At handling file-caches, the only race we have to take care of is "moving"
account, IOW, overwriting page_cgroup->mem_cgroup. 
(See comment in the patch)

Unlike charge/uncharge, "move" happens not so frequently. It happens only when
rmdir() and task-moving (with a special settings.)
This patch adds a race-checker for file-cache-status accounting v.s. account
moving. The new per-cpu-per-memcg counter MEM_CGROUP_ON_MOVE is added.
The routine for account move 
  1. Increment it before start moving
  2. Call synchronize_rcu()
  3. Decrement it after the end of moving.
By this, file-status-counting routine can check it needs to call
lock_page_cgroup(). In most case, I doesn't need to call it.

Changelog: 20100901
 - changes id_to_memcg(pc, true) to be id_to_memcg(pc, false)
   in update_file_mapped()
 - updated comments on lock rule of update_file_mapped()
Changelog: 20100825
 - added a comment about mc.lock
 - fixed bad lock.
Changelog: 20100804
 - added a comment for possible optimization hint.
Changelog: 20100730
 - some cleanup.
Changelog: 20100729
 - replaced __this_cpu_xxx() with this_cpu_xxx
   (because we don't call spinlock)
 - added VM_BUG_ON().

Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/memcontrol.c |   98 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 85 insertions(+), 13 deletions(-)

Index: mmotm-0827/mm/memcontrol.c
===================================================================
--- mmotm-0827.orig/mm/memcontrol.c
+++ mmotm-0827/mm/memcontrol.c
@@ -90,6 +90,7 @@ enum mem_cgroup_stat_index {
 	MEM_CGROUP_STAT_PGPGOUT_COUNT,	/* # of pages paged out */
 	MEM_CGROUP_STAT_SWAPOUT, /* # of pages, swapped out */
 	MEM_CGROUP_EVENTS,	/* incremented at every  pagein/pageout */
+	MEM_CGROUP_ON_MOVE,   /* A check for locking move account/status */
 
 	MEM_CGROUP_STAT_NSTATS,
 };
@@ -1092,7 +1093,52 @@ static unsigned int get_swappiness(struc
 	return swappiness;
 }
 
-/* A routine for testing mem is not under move_account */
+static void mem_cgroup_start_move(struct mem_cgroup *mem)
+{
+	int cpu;
+	/*
+	 * reuse mc.lock.
+	 */
+	spin_lock(&mc.lock);
+	/* TODO: Can we optimize this by for_each_online_cpu() ? */
+	for_each_possible_cpu(cpu)
+		per_cpu(mem->stat->count[MEM_CGROUP_ON_MOVE], cpu) += 1;
+	spin_unlock(&mc.lock);
+
+	synchronize_rcu();
+}
+
+static void mem_cgroup_end_move(struct mem_cgroup *mem)
+{
+	int cpu;
+
+	if (!mem)
+		return;
+	/* for fast checking in mem_cgroup_update_file_stat() etc..*/
+	spin_lock(&mc.lock);
+	for_each_possible_cpu(cpu)
+		per_cpu(mem->stat->count[MEM_CGROUP_ON_MOVE], cpu) -= 1;
+	spin_unlock(&mc.lock);
+}
+
+/*
+ * mem_cgroup_is_moved -- checking a cgroup is mc.from target or not.
+ *                          used for avoiding race.
+ * mem_cgroup_under_move -- checking a cgroup is mc.from or mc.to or
+ *			    under hierarchy of them. used for waiting at
+ *			    memory pressure.
+ * Result of is_moved can be trusted until the end of rcu_read_unlock().
+ * The caller must do
+ *	rcu_read_lock();
+ *	result = mem_cgroup_is_moved();
+ *	.....make use of result here....
+ *	rcu_read_unlock();
+ */
+static bool mem_cgroup_is_moved(struct mem_cgroup *mem)
+{
+	VM_BUG_ON(!rcu_read_lock_held());
+	return this_cpu_read(mem->stat->count[MEM_CGROUP_ON_MOVE]) > 0;
+}
 
 static bool mem_cgroup_under_move(struct mem_cgroup *mem)
 {
@@ -1503,34 +1549,56 @@ bool mem_cgroup_handle_oom(struct mem_cg
 /*
  * Currently used to update mapped file statistics, but the routine can be
  * generalized to update other statistics as well.
+ *
+ * Notes: How to consider race conditions.
+ *
+ * Lock against "charge" is not required. All operations happens after the
+ * page is attached to address_space->mapping, charge must be finished.
+ *
+ * Rquired lock against "move" is held if necessary and account move code
+ * does proper move of statistics data.
+ *
+ * Lock against "uncharge" is not required. Because uncharge() doesn't clear
+ * pc->mem_cgroup, we can catch proper memcg even under race with truncation.
+ * Another thinking, while we call this, we grab a page_count of the page. So,
+ * the page will be never reused for other purpose and page_cgroup->mem_cgroup
+ * will contain the last data it had.
  */
+
 void mem_cgroup_update_file_mapped(struct page *page, int val)
 {
 	struct mem_cgroup *mem;
 	struct page_cgroup *pc;
+	bool need_lock = false;
 
 	pc = lookup_page_cgroup(page);
 	if (unlikely(!pc))
 		return;
-
-	lock_page_cgroup(pc);
-	mem = id_to_memcg(pc->mem_cgroup, true);
-	if (!mem || !PageCgroupUsed(pc))
+	rcu_read_lock();
+	mem = id_to_memcg(pc->mem_cgroup, false);
+	if (likely(mem)) {
+		if (mem_cgroup_is_moved(mem)) {
+			/* need to serialize with move_account */
+			lock_page_cgroup(pc);
+			need_lock = true;
+			mem = id_to_memcg(pc->mem_cgroup, true);
+			if (unlikely(!mem))
+				goto done;
+		}
+	}
+	if (unlikely(!PageCgroupUsed(pc)))
 		goto done;
-
-	/*
-	 * Preemption is already disabled. We can use __this_cpu_xxx
-	 */
 	if (val > 0) {
-		__this_cpu_inc(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
+		this_cpu_inc(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
 		SetPageCgroupFileMapped(pc);
 	} else {
-		__this_cpu_dec(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
+		this_cpu_dec(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
 		ClearPageCgroupFileMapped(pc);
 	}
-
 done:
-	unlock_page_cgroup(pc);
+	if (need_lock)
+		unlock_page_cgroup(pc);
+	rcu_read_unlock();
 }
 
 /*
@@ -3070,6 +3138,7 @@ move_account:
 		lru_add_drain_all();
 		drain_all_stock_sync();
 		ret = 0;
+		mem_cgroup_start_move(mem);
 		for_each_node_state(node, N_HIGH_MEMORY) {
 			for (zid = 0; !ret && zid < MAX_NR_ZONES; zid++) {
 				enum lru_list l;
@@ -3083,6 +3152,7 @@ move_account:
 			if (ret)
 				break;
 		}
+		mem_cgroup_end_move(mem);
 		memcg_oom_recover(mem);
 		/* it seems parent cgroup doesn't have enough mem */
 		if (ret == -ENOMEM)
@@ -4560,6 +4630,7 @@ static void mem_cgroup_clear_mc(void)
 	mc.to = NULL;
 	mc.moving_task = NULL;
 	spin_unlock(&mc.lock);
+	mem_cgroup_end_move(from);
 	memcg_oom_recover(from);
 	memcg_oom_recover(to);
 	wake_up_all(&mc.waitq);
@@ -4590,6 +4661,7 @@ static int mem_cgroup_can_attach(struct 
 			VM_BUG_ON(mc.moved_charge);
 			VM_BUG_ON(mc.moved_swap);
 			VM_BUG_ON(mc.moving_task);
+			mem_cgroup_start_move(from);
 			spin_lock(&mc.lock);
 			mc.from = from;
 			mc.to = mem;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 5/5] memcg: generic file status accounting

[KAMEZAWA Hiroyuki]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Preparing for adding new status arounf file caches.(dirty, writeback,etc..)
Using a unified macro and more generic names.
All counters will have the same rule for updating.

Regiewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 include/linux/memcontrol.h  |   24 ++++++++++++++++++++
 include/linux/page_cgroup.h |   20 ++++++++++++-----
 mm/memcontrol.c             |   51 ++++++++++++++++++--------------------------
 3 files changed, 59 insertions(+), 36 deletions(-)

Index: mmotm-0827/include/linux/memcontrol.h
===================================================================
--- mmotm-0827.orig/include/linux/memcontrol.h
+++ mmotm-0827/include/linux/memcontrol.h
@@ -25,6 +25,30 @@ struct page_cgroup;
 struct page;
 struct mm_struct;
 
+/*
+ * Per-cpu Statistics for memory cgroup.
+ */
+enum mem_cgroup_stat_index {
+	/*
+	 * For MEM_CONTAINER_TYPE_ALL, usage = pagecache + rss.
+	 */
+	MEM_CGROUP_STAT_CACHE,		/* # of pages charged as cache */
+	MEM_CGROUP_STAT_RSS,	   /* # of pages charged as anon rss */
+	MEM_CGROUP_STAT_PGPGIN_COUNT,	/* # of pages paged in */
+	MEM_CGROUP_STAT_PGPGOUT_COUNT,	/* # of pages paged out */
+	MEM_CGROUP_STAT_SWAPOUT, /* # of pages, swapped out */
+	MEM_CGROUP_EVENTS,	/* incremented at every  pagein/pageout */
+	MEM_CGROUP_ON_MOVE,   /* A check for locking move account/status */
+	/* When you add new member for file-stat, please update page_cgroup.h */
+	MEM_CGROUP_FSTAT_BASE,
+	MEM_CGROUP_FSTAT_FILE_MAPPED = MEM_CGROUP_FSTAT_BASE,
+	MEM_CGROUP_FSTAT_END,
+	MEM_CGROUP_STAT_NSTATS = MEM_CGROUP_FSTAT_END,
+};
+
+#define MEMCG_FSTAT_IDX(idx)	((idx) - MEM_CGROUP_FSTAT_BASE)
+#define NR_FILE_FLAGS_MEMCG ((MEM_CGROUP_FSTAT_END - MEM_CGROUP_FSTAT_BASE))
+
 extern unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
 					struct list_head *dst,
 					unsigned long *scanned, int order,
Index: mmotm-0827/mm/memcontrol.c
===================================================================
--- mmotm-0827.orig/mm/memcontrol.c
+++ mmotm-0827/mm/memcontrol.c
@@ -76,24 +76,6 @@ static int really_do_swap_account __init
 #define THRESHOLDS_EVENTS_THRESH (7) /* once in 128 */
 #define SOFTLIMIT_EVENTS_THRESH (10) /* once in 1024 */
 
-/*
- * Statistics for memory cgroup.
- */
-enum mem_cgroup_stat_index {
-	/*
-	 * For MEM_CONTAINER_TYPE_ALL, usage = pagecache + rss.
-	 */
-	MEM_CGROUP_STAT_CACHE, 	   /* # of pages charged as cache */
-	MEM_CGROUP_STAT_RSS,	   /* # of pages charged as anon rss */
-	MEM_CGROUP_STAT_FILE_MAPPED,  /* # of pages charged as file rss */
-	MEM_CGROUP_STAT_PGPGIN_COUNT,	/* # of pages paged in */
-	MEM_CGROUP_STAT_PGPGOUT_COUNT,	/* # of pages paged out */
-	MEM_CGROUP_STAT_SWAPOUT, /* # of pages, swapped out */
-	MEM_CGROUP_EVENTS,	/* incremented at every  pagein/pageout */
-	MEM_CGROUP_ON_MOVE,   /* A check for locking move account/status */
-
-	MEM_CGROUP_STAT_NSTATS,
-};
 
 struct mem_cgroup_stat_cpu {
 	s64 count[MEM_CGROUP_STAT_NSTATS];
@@ -1565,7 +1547,7 @@ bool mem_cgroup_handle_oom(struct mem_cg
  * will contain the last data it had.
  */
 
-void mem_cgroup_update_file_mapped(struct page *page, int val)
+static void mem_cgroup_update_file_stat(struct page *page, int idx, int val)
 {
 	struct mem_cgroup *mem;
 	struct page_cgroup *pc;
@@ -1589,11 +1571,11 @@ void mem_cgroup_update_file_mapped(struc
 	if (unlikely(!PageCgroupUsed(pc)))
 		goto done;
 	if (val > 0) {
-		this_cpu_inc(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
-		SetPageCgroupFileMapped(pc);
+		this_cpu_inc(mem->stat->count[idx]);
+		set_bit(fflag_idx(MEMCG_FSTAT_IDX(idx)), &pc->flags);
 	} else {
-		this_cpu_dec(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
-		ClearPageCgroupFileMapped(pc);
+		this_cpu_dec(mem->stat->count[idx]);
+		clear_bit(fflag_idx(MEMCG_FSTAT_IDX(idx)), &pc->flags);
 	}
 done:
 	if (need_lock)
@@ -1601,6 +1583,12 @@ done:
 	rcu_read_unlock();
 }
 
+void mem_cgroup_update_file_mapped(struct page *page, int val)
+{
+	return mem_cgroup_update_file_stat(page,
+		MEM_CGROUP_FSTAT_FILE_MAPPED, val);
+}
+
 /*
  * size of first charge trial. "32" comes from vmscan.c's magic value.
  * TODO: maybe necessary to use big numbers in big irons.
@@ -2060,17 +2048,20 @@ static void __mem_cgroup_commit_charge(s
 static void __mem_cgroup_move_account(struct page_cgroup *pc,
 	struct mem_cgroup *from, struct mem_cgroup *to, bool uncharge)
 {
+	int i;
 	VM_BUG_ON(from == to);
 	VM_BUG_ON(PageLRU(pc->page));
 	VM_BUG_ON(!PageCgroupLocked(pc));
 	VM_BUG_ON(!PageCgroupUsed(pc));
 	VM_BUG_ON(id_to_memcg(pc->mem_cgroup, true) != from);
 
-	if (PageCgroupFileMapped(pc)) {
-		/* Update mapped_file data for mem_cgroup */
+	for (i = MEM_CGROUP_FSTAT_BASE; i < MEM_CGROUP_FSTAT_END; ++i) {
+		if (!test_bit(fflag_idx(MEMCG_FSTAT_IDX(i)), &pc->flags))
+			continue;
+		/* Update file statistics for mem_cgroup */
 		preempt_disable();
-		__this_cpu_dec(from->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
-		__this_cpu_inc(to->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
+		__this_cpu_dec(from->stat->count[i]);
+		__this_cpu_inc(to->stat->count[i]);
 		preempt_enable();
 	}
 	mem_cgroup_charge_statistics(from, pc, false);
@@ -3501,7 +3492,7 @@ static int mem_cgroup_get_local_stat(str
 	s->stat[MCS_CACHE] += val * PAGE_SIZE;
 	val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_RSS);
 	s->stat[MCS_RSS] += val * PAGE_SIZE;
-	val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_FILE_MAPPED);
+	val = mem_cgroup_read_stat(mem, MEM_CGROUP_FSTAT_FILE_MAPPED);
 	s->stat[MCS_FILE_MAPPED] += val * PAGE_SIZE;
 	val = mem_cgroup_read_stat(mem, MEM_CGROUP_STAT_PGPGIN_COUNT);
 	s->stat[MCS_PGPGIN] += val;
Index: mmotm-0827/include/linux/page_cgroup.h
===================================================================
--- mmotm-0827.orig/include/linux/page_cgroup.h
+++ mmotm-0827/include/linux/page_cgroup.h
@@ -3,6 +3,7 @@
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR
 #include <linux/bit_spinlock.h>
+#include <linux/memcontrol.h> /* for flags */
 /*
  * Page Cgroup can be considered as an extended mem_map.
  * A page_cgroup page is associated with every page descriptor. The
@@ -43,10 +44,22 @@ enum {
 	PCG_CACHE, /* charged as cache */
 	PCG_USED, /* this object is in use. */
 	PCG_ACCT_LRU, /* page has been accounted for */
-	PCG_FILE_MAPPED, /* page is accounted as "mapped" */
 	PCG_MIGRATION, /* under page migration */
+	PCG_FILE_FLAGS_MEMCG, /* see memcontrol.h */
+	PCG_FILE_FLAGS_MEMCG_END
+		= PCG_FILE_FLAGS_MEMCG + NR_FILE_FLAGS_MEMCG - 1,
 };
 
+/*
+ * file-stat flags are defined regarding to memcg's stat information.
+ * Here, just defines a macro for indexing
+ */
+static inline int fflag_idx(int idx)
+{
+	VM_BUG_ON((idx) >= NR_FILE_FLAGS_MEMCG);
+	return (idx) + PCG_FILE_FLAGS_MEMCG;
+}
+
 #define TESTPCGFLAG(uname, lname)			\
 static inline int PageCgroup##uname(struct page_cgroup *pc)	\
 	{ return test_bit(PCG_##lname, &pc->flags); }
@@ -79,11 +92,6 @@ CLEARPCGFLAG(AcctLRU, ACCT_LRU)
 TESTPCGFLAG(AcctLRU, ACCT_LRU)
 TESTCLEARPCGFLAG(AcctLRU, ACCT_LRU)
 
-
-SETPCGFLAG(FileMapped, FILE_MAPPED)
-CLEARPCGFLAG(FileMapped, FILE_MAPPED)
-TESTPCGFLAG(FileMapped, FILE_MAPPED)
-
 SETPCGFLAG(Migration, MIGRATION)
 CLEARPCGFLAG(Migration, MIGRATION)
 TESTPCGFLAG(Migration, MIGRATION)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/5] memcg: more use of css ID in memcg.

[Daisuke Nishimura]

On Wed, 1 Sep 2010 15:42:59 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> Now, memory cgroup has an ID per cgroup and make use of it at
>  - hierarchy walk,
>  - swap recording.
> 
> This patch is for making more use of it. The final purpose is
> to replace page_cgroup->mem_cgroup's pointer to an unsigned short.
> 
> This patch caches a pointer of memcg in an array. By this, we
> don't have to call css_lookup() which requires radix-hash walk.
> This saves some amount of memory footprint at lookup memcg via id.
> 
> Changelog: 20100901
>  - added unregster_memcg_id() and did some clean up.
>  - removed ->valid.
>  - fixed mem_cgroup_num counter handling.
> 
> Changelog: 20100825
>  - applied comments.
> 
> Changelog: 20100811
>  - adjusted onto mmotm-2010-08-11
>  - fixed RCU related parts.
>  - use attach_id() callback.
> 
> Changelog: 20100804
>  - fixed description in init/Kconfig
> 
> Changelog: 20100730
>  - fixed rcu_read_unlock() placement.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] memcg: towards I/O aware memcg v7.

[KAMEZAWA Hiroyuki]

On Wed, 1 Sep 2010 15:39:51 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> 
> Major changes from v6 is
>  a) added documentation about CSS ID.
>  b) fixed typos and bugs.
>  c) refleshed some comments
> 
> based on mmotm-2010-08-27
> 
> Patch brief view:
>  1. changes css ID allocation in kernel/cgroup.c
>  2. use ID-array in memcg.
>  3. record ID to page_cgroup rather than pointer.
>  4. make update_file_mapped to be RCU aware routine instead of spinlock.
>  5. make update_file_mapped as general-purpose function.
> 
> 

It seems that it will be better to re-order this series into

 A-part: lockless update of stats.
 B-part: ID managenet.

2 series. (to get ack.) Thank you for all helps.
BTW, what's the problem with ID patches ?

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/5] cgroup: change allocation of css ID placement

[Greg Thelen]

On Tue, Aug 31, 2010 at 11:41 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> Now, css'id is allocated after ->create() is called. But to make use of ID
> in ->create(), it should be available before ->create().
>
> In another thinking, considering the ID is tightly coupled with "css",
> it should be allocated when "css" is allocated.
> This patch moves alloc_css_id() to css allocation routine. Now, only 2 subsys,
> memory and blkio are using ID. (To support complicated hierarchy walk.)
>
> ID will be used in mem cgroup's ->create(), later.
>
> This patch adds css ID documentation which is not provided.
>
> Note:
> If someone changes rules of css allocation, ID allocation should be changed.
>
> Changelog: 2010/09/01
>  - modified cgroups.txt
>
> Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  Documentation/cgroups/cgroups.txt |   48 ++++++++++++++++++++++++++++++++++++
>  block/blk-cgroup.c                |    9 ++++++
>  include/linux/cgroup.h            |   16 ++++++------
>  kernel/cgroup.c                   |   50 +++++++++++---------------------------
>  mm/memcontrol.c                   |    5 +++
>  5 files changed, 86 insertions(+), 42 deletions(-)
>
> Index: mmotm-0827/kernel/cgroup.c
> ===================================================================
> --- mmotm-0827.orig/kernel/cgroup.c
> +++ mmotm-0827/kernel/cgroup.c
> @@ -289,9 +289,6 @@ struct cg_cgroup_link {
>  static struct css_set init_css_set;
>  static struct cg_cgroup_link init_css_set_link;
>
> -static int cgroup_init_idr(struct cgroup_subsys *ss,
> -                          struct cgroup_subsys_state *css);
> -
>  /* css_set_lock protects the list of css_set objects, and the
>  * chain of tasks off each css_set.  Nests outside task->alloc_lock
>  * due to cgroup_iter_start() */
> @@ -770,9 +767,6 @@ static struct backing_dev_info cgroup_ba
>        .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK,
>  };
>
> -static int alloc_css_id(struct cgroup_subsys *ss,
> -                       struct cgroup *parent, struct cgroup *child);
> -
>  static struct inode *cgroup_new_inode(mode_t mode, struct super_block *sb)
>  {
>        struct inode *inode = new_inode(sb);
> @@ -3258,7 +3252,8 @@ static void init_cgroup_css(struct cgrou
>        css->cgroup = cgrp;
>        atomic_set(&css->refcnt, 1);
>        css->flags = 0;
> -       css->id = NULL;
> +       if (!ss->use_id)
> +               css->id = NULL;
>        if (cgrp == dummytop)
>                set_bit(CSS_ROOT, &css->flags);
>        BUG_ON(cgrp->subsys[ss->subsys_id]);
> @@ -3343,12 +3338,6 @@ static long cgroup_create(struct cgroup
>                        goto err_destroy;
>                }
>                init_cgroup_css(css, ss, cgrp);
> -               if (ss->use_id) {
> -                       err = alloc_css_id(ss, parent, cgrp);
> -                       if (err)
> -                               goto err_destroy;
> -               }
> -               /* At error, ->destroy() callback has to free assigned ID. */
>        }
>
>        cgroup_lock_hierarchy(root);
> @@ -3710,17 +3699,6 @@ int __init_or_module cgroup_load_subsys(
>
>        /* our new subsystem will be attached to the dummy hierarchy. */
>        init_cgroup_css(css, ss, dummytop);
> -       /* init_idr must be after init_cgroup_css because it sets css->id. */
> -       if (ss->use_id) {
> -               int ret = cgroup_init_idr(ss, css);
> -               if (ret) {
> -                       dummytop->subsys[ss->subsys_id] = NULL;
> -                       ss->destroy(ss, dummytop);
> -                       subsys[i] = NULL;
> -                       mutex_unlock(&cgroup_mutex);
> -                       return ret;
> -               }
> -       }
>
>        /*
>         * Now we need to entangle the css into the existing css_sets. unlike
> @@ -3889,8 +3867,6 @@ int __init cgroup_init(void)
>                struct cgroup_subsys *ss = subsys[i];
>                if (!ss->early_init)
>                        cgroup_init_subsys(ss);
> -               if (ss->use_id)
> -                       cgroup_init_idr(ss, init_css_set.subsys[ss->subsys_id]);
>        }
>
>        /* Add init_css_set to the hash table */
> @@ -4604,8 +4580,8 @@ err_out:
>
>  }
>
> -static int __init_or_module cgroup_init_idr(struct cgroup_subsys *ss,
> -                                           struct cgroup_subsys_state *rootcss)
> +static int cgroup_init_idr(struct cgroup_subsys *ss,
> +                           struct cgroup_subsys_state *rootcss)
>  {
>        struct css_id *newid;
>
> @@ -4617,21 +4593,25 @@ static int __init_or_module cgroup_init_
>                return PTR_ERR(newid);
>
>        newid->stack[0] = newid->id;
> -       newid->css = rootcss;
> -       rootcss->id = newid;
> +       rcu_assign_pointer(newid->css, rootcss);
> +       rcu_assign_pointer(rootcss->id, newid);
>        return 0;
>  }
>
> -static int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *parent,
> -                       struct cgroup *child)
> +int alloc_css_id(struct cgroup_subsys *ss,
> +       struct cgroup *cgrp, struct cgroup_subsys_state *css)
Must also add EXPORT_SYMBOL_GPL(alloc_css_id) to supported CONFIG_BLK_CGROUP=m.
>  {
>        int subsys_id, i, depth = 0;
> -       struct cgroup_subsys_state *parent_css, *child_css;
> +       struct cgroup_subsys_state *parent_css;
> +       struct cgroup *parent;
>        struct css_id *child_id, *parent_id;
>
> +       if (cgrp == dummytop)
> +               return cgroup_init_idr(ss, css);
> +
> +       parent = cgrp->parent;
>        subsys_id = ss->subsys_id;
>        parent_css = parent->subsys[subsys_id];
> -       child_css = child->subsys[subsys_id];
>        parent_id = parent_css->id;
>        depth = parent_id->depth + 1;
>
> @@ -4646,7 +4626,7 @@ static int alloc_css_id(struct cgroup_su
>         * child_id->css pointer will be set after this cgroup is available
>         * see cgroup_populate_dir()
>         */
> -       rcu_assign_pointer(child_css->id, child_id);
> +       rcu_assign_pointer(css->id, child_id);
>
>        return 0;
>  }
> Index: mmotm-0827/include/linux/cgroup.h
> ===================================================================
> --- mmotm-0827.orig/include/linux/cgroup.h
> +++ mmotm-0827/include/linux/cgroup.h
> @@ -588,9 +588,11 @@ static inline int cgroup_attach_task_cur
>  /*
>  * CSS ID is ID for cgroup_subsys_state structs under subsys. This only works
>  * if cgroup_subsys.use_id == true. It can be used for looking up and scanning.
> - * CSS ID is assigned at cgroup allocation (create) automatically
> - * and removed when subsys calls free_css_id() function. This is because
> - * the lifetime of cgroup_subsys_state is subsys's matter.
> + * CSS ID must be assigned by subsys itself at cgroup creation and deleted
> + * when subsys calls free_css_id() function. This is because the life time of
To be consistent with document: s/life time/lifetime/
> + * of cgroup_subsys_state is subsys's matter.
> + *
> + * ID->css look up is available after cgroup's directory is populated.
>  *
>  * Looking up and scanning function should be called under rcu_read_lock().
>  * Taking cgroup_mutex()/hierarchy_mutex() is not necessary for following calls.
> @@ -598,10 +600,10 @@ static inline int cgroup_attach_task_cur
>  * destroyed". The caller should check css and cgroup's status.
>  */
>
> -/*
> - * Typically Called at ->destroy(), or somewhere the subsys frees
> - * cgroup_subsys_state.
> - */
> +/* Should be called in ->create() by subsys itself */
> +int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *newgr,
> +               struct cgroup_subsys_state *css);
> +/* Typically Called at ->destroy(), or somewhere the subsys frees css */
s/Called/called/
>  void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css);
>
>  /* Find a cgroup_subsys_state which has given ID */
> Index: mmotm-0827/mm/memcontrol.c
> ===================================================================
> --- mmotm-0827.orig/mm/memcontrol.c
> +++ mmotm-0827/mm/memcontrol.c
> @@ -4141,6 +4141,11 @@ mem_cgroup_create(struct cgroup_subsys *
>                if (alloc_mem_cgroup_per_zone_info(mem, node))
>                        goto free_out;
>
> +       error = alloc_css_id(ss, cont, &mem->css);
> +       if (error)
> +               goto free_out;
> +       /* Here, css_id(&mem->css) works. but css_lookup(id)->mem doesn't */
> +
>        /* root ? */
>        if (cont->parent == NULL) {
>                int cpu;
> Index: mmotm-0827/block/blk-cgroup.c
> ===================================================================
> --- mmotm-0827.orig/block/blk-cgroup.c
> +++ mmotm-0827/block/blk-cgroup.c
> @@ -958,9 +958,13 @@ blkiocg_create(struct cgroup_subsys *sub
>  {
>        struct blkio_cgroup *blkcg;
>        struct cgroup *parent = cgroup->parent;
> +       int ret;
>
>        if (!parent) {
>                blkcg = &blkio_root_cgroup;
> +               ret = alloc_css_id(subsys, cgroup, &blkcg->css);
> +               if (ret)
> +                       return ERR_PTR(ret);
>                goto done;
>        }
>
> @@ -971,6 +975,11 @@ blkiocg_create(struct cgroup_subsys *sub
>        blkcg = kzalloc(sizeof(*blkcg), GFP_KERNEL);
>        if (!blkcg)
>                return ERR_PTR(-ENOMEM);
> +       ret = alloc_css_id(subsys, cgroup, &blkcg->css);
> +       if (ret) {
> +               kfree(blkcg);
> +               return ERR_PTR(ret);
> +       }
>
>        blkcg->weight = BLKIO_WEIGHT_DEFAULT;
>  done:
> Index: mmotm-0827/Documentation/cgroups/cgroups.txt
> ===================================================================
> --- mmotm-0827.orig/Documentation/cgroups/cgroups.txt
> +++ mmotm-0827/Documentation/cgroups/cgroups.txt
> @@ -621,6 +621,54 @@ and root cgroup. Currently this will onl
>  the default hierarchy (which never has sub-cgroups) and a hierarchy
>  that is being created/destroyed (and hence has no sub-cgroups).
>
> +3.4 cgroup subsys state IDs.
> +------------
> +When subsystem sets use_id == true, an ID per [cgroup, subsys] is added
> +and it will be tied to cgroup_subsys_state object.
> +
> +When use_id==true can use following interfaces. But please note that
> +allocation/free an ID is subsystem's job because cgroup_subsys_state
> +object's lifetime is subsystem's matter.
> +
> +unsigned short css_id(struct cgroup_subsys_state *css)
> +
> +Returns ID of cgroup_subsys_state
Please add trailing '.' (period character).

> +
> +unsigend short css_depth(struct cgroup_subsys_state *css)
Typo: s/unsigend/unsigned/
> +
> +Returns the level which "css" is exisiting under hierarchy tree.
> +The root cgroup's depth 0, its children are 1, children's children are
> +2....
> +
> +int alloc_css_id(struct struct cgroup_subsys *ss, struct cgroup *newgr,
> +                struct cgroup_subsys_state *css);
> +
> +Attach an new ID to given css under subsystem ([ss, cgroup])
> +should be called in ->create() callback.
> +
> +void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css);
> +
> +Free ID attached to "css" under subsystem. Should be called before
> +"css" is freed.
> +
> +struct cgroup_subsys_state *css_lookup(struct cgroup_subsys *ss, int id);
> +
> +Look up cgroup_subsys_state via ID. Should be called under rcu_read_lock().
> +
> +struct cgroup_subsys_state *css_get_next(struct cgroup_subsys *ss, int id,
> +                struct cgroup_subsys_state *root, int *foundid);
> +
> +Returns ID which is under "root" i.e. under sub-directory of "root"
> +cgroup's directory at considering cgroup hierarchy. The order of IDs
> +returned by this function is not sorted. Please be careful.
> +
> +bool css_is_ancestor(struct cgroup_subsys_state *cg,
> +                     const struct cgroup_subsys_state *root);

To match code: s/cg/child/

> +
> +Returns true if "root" and "cs" is under the same hierarchy and
> +"root" can be found when you see all ->parent from "cs" until
This may be more clear: s/see all/walk all/

> +the root cgroup.
As above: s/cs/child/

> +
>  4. Questions
>  ============
>
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/5] cgroup: change allocation of css ID placement

[KAMEZAWA Hiroyuki]

Thank you for review.

On Thu, 9 Sep 2010 09:32:32 -0700
Greg Thelen <gthelen@google.com> wrote:

> On Tue, Aug 31, 2010 at 11:41 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> >
> > Now, css'id is allocated after ->create() is called. But to make use of ID
> > in ->create(), it should be available before ->create().
> >
> > In another thinking, considering the ID is tightly coupled with "css",
> > it should be allocated when "css" is allocated.
> > This patch moves alloc_css_id() to css allocation routine. Now, only 2 subsys,
> > memory and blkio are using ID. (To support complicated hierarchy walk.)
> >
> > ID will be used in mem cgroup's ->create(), later.
> >
> > This patch adds css ID documentation which is not provided.
> >
> > Note:
> > If someone changes rules of css allocation, ID allocation should be changed.
> >
> > Changelog: 2010/09/01
> > A - modified cgroups.txt
> >
> > Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> > Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> > A Documentation/cgroups/cgroups.txt | A  48 ++++++++++++++++++++++++++++++++++++
> > A block/blk-cgroup.c A  A  A  A  A  A  A  A | A  A 9 ++++++
> > A include/linux/cgroup.h A  A  A  A  A  A | A  16 ++++++------
> > A kernel/cgroup.c A  A  A  A  A  A  A  A  A  | A  50 +++++++++++---------------------------
> > A mm/memcontrol.c A  A  A  A  A  A  A  A  A  | A  A 5 +++
> > A 5 files changed, 86 insertions(+), 42 deletions(-)
> >
> > Index: mmotm-0827/kernel/cgroup.c
> > ===================================================================
> > --- mmotm-0827.orig/kernel/cgroup.c
> > +++ mmotm-0827/kernel/cgroup.c
> > @@ -289,9 +289,6 @@ struct cg_cgroup_link {
> > A static struct css_set init_css_set;
> > A static struct cg_cgroup_link init_css_set_link;
> >
> > -static int cgroup_init_idr(struct cgroup_subsys *ss,
> > - A  A  A  A  A  A  A  A  A  A  A  A  A struct cgroup_subsys_state *css);
> > -
> > A /* css_set_lock protects the list of css_set objects, and the
> > A * chain of tasks off each css_set. A Nests outside task->alloc_lock
> > A * due to cgroup_iter_start() */
> > @@ -770,9 +767,6 @@ static struct backing_dev_info cgroup_ba
> > A  A  A  A .capabilities A  = BDI_CAP_NO_ACCT_AND_WRITEBACK,
> > A };
> >
> > -static int alloc_css_id(struct cgroup_subsys *ss,
> > - A  A  A  A  A  A  A  A  A  A  A  struct cgroup *parent, struct cgroup *child);
> > -
> > A static struct inode *cgroup_new_inode(mode_t mode, struct super_block *sb)
> > A {
> > A  A  A  A struct inode *inode = new_inode(sb);
> > @@ -3258,7 +3252,8 @@ static void init_cgroup_css(struct cgrou
> > A  A  A  A css->cgroup = cgrp;
> > A  A  A  A atomic_set(&css->refcnt, 1);
> > A  A  A  A css->flags = 0;
> > - A  A  A  css->id = NULL;
> > + A  A  A  if (!ss->use_id)
> > + A  A  A  A  A  A  A  css->id = NULL;
> > A  A  A  A if (cgrp == dummytop)
> > A  A  A  A  A  A  A  A set_bit(CSS_ROOT, &css->flags);
> > A  A  A  A BUG_ON(cgrp->subsys[ss->subsys_id]);
> > @@ -3343,12 +3338,6 @@ static long cgroup_create(struct cgroup
> > A  A  A  A  A  A  A  A  A  A  A  A goto err_destroy;
> > A  A  A  A  A  A  A  A }
> > A  A  A  A  A  A  A  A init_cgroup_css(css, ss, cgrp);
> > - A  A  A  A  A  A  A  if (ss->use_id) {
> > - A  A  A  A  A  A  A  A  A  A  A  err = alloc_css_id(ss, parent, cgrp);
> > - A  A  A  A  A  A  A  A  A  A  A  if (err)
> > - A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  goto err_destroy;
> > - A  A  A  A  A  A  A  }
> > - A  A  A  A  A  A  A  /* At error, ->destroy() callback has to free assigned ID. */
> > A  A  A  A }
> >
> > A  A  A  A cgroup_lock_hierarchy(root);
> > @@ -3710,17 +3699,6 @@ int __init_or_module cgroup_load_subsys(
> >
> > A  A  A  A /* our new subsystem will be attached to the dummy hierarchy. */
> > A  A  A  A init_cgroup_css(css, ss, dummytop);
> > - A  A  A  /* init_idr must be after init_cgroup_css because it sets css->id. */
> > - A  A  A  if (ss->use_id) {
> > - A  A  A  A  A  A  A  int ret = cgroup_init_idr(ss, css);
> > - A  A  A  A  A  A  A  if (ret) {
> > - A  A  A  A  A  A  A  A  A  A  A  dummytop->subsys[ss->subsys_id] = NULL;
> > - A  A  A  A  A  A  A  A  A  A  A  ss->destroy(ss, dummytop);
> > - A  A  A  A  A  A  A  A  A  A  A  subsys[i] = NULL;
> > - A  A  A  A  A  A  A  A  A  A  A  mutex_unlock(&cgroup_mutex);
> > - A  A  A  A  A  A  A  A  A  A  A  return ret;
> > - A  A  A  A  A  A  A  }
> > - A  A  A  }
> >
> > A  A  A  A /*
> > A  A  A  A  * Now we need to entangle the css into the existing css_sets. unlike
> > @@ -3889,8 +3867,6 @@ int __init cgroup_init(void)
> > A  A  A  A  A  A  A  A struct cgroup_subsys *ss = subsys[i];
> > A  A  A  A  A  A  A  A if (!ss->early_init)
> > A  A  A  A  A  A  A  A  A  A  A  A cgroup_init_subsys(ss);
> > - A  A  A  A  A  A  A  if (ss->use_id)
> > - A  A  A  A  A  A  A  A  A  A  A  cgroup_init_idr(ss, init_css_set.subsys[ss->subsys_id]);
> > A  A  A  A }
> >
> > A  A  A  A /* Add init_css_set to the hash table */
> > @@ -4604,8 +4580,8 @@ err_out:
> >
> > A }
> >
> > -static int __init_or_module cgroup_init_idr(struct cgroup_subsys *ss,
> > - A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  struct cgroup_subsys_state *rootcss)
> > +static int cgroup_init_idr(struct cgroup_subsys *ss,
> > + A  A  A  A  A  A  A  A  A  A  A  A  A  struct cgroup_subsys_state *rootcss)
> > A {
> > A  A  A  A struct css_id *newid;
> >
> > @@ -4617,21 +4593,25 @@ static int __init_or_module cgroup_init_
> > A  A  A  A  A  A  A  A return PTR_ERR(newid);
> >
> > A  A  A  A newid->stack[0] = newid->id;
> > - A  A  A  newid->css = rootcss;
> > - A  A  A  rootcss->id = newid;
> > + A  A  A  rcu_assign_pointer(newid->css, rootcss);
> > + A  A  A  rcu_assign_pointer(rootcss->id, newid);
> > A  A  A  A return 0;
> > A }
> >
> > -static int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *parent,
> > - A  A  A  A  A  A  A  A  A  A  A  struct cgroup *child)
> > +int alloc_css_id(struct cgroup_subsys *ss,
> > + A  A  A  struct cgroup *cgrp, struct cgroup_subsys_state *css)
> Must also add EXPORT_SYMBOL_GPL(alloc_css_id) to supported CONFIG_BLK_CGROUP=m.

Ah, yes. 

> > A {
> > A  A  A  A int subsys_id, i, depth = 0;
> > - A  A  A  struct cgroup_subsys_state *parent_css, *child_css;
> > + A  A  A  struct cgroup_subsys_state *parent_css;
> > + A  A  A  struct cgroup *parent;
> > A  A  A  A struct css_id *child_id, *parent_id;
> >
> > + A  A  A  if (cgrp == dummytop)
> > + A  A  A  A  A  A  A  return cgroup_init_idr(ss, css);
> > +
> > + A  A  A  parent = cgrp->parent;
> > A  A  A  A subsys_id = ss->subsys_id;
> > A  A  A  A parent_css = parent->subsys[subsys_id];
> > - A  A  A  child_css = child->subsys[subsys_id];
> > A  A  A  A parent_id = parent_css->id;
> > A  A  A  A depth = parent_id->depth + 1;
> >
> > @@ -4646,7 +4626,7 @@ static int alloc_css_id(struct cgroup_su
> > A  A  A  A  * child_id->css pointer will be set after this cgroup is available
> > A  A  A  A  * see cgroup_populate_dir()
> > A  A  A  A  */
> > - A  A  A  rcu_assign_pointer(child_css->id, child_id);
> > + A  A  A  rcu_assign_pointer(css->id, child_id);
> >
> > A  A  A  A return 0;
> > A }
> > Index: mmotm-0827/include/linux/cgroup.h
> > ===================================================================
> > --- mmotm-0827.orig/include/linux/cgroup.h
> > +++ mmotm-0827/include/linux/cgroup.h
> > @@ -588,9 +588,11 @@ static inline int cgroup_attach_task_cur
> > A /*
> > A * CSS ID is ID for cgroup_subsys_state structs under subsys. This only works
> > A * if cgroup_subsys.use_id == true. It can be used for looking up and scanning.
> > - * CSS ID is assigned at cgroup allocation (create) automatically
> > - * and removed when subsys calls free_css_id() function. This is because
> > - * the lifetime of cgroup_subsys_state is subsys's matter.
> > + * CSS ID must be assigned by subsys itself at cgroup creation and deleted
> > + * when subsys calls free_css_id() function. This is because the life time of
> To be consistent with document: s/life time/lifetime/
> > + * of cgroup_subsys_state is subsys's matter.
> > + *
> > + * ID->css look up is available after cgroup's directory is populated.
> > A *
> > A * Looking up and scanning function should be called under rcu_read_lock().
> > A * Taking cgroup_mutex()/hierarchy_mutex() is not necessary for following calls.
> > @@ -598,10 +600,10 @@ static inline int cgroup_attach_task_cur
> > A * destroyed". The caller should check css and cgroup's status.
> > A */
> >
> > -/*
> > - * Typically Called at ->destroy(), or somewhere the subsys frees
> > - * cgroup_subsys_state.
> > - */
> > +/* Should be called in ->create() by subsys itself */
> > +int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *newgr,
> > + A  A  A  A  A  A  A  struct cgroup_subsys_state *css);
> > +/* Typically Called at ->destroy(), or somewhere the subsys frees css */
> s/Called/called/

will fix.


> > A void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css);
> >
> > A /* Find a cgroup_subsys_state which has given ID */
> > Index: mmotm-0827/mm/memcontrol.c
> > ===================================================================
> > --- mmotm-0827.orig/mm/memcontrol.c
> > +++ mmotm-0827/mm/memcontrol.c
> > @@ -4141,6 +4141,11 @@ mem_cgroup_create(struct cgroup_subsys *
> > A  A  A  A  A  A  A  A if (alloc_mem_cgroup_per_zone_info(mem, node))
> > A  A  A  A  A  A  A  A  A  A  A  A goto free_out;
> >
> > + A  A  A  error = alloc_css_id(ss, cont, &mem->css);
> > + A  A  A  if (error)
> > + A  A  A  A  A  A  A  goto free_out;
> > + A  A  A  /* Here, css_id(&mem->css) works. but css_lookup(id)->mem doesn't */
> > +
> > A  A  A  A /* root ? */
> > A  A  A  A if (cont->parent == NULL) {
> > A  A  A  A  A  A  A  A int cpu;
> > Index: mmotm-0827/block/blk-cgroup.c
> > ===================================================================
> > --- mmotm-0827.orig/block/blk-cgroup.c
> > +++ mmotm-0827/block/blk-cgroup.c
> > @@ -958,9 +958,13 @@ blkiocg_create(struct cgroup_subsys *sub
> > A {
> > A  A  A  A struct blkio_cgroup *blkcg;
> > A  A  A  A struct cgroup *parent = cgroup->parent;
> > + A  A  A  int ret;
> >
> > A  A  A  A if (!parent) {
> > A  A  A  A  A  A  A  A blkcg = &blkio_root_cgroup;
> > + A  A  A  A  A  A  A  ret = alloc_css_id(subsys, cgroup, &blkcg->css);
> > + A  A  A  A  A  A  A  if (ret)
> > + A  A  A  A  A  A  A  A  A  A  A  return ERR_PTR(ret);
> > A  A  A  A  A  A  A  A goto done;
> > A  A  A  A }
> >
> > @@ -971,6 +975,11 @@ blkiocg_create(struct cgroup_subsys *sub
> > A  A  A  A blkcg = kzalloc(sizeof(*blkcg), GFP_KERNEL);
> > A  A  A  A if (!blkcg)
> > A  A  A  A  A  A  A  A return ERR_PTR(-ENOMEM);
> > + A  A  A  ret = alloc_css_id(subsys, cgroup, &blkcg->css);
> > + A  A  A  if (ret) {
> > + A  A  A  A  A  A  A  kfree(blkcg);
> > + A  A  A  A  A  A  A  return ERR_PTR(ret);
> > + A  A  A  }
> >
> > A  A  A  A blkcg->weight = BLKIO_WEIGHT_DEFAULT;
> > A done:
> > Index: mmotm-0827/Documentation/cgroups/cgroups.txt
> > ===================================================================
> > --- mmotm-0827.orig/Documentation/cgroups/cgroups.txt
> > +++ mmotm-0827/Documentation/cgroups/cgroups.txt
> > @@ -621,6 +621,54 @@ and root cgroup. Currently this will onl
> > A the default hierarchy (which never has sub-cgroups) and a hierarchy
> > A that is being created/destroyed (and hence has no sub-cgroups).
> >
> > +3.4 cgroup subsys state IDs.
> > +------------
> > +When subsystem sets use_id == true, an ID per [cgroup, subsys] is added
> > +and it will be tied to cgroup_subsys_state object.
> > +
> > +When use_id==true can use following interfaces. But please note that
> > +allocation/free an ID is subsystem's job because cgroup_subsys_state
> > +object's lifetime is subsystem's matter.
> > +
> > +unsigned short css_id(struct cgroup_subsys_state *css)
> > +
> > +Returns ID of cgroup_subsys_state
> Please add trailing '.' (period character).
> 

will fix.

> > +
> > +unsigend short css_depth(struct cgroup_subsys_state *css)
> Typo: s/unsigend/unsigned/
> > +
> > +Returns the level which "css" is exisiting under hierarchy tree.
> > +The root cgroup's depth 0, its children are 1, children's children are
> > +2....
> > +
> > +int alloc_css_id(struct struct cgroup_subsys *ss, struct cgroup *newgr,
> > + A  A  A  A  A  A  A  A struct cgroup_subsys_state *css);
> > +
> > +Attach an new ID to given css under subsystem ([ss, cgroup])
> > +should be called in ->create() callback.
> > +
> > +void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css);
> > +
> > +Free ID attached to "css" under subsystem. Should be called before
> > +"css" is freed.
> > +
> > +struct cgroup_subsys_state *css_lookup(struct cgroup_subsys *ss, int id);
> > +
> > +Look up cgroup_subsys_state via ID. Should be called under rcu_read_lock().
> > +
> > +struct cgroup_subsys_state *css_get_next(struct cgroup_subsys *ss, int id,
> > + A  A  A  A  A  A  A  A struct cgroup_subsys_state *root, int *foundid);
> > +
> > +Returns ID which is under "root" i.e. under sub-directory of "root"
> > +cgroup's directory at considering cgroup hierarchy. The order of IDs
> > +returned by this function is not sorted. Please be careful.
> > +
> > +bool css_is_ancestor(struct cgroup_subsys_state *cg,
> > + A  A  A  A  A  A  A  A  A  A  const struct cgroup_subsys_state *root);
> 
> To match code: s/cg/child/
> 
will fix.

> > +
> > +Returns true if "root" and "cs" is under the same hierarchy and
> > +"root" can be found when you see all ->parent from "cs" until
> This may be more clear: s/see all/walk all/
> 
> > +the root cgroup.
> As above: s/cs/child/
> 

will fix.

I'll reorder patches and post file-stat ones 1st.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>