[PATCH 12/12] memcg: fix race at charging swap-in

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: KAMEZAWA Hiroyuki
Date: Wednesday, September 24, 2008 - 11:36 pm

There is a small race in do_swap_page(). When the page swapped-in is charged,
the mapcount can be greater than 0. But, at the same time some process (shares
it ) call unmap and make mapcount 1->0 and the page is uncharged.

For fixing this, I added a new interface.
  - precharge
   account to res_counter by PAGE_SIZE and try to free pages if necessary.
  - commit	
   register page_cgroup and add to LRU if necessary.
  - cancel
   uncharge PAGE_SIZE because of do_swap_page failure.

This protocol uses PCG_USED bit on page_cgroup for avoiding over accounting.
Usual mem_cgroup_charge_common() does precharge -> commit at a time.

These precharge/commit/cancel is useful and can be used for other places,
 - shmem, (and other places need precharge.)
 - migration
 - move_account(force_empty) etc...
etc..we'll revisit later.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

 include/linux/memcontrol.h |   21 +++++++
 mm/memcontrol.c            |  135 +++++++++++++++++++++++++++++++--------------
 mm/memory.c                |    6 +-
 3 files changed, 120 insertions(+), 42 deletions(-)

Index: mmotm-2.6.27-rc7+/include/linux/memcontrol.h
===================================================================
--- mmotm-2.6.27-rc7+.orig/include/linux/memcontrol.h
+++ mmotm-2.6.27-rc7+/include/linux/memcontrol.h
@@ -31,6 +31,13 @@ struct mm_struct;
 
 extern int mem_cgroup_charge(struct page *page, struct mm_struct *mm,
 				gfp_t gfp_mask);
+/* for swap handling */
+extern int mem_cgroup_precharge(struct mm_struct *mm,
+		gfp_t gfp_mask, struct mem_cgroup **ptr);
+extern void mem_cgroup_commit_charge_swap(struct page *page,
+					struct mem_cgroup *ptr);
+extern void mem_cgroup_cancel_charge_swap(struct mem_cgroup *ptr);
+
 extern int mem_cgroup_cache_charge(struct page *page, struct mm_struct *mm,
 					gfp_t gfp_mask);
 extern void mem_cgroup_move_lists(struct page *page, enum lru_list lru);
@@ -85,6 +92,20 @@ static inline int mem_cgroup_cache_charg
 	return 0;
 }
 
+static int mem_cgroup_precharge(struct mm_struct *mm,
+				gfp_t gfp_mask, struct mem_cgroup **ptr)
+{
+	return 0;
+}
+
+static void mem_cgroup_commit_charge_swap(struct page *page,
+					  struct mem_cgroup *ptr)
+{
+}
+static void mem_cgroup_cancel_charge_swap(struct mem_cgroup *ptr)
+{
+}
+
 static inline void mem_cgroup_uncharge_page(struct page *page)
 {
 }
Index: mmotm-2.6.27-rc7+/mm/memcontrol.c
===================================================================
--- mmotm-2.6.27-rc7+.orig/mm/memcontrol.c
+++ mmotm-2.6.27-rc7+/mm/memcontrol.c
@@ -698,52 +698,44 @@ static void drain_page_cgroup_all(void)
 
 
 /*
- * Charge the memory controller for page usage.
- * Return
- * 0 if the charge was successful
- * < 0 if the cgroup is over its limit
+ * charge against mem_cgroup linked to this mm. (or *ptr)
+ *
+ * This just charge PAGE_SIZE and reduce memory usage if necessary.
+ *
+ * Pages on radix-tree is charged at radix-tree add/remove under lock.
+ * new pages are charged at allocation and both are guaranteed to be that
+ * there are no racy users. We does precharge->commit at once.
+ *
+ * About swapcache, we can't trust page->mapcount until it's mapped.
+ * Then we do precharge before map and commit/cancel after the mapping is
+ * established. (see below, we have commit_swap and cancel_swap)
  */
-static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm,
-				gfp_t gfp_mask, enum charge_type ctype,
-				struct mem_cgroup *memcg)
+
+int mem_cgroup_precharge(struct mm_struct *mm,
+			 gfp_t mask, struct mem_cgroup **ptr)
 {
 	struct mem_cgroup *mem;
-	struct page_cgroup *pc;
-	unsigned long nr_retries = MEM_CGROUP_RECLAIM_RETRIES;
-	struct mem_cgroup_per_zone *mz;
-	unsigned long flags;
-
-	pc = lookup_page_cgroup(page);
-	/* can happen at boot */
-	if (unlikely(!pc))
-		return 0;
-	prefetchw(pc);
-	/*
-	 * We always charge the cgroup the mm_struct belongs to.
-	 * The mm_struct's mem_cgroup changes on task migration if the
-	 * thread group leader migrates. It's possible that mm is not
-	 * set, if so charge the init_mm (happens for pagecache usage).
-	 */
+	int nr_retries = MEM_CGROUP_RECLAIM_RETRIES;
 
-	if (likely(!memcg)) {
-		rcu_read_lock();
+	rcu_read_lock();
+	if (!*ptr) {
 		mem = mem_cgroup_from_task(rcu_dereference(mm->owner));
 		if (unlikely(!mem)) {
 			rcu_read_unlock();
-			return 0;
+			return -ESRCH;
 		}
-		rcu_read_unlock();
+		*ptr = mem;
 	} else {
-		mem = memcg;
+		mem = *ptr;
 	}
+	rcu_read_unlock();
 
+	css_get(&mem->css);
 	while (unlikely(res_counter_charge(&mem->res, PAGE_SIZE))) {
-		if (!(gfp_mask & __GFP_WAIT))
-			goto out;
-
-		if (try_to_free_mem_cgroup_pages(mem, gfp_mask))
+		if (!(mask & __GFP_WAIT))
+			goto nomem;
+		if (try_to_free_mem_cgroup_pages(mem, mask))
 			continue;
-
 		/*
 		 * try_to_free_mem_cgroup_pages() might not give us a full
 		 * picture of reclaim. Some pages are reclaimed and might be
@@ -755,16 +747,31 @@ static int mem_cgroup_charge_common(stru
 			continue;
 
 		if (!nr_retries--) {
-			mem_cgroup_out_of_memory(mem, gfp_mask);
-			goto out;
+			mem_cgroup_out_of_memory(mem, mask);
+			goto nomem;
 		}
 	}
+	return 0;
+nomem:
+	css_put(&mem->css);
+	return -ENOMEM;
+}
 
+void mem_cgroup_commit_charge(struct page_cgroup *pc,
+			      struct mem_cgroup *mem,
+			      enum charge_type ctype)
+{
+	struct mem_cgroup_per_zone *mz;
+	unsigned long flags;
+
+	if (!mem)
+		return;
 	preempt_disable();
 	if (TestSetPageCgroupUsed(pc)) {
 		res_counter_uncharge(&mem->res, PAGE_SIZE);
+		css_put(&mem->css);
 		preempt_enable();
-		goto done;
+		return;
 	}
 	/*
  	 *  page cgroup is *unused* now....but....
@@ -786,14 +793,43 @@ static int mem_cgroup_charge_common(stru
 
 	pc->mem_cgroup = mem;
 	set_page_cgroup_lru(pc);
+	css_put(&mem->css);
 	preempt_enable();
+}
 
-done:
+/*
+ * Charge the memory controller for page usage.
+ * Return
+ * 0 if the charge was successful
+ * < 0 if the cgroup is over its limit
+ */
+static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm,
+				gfp_t gfp_mask, enum charge_type ctype,
+				struct mem_cgroup *memcg)
+{
+	struct page_cgroup *pc;
+	struct mem_cgroup *ptr = memcg;
+	int ret;
+
+	pc = lookup_page_cgroup(page);
+	/* can happen at boot */
+	if (unlikely(!pc))
+		return 0;
+	prefetchw(pc);
+
+	ret = mem_cgroup_precharge(mm, gfp_mask, &ptr);
+	if (likely(!ret)) {
+		mem_cgroup_commit_charge(pc, ptr, ctype);
+		return 0;
+	}
+	if (unlikely((ret == -ENOMEM)))
+		return ret;
+	/* ESRCH case */
 	return 0;
-out:
-	return -ENOMEM;
 }
 
+
+
 int mem_cgroup_charge(struct page *page, struct mm_struct *mm, gfp_t gfp_mask)
 {
 	if (mem_cgroup_subsys.disabled)
@@ -806,7 +842,7 @@ int mem_cgroup_charge(struct page *page,
 	 * But page->mapping may have out-of-use anon_vma pointer,
 	 * detecit it by PageAnon() check. newly-mapped-anon's page->mapping
 	 * is NULL.
-  	 */
+	 */
 	if (page_mapped(page) || (page->mapping && !PageAnon(page)))
 		return 0;
 	if (unlikely(!mm))
@@ -857,6 +893,25 @@ int mem_cgroup_cache_charge(struct page 
 				MEM_CGROUP_CHARGE_TYPE_SHMEM, NULL);
 }
 
+
+void mem_cgroup_commit_charge_swap(struct page *page, struct mem_cgroup *ptr)
+{
+	struct page_cgroup *pc;
+	if (!ptr)
+		return;
+	pc = lookup_page_cgroup(page);
+	mem_cgroup_commit_charge(pc, ptr, MEM_CGROUP_CHARGE_TYPE_MAPPED);
+}
+
+void mem_cgroup_cancel_charge_swap(struct mem_cgroup *mem)
+{
+	if (!mem)
+		return;
+	res_counter_uncharge(&mem->res, PAGE_SIZE);
+	css_put(&mem->css);
+}
+
+
 /*
  * uncharge if !page_mapped(page)
  */
Index: mmotm-2.6.27-rc7+/mm/memory.c
===================================================================
--- mmotm-2.6.27-rc7+.orig/mm/memory.c
+++ mmotm-2.6.27-rc7+/mm/memory.c
@@ -2287,6 +2287,7 @@ static int do_swap_page(struct mm_struct
 	struct page *page;
 	swp_entry_t entry;
 	pte_t pte;
+	struct mem_cgroup *ptr = NULL;
 	int ret = 0;
 
 	if (!pte_unmap_same(mm, pmd, page_table, orig_pte))
@@ -2323,7 +2324,7 @@ static int do_swap_page(struct mm_struct
 	lock_page(page);
 	delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
 
-	if (mem_cgroup_charge(page, mm, GFP_KERNEL)) {
+	if (mem_cgroup_precharge(mm, GFP_KERNEL, &ptr) == -ENOMEM) {
 		ret = VM_FAULT_OOM;
 		goto out;
 	}
@@ -2353,6 +2354,7 @@ static int do_swap_page(struct mm_struct
 	flush_icache_page(vma, page);
 	set_pte_at(mm, address, page_table, pte);
 	page_add_anon_rmap(page, vma, address);
+	mem_cgroup_commit_charge_swap(page, ptr);
 
 	swap_free(entry);
 	if (vm_swap_full() || (vma->vm_flags & VM_LOCKED) || PageMlocked(page))
@@ -2373,7 +2375,7 @@ unlock:
 out:
 	return ret;
 out_nomap:
-	mem_cgroup_uncharge_page(page);
+	mem_cgroup_cancel_charge_swap(ptr);
 	pte_unmap_unlock(page_table, ptl);
 	unlock_page(page);
 	page_cache_release(page);

--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:11 pm)
[PATCH 1/12] memcg avoid accounting special mappings not o ..., KAMEZAWA Hiroyuki, (Wed Sep 24, 11:13 pm)
[PATCH 2/12] memcg move charege() call to swapped-in page ..., KAMEZAWA Hiroyuki, (Wed Sep 24, 11:14 pm)
[PATCH 3/12] memcg make root cgroup unlimited., KAMEZAWA Hiroyuki, (Wed Sep 24, 11:15 pm)
[PATCH 4/12] memcg make page-&gt;mapping NULL before calling ..., KAMEZAWA Hiroyuki, (Wed Sep 24, 11:16 pm)
[PATCH 5/12] memcg make page_cgroup-&gt;flags atomic, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:17 pm)
[PATCH 6/12] memcg optimize percpu stat, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:18 pm)
[PATCH 7/12] memcg add function to move account, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:27 pm)
[PATCH 8/12] memcg rewrite force empty to move account to root, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:29 pm)
[PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:32 pm)
[PATCH 10/12] memcg free page_cgroup from LRU in lazy, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:33 pm)
[PATCH 11/12] memcg add to LRU in lazy, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:35 pm)
[PATCH 12/12] memcg: fix race at charging swap-in, KAMEZAWA Hiroyuki, (Wed Sep 24, 11:36 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, Dave Hansen, (Thu Sep 25, 11:40 am)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, Daisuke Nishimura, (Thu Sep 25, 6:00 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Thu Sep 25, 6:17 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Thu Sep 25, 6:22 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Thu Sep 25, 6:43 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Thu Sep 25, 7:05 pm)
[PATCH(fixed) 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Thu Sep 25, 7:21 pm)
[PATCH(fixed) 10/12] free page cgroup from LRU in lazy, KAMEZAWA Hiroyuki, (Thu Sep 25, 7:25 pm)
[PATCH(fixed) 11/12] free page cgroup from LRU in add, KAMEZAWA Hiroyuki, (Thu Sep 25, 7:28 pm)
Re: [PATCH 0/12] memcg updates v5, Daisuke Nishimura, (Thu Sep 25, 7:32 pm)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Thu Sep 25, 7:58 pm)
Re: [PATCH 0/12] memcg updates v5, Daisuke Nishimura, (Thu Sep 25, 8:00 pm)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Thu Sep 25, 8:04 pm)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Thu Sep 25, 9:05 pm)
Re: [PATCH 0/12] memcg updates v5, Daisuke Nishimura, (Thu Sep 25, 10:24 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, Daisuke Nishimura, (Thu Sep 25, 10:54 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Thu Sep 25, 11:54 pm)
Re: [PATCH 7/12] memcg add function to move account, Daisuke Nishimura, (Fri Sep 26, 12:30 am)
Re: [PATCH 0/12] memcg updates v5, Balbir Singh, (Fri Sep 26, 1:18 am)
Re: [PATCH 3/12] memcg make root cgroup unlimited., Balbir Singh, (Fri Sep 26, 1:41 am)
Re: [PATCH 1/12] memcg avoid accounting special mappings n ..., KAMEZAWA Hiroyuki, (Fri Sep 26, 2:17 am)
Re: [PATCH 2/12] memcg move charege() call to swapped-in p ..., KAMEZAWA Hiroyuki, (Fri Sep 26, 2:18 am)
Re: [PATCH 3/12] memcg make root cgroup unlimited., KAMEZAWA Hiroyuki, (Fri Sep 26, 2:21 am)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Fri Sep 26, 2:22 am)
Re: [PATCH 7/12] memcg add function to move account, KAMEZAWA Hiroyuki, (Fri Sep 26, 2:24 am)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Fri Sep 26, 2:28 am)
Re: [PATCH 3/12] memcg make root cgroup unlimited., Balbir Singh, (Fri Sep 26, 2:29 am)
Re: [PATCH 0/12] memcg updates v5, Balbir Singh, (Fri Sep 26, 2:31 am)
Re: [PATCH 6/12] memcg optimize percpu stat, Balbir Singh, (Fri Sep 26, 2:53 am)
Re: [PATCH 1/12] memcg avoid accounting special mappings n ..., KAMEZAWA Hiroyuki, (Fri Sep 26, 2:55 am)
Re: [PATCH 3/12] memcg make root cgroup unlimited., KAMEZAWA Hiroyuki, (Fri Sep 26, 2:59 am)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Fri Sep 26, 3:36 am)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Fri Sep 26, 3:43 am)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Fri Sep 26, 7:53 pm)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Fri Sep 26, 8:19 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Fri Sep 26, 8:25 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Fri Sep 26, 8:47 pm)
Re: [PATCH 5/12] memcg make page_cgroup-&gt;flags atomic, Balbir Singh, (Fri Sep 26, 11:58 pm)
Re: [PATCH 7/12] memcg add function to move account, Balbir Singh, (Sat Sep 27, 12:56 am)
Re: Re: [PATCH 7/12] memcg add function to move account, kamezawa.hiroyu, (Sat Sep 27, 1:35 am)
Re: [PATCH 0/12] memcg updates v5, Balbir Singh, (Sun Sep 28, 8:02 pm)
Re: [PATCH 0/12] memcg updates v5, KAMEZAWA Hiroyuki, (Sun Sep 28, 8:27 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, Balbir Singh, (Tue Sep 30, 9:03 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Tue Sep 30, 10:07 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, Balbir Singh, (Tue Sep 30, 10:30 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Tue Sep 30, 10:32 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Tue Sep 30, 10:41 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, Balbir Singh, (Tue Sep 30, 10:59 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Tue Sep 30, 11:12 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, KAMEZAWA Hiroyuki, (Tue Sep 30, 11:17 pm)
Re: [PATCH 9/12] memcg allocate all page_cgroup at boot, Balbir Singh, (Tue Sep 30, 11:26 pm)