From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>, "nishimura@mxp.nes.nec.co.jp" <nishimura@mxp.nes.nec.co.jp>, "balbir@linux.vnet.ibm.com" <balbir@linux.vnet.ibm.com>, "hannes@cmpxchg.org" <hannes@cmpxchg.org>, "akpm@linux-foundation.org" <akpm@linux-foundation.org> Subject: [PATCH 3/3] memcg: fix race at move_parent around compound_order() Date: Tue, 25 Jan 2011 15:05:16 +0900 [thread overview] Message-ID: <20110125150516.fb2f5e06.kamezawa.hiroyu@jp.fujitsu.com> (raw) In-Reply-To: <20110125145720.cd0cbe16.kamezawa.hiroyu@jp.fujitsu.com> Based on 2.6.38-rc2 + mm-memcontrolc-fix-uninitialized-variable-use-in-mem_cgroup_move_parent.patch == From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> A fix up mem_cgroup_move_parent() which use compound_order() in asynchrnous manner. This compound_order() may return unknown value because we don't take lock. Use PageTransHuge() and HPAGE_SIZE instead of it. Also clean up for mem_cgroup_move_parent(). - remove unnecessary initialization of local variable. - rename charge_size -> page_size - remove unnecessary (wrong) comment. - added a comment about THP. Changelog: - fixed page size calculation for avoiding race. Note: Current design take compound_page_lock() in caller of move_account(). This should be revisited when we implement direct move_task of hugepage without splitting. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- mm/memcontrol.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) Index: linux-2.6.38-rc2/mm/memcontrol.c =================================================================== --- linux-2.6.38-rc2.orig/mm/memcontrol.c +++ linux-2.6.38-rc2/mm/memcontrol.c @@ -2234,7 +2234,12 @@ static int mem_cgroup_move_account(struc { int ret = -EINVAL; unsigned long flags; - + /* + * The page is isolated from LRU. So, collapse function + * will not handle this page. But page splitting can happen. + * Do this check under compound_page_lock(). The caller should + * hold it. + */ if ((charge_size > PAGE_SIZE) && !PageTransHuge(pc->page)) return -EBUSY; @@ -2266,7 +2271,7 @@ static int mem_cgroup_move_parent(struct struct cgroup *cg = child->css.cgroup; struct cgroup *pcg = cg->parent; struct mem_cgroup *parent; - int charge = PAGE_SIZE; + int page_size = PAGE_SIZE; unsigned long flags; int ret; @@ -2279,22 +2284,24 @@ static int mem_cgroup_move_parent(struct goto out; if (isolate_lru_page(page)) goto put; - /* The page is isolated from LRU and we have no race with splitting */ - charge = PAGE_SIZE << compound_order(page); + + if (PageTransHuge(page)) + page_size = HPAGE_SIZE; parent = mem_cgroup_from_cont(pcg); - ret = __mem_cgroup_try_charge(NULL, gfp_mask, &parent, false, charge); + ret = __mem_cgroup_try_charge(NULL, gfp_mask, + &parent, false, page_size); if (ret || !parent) goto put_back; - if (charge > PAGE_SIZE) + if (page_size > PAGE_SIZE) flags = compound_lock_irqsave(page); - ret = mem_cgroup_move_account(pc, child, parent, true, charge); + ret = mem_cgroup_move_account(pc, child, parent, true, page_size); if (ret) - mem_cgroup_cancel_charge(parent, charge); + mem_cgroup_cancel_charge(parent, page_size); - if (charge > PAGE_SIZE) + if (page_size > PAGE_SIZE) compound_unlock_irqrestore(page, flags); put_back: putback_lru_page(page);
WARNING: multiple messages have this Message-ID (diff)
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>, "nishimura@mxp.nes.nec.co.jp" <nishimura@mxp.nes.nec.co.jp>, "balbir@linux.vnet.ibm.com" <balbir@linux.vnet.ibm.com>, "hannes@cmpxchg.org" <hannes@cmpxchg.org>, "akpm@linux-foundation.org" <akpm@linux-foundation.org> Subject: [PATCH 3/3] memcg: fix race at move_parent around compound_order() Date: Tue, 25 Jan 2011 15:05:16 +0900 [thread overview] Message-ID: <20110125150516.fb2f5e06.kamezawa.hiroyu@jp.fujitsu.com> (raw) In-Reply-To: <20110125145720.cd0cbe16.kamezawa.hiroyu@jp.fujitsu.com> Based on 2.6.38-rc2 + mm-memcontrolc-fix-uninitialized-variable-use-in-mem_cgroup_move_parent.patch == From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> A fix up mem_cgroup_move_parent() which use compound_order() in asynchrnous manner. This compound_order() may return unknown value because we don't take lock. Use PageTransHuge() and HPAGE_SIZE instead of it. Also clean up for mem_cgroup_move_parent(). - remove unnecessary initialization of local variable. - rename charge_size -> page_size - remove unnecessary (wrong) comment. - added a comment about THP. Changelog: - fixed page size calculation for avoiding race. Note: Current design take compound_page_lock() in caller of move_account(). This should be revisited when we implement direct move_task of hugepage without splitting. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- mm/memcontrol.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) Index: linux-2.6.38-rc2/mm/memcontrol.c =================================================================== --- linux-2.6.38-rc2.orig/mm/memcontrol.c +++ linux-2.6.38-rc2/mm/memcontrol.c @@ -2234,7 +2234,12 @@ static int mem_cgroup_move_account(struc { int ret = -EINVAL; unsigned long flags; - + /* + * The page is isolated from LRU. So, collapse function + * will not handle this page. But page splitting can happen. + * Do this check under compound_page_lock(). The caller should + * hold it. + */ if ((charge_size > PAGE_SIZE) && !PageTransHuge(pc->page)) return -EBUSY; @@ -2266,7 +2271,7 @@ static int mem_cgroup_move_parent(struct struct cgroup *cg = child->css.cgroup; struct cgroup *pcg = cg->parent; struct mem_cgroup *parent; - int charge = PAGE_SIZE; + int page_size = PAGE_SIZE; unsigned long flags; int ret; @@ -2279,22 +2284,24 @@ static int mem_cgroup_move_parent(struct goto out; if (isolate_lru_page(page)) goto put; - /* The page is isolated from LRU and we have no race with splitting */ - charge = PAGE_SIZE << compound_order(page); + + if (PageTransHuge(page)) + page_size = HPAGE_SIZE; parent = mem_cgroup_from_cont(pcg); - ret = __mem_cgroup_try_charge(NULL, gfp_mask, &parent, false, charge); + ret = __mem_cgroup_try_charge(NULL, gfp_mask, + &parent, false, page_size); if (ret || !parent) goto put_back; - if (charge > PAGE_SIZE) + if (page_size > PAGE_SIZE) flags = compound_lock_irqsave(page); - ret = mem_cgroup_move_account(pc, child, parent, true, charge); + ret = mem_cgroup_move_account(pc, child, parent, true, page_size); if (ret) - mem_cgroup_cancel_charge(parent, charge); + mem_cgroup_cancel_charge(parent, page_size); - if (charge > PAGE_SIZE) + if (page_size > PAGE_SIZE) compound_unlock_irqrestore(page, flags); put_back: putback_lru_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/ . Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2011-01-25 6:11 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-01-25 5:57 [PATCH 0/3] 3 bugfixes for memory cgroup (2.6.38-rc2) KAMEZAWA Hiroyuki 2011-01-25 5:57 ` KAMEZAWA Hiroyuki 2011-01-25 6:00 ` [PATCH 1/3] memcg: fix account leak at failure of memsw acconting KAMEZAWA Hiroyuki 2011-01-25 7:07 ` Johannes Weiner 2011-01-25 8:31 ` Daisuke Nishimura 2011-01-25 6:01 ` [PATCH 2/3] memcg: bugfix check mem_cgroup_disabled() at split fixup KAMEZAWA Hiroyuki 2011-01-25 6:01 ` KAMEZAWA Hiroyuki 2011-01-25 6:05 ` KAMEZAWA Hiroyuki [this message] 2011-01-25 6:05 ` [PATCH 3/3] memcg: fix race at move_parent around compound_order() KAMEZAWA Hiroyuki 2011-01-25 7:07 ` Johannes Weiner 2011-01-25 7:07 ` Johannes Weiner 2011-01-25 8:31 ` Daisuke Nishimura 2011-01-25 8:31 ` Daisuke Nishimura
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20110125150516.fb2f5e06.kamezawa.hiroyu@jp.fujitsu.com \ --to=kamezawa.hiroyu@jp.fujitsu.com \ --cc=akpm@linux-foundation.org \ --cc=balbir@linux.vnet.ibm.com \ --cc=hannes@cmpxchg.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=nishimura@mxp.nes.nec.co.jp \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.