* [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() [not found] <20090426231752.36498c90.d-nishimura@mtf.biglobe.ne.jp> @ 2009-04-27 0:51 ` Daisuke Nishimura 2009-04-27 1:14 ` KAMEZAWA Hiroyuki 2009-04-27 6:53 ` Balbir Singh 0 siblings, 2 replies; 7+ messages in thread From: Daisuke Nishimura @ 2009-04-27 0:51 UTC (permalink / raw) To: Andrew Morton Cc: linux-mm, KAMEZAWA Hiroyuki, Balbir Singh, Daisuke Nishimura From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> memcg: fix try_get_mem_cgroup_from_swapcache() This is a bugfix for commit 3c776e64660028236313f0e54f3a9945764422df(included 2.6.30-rc1). Used bit of swapcache is solid under page lock, but considering move_account, pc->mem_cgroup is not. We need lock_page_cgroup() anyway. Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> --- mm/memcontrol.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index ccc69b4..84f856c 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1024,9 +1024,7 @@ static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page) return NULL; pc = lookup_page_cgroup(page); - /* - * Used bit of swapcache is solid under page lock. - */ + lock_page_cgroup(pc); if (PageCgroupUsed(pc)) { mem = pc->mem_cgroup; if (mem && !css_tryget(&mem->css)) @@ -1040,6 +1038,7 @@ static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page) mem = NULL; rcu_read_unlock(); } + unlock_page_cgroup(pc); return 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> ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() 2009-04-27 0:51 ` [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() Daisuke Nishimura @ 2009-04-27 1:14 ` KAMEZAWA Hiroyuki 2009-04-27 6:53 ` Balbir Singh 1 sibling, 0 replies; 7+ messages in thread From: KAMEZAWA Hiroyuki @ 2009-04-27 1:14 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: Andrew Morton, linux-mm, Balbir Singh On Mon, 27 Apr 2009 09:51:00 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > memcg: fix try_get_mem_cgroup_from_swapcache() > > This is a bugfix for commit 3c776e64660028236313f0e54f3a9945764422df(included 2.6.30-rc1). > Used bit of swapcache is solid under page lock, but considering move_account, > pc->mem_cgroup is not. > > We need lock_page_cgroup() anyway. > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> you are right. Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > --- > mm/memcontrol.c | 5 ++--- > 1 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index ccc69b4..84f856c 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1024,9 +1024,7 @@ static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page) > return NULL; > > pc = lookup_page_cgroup(page); > - /* > - * Used bit of swapcache is solid under page lock. > - */ > + lock_page_cgroup(pc); > if (PageCgroupUsed(pc)) { > mem = pc->mem_cgroup; > if (mem && !css_tryget(&mem->css)) > @@ -1040,6 +1038,7 @@ static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page) > mem = NULL; > rcu_read_unlock(); > } > + unlock_page_cgroup(pc); > return 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() 2009-04-27 0:51 ` [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() Daisuke Nishimura 2009-04-27 1:14 ` KAMEZAWA Hiroyuki @ 2009-04-27 6:53 ` Balbir Singh 2009-04-27 6:59 ` KAMEZAWA Hiroyuki 1 sibling, 1 reply; 7+ messages in thread From: Balbir Singh @ 2009-04-27 6:53 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: Andrew Morton, linux-mm, KAMEZAWA Hiroyuki * nishimura@mxp.nes.nec.co.jp <nishimura@mxp.nes.nec.co.jp> [2009-04-27 09:51:00]: > From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > memcg: fix try_get_mem_cgroup_from_swapcache() > > This is a bugfix for commit 3c776e64660028236313f0e54f3a9945764422df(included 2.6.30-rc1). > Used bit of swapcache is solid under page lock, but considering move_account, > pc->mem_cgroup is not. > > We need lock_page_cgroup() anyway. > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> I think we need to start documenting the locks the page_cgroup lock nests under. If memcg_tasklist were a spinlock instead of mutex, could we use that instead of page_cgroup lock, since we care only about task migration? -- Balbir -- 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() 2009-04-27 6:53 ` Balbir Singh @ 2009-04-27 6:59 ` KAMEZAWA Hiroyuki 2009-04-27 7:03 ` Balbir Singh 0 siblings, 1 reply; 7+ messages in thread From: KAMEZAWA Hiroyuki @ 2009-04-27 6:59 UTC (permalink / raw) To: balbir; +Cc: Daisuke Nishimura, Andrew Morton, linux-mm On Mon, 27 Apr 2009 12:23:58 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > * nishimura@mxp.nes.nec.co.jp <nishimura@mxp.nes.nec.co.jp> [2009-04-27 09:51:00]: > > > From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > > > memcg: fix try_get_mem_cgroup_from_swapcache() > > > > This is a bugfix for commit 3c776e64660028236313f0e54f3a9945764422df(included 2.6.30-rc1). > > Used bit of swapcache is solid under page lock, but considering move_account, > > pc->mem_cgroup is not. > > > > We need lock_page_cgroup() anyway. > > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > I think we need to start documenting the locks the > page_cgroup lock nests under. > Addin some comments on source code may be necessary. > If memcg_tasklist were a spinlock instead of mutex, could we use that > instead of page_cgroup lock, since we care only about task migration? > Hmm ? Another problem ? I can't catch what you ask. move_account() is a function called by force_empty()->"move account to parent" 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() 2009-04-27 6:59 ` KAMEZAWA Hiroyuki @ 2009-04-27 7:03 ` Balbir Singh 2009-04-27 7:08 ` KAMEZAWA Hiroyuki 0 siblings, 1 reply; 7+ messages in thread From: Balbir Singh @ 2009-04-27 7:03 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: Daisuke Nishimura, Andrew Morton, linux-mm * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-27 15:59:53]: > On Mon, 27 Apr 2009 12:23:58 +0530 > Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > * nishimura@mxp.nes.nec.co.jp <nishimura@mxp.nes.nec.co.jp> [2009-04-27 09:51:00]: > > > > > From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > > > > > memcg: fix try_get_mem_cgroup_from_swapcache() > > > > > > This is a bugfix for commit 3c776e64660028236313f0e54f3a9945764422df(included 2.6.30-rc1). > > > Used bit of swapcache is solid under page lock, but considering move_account, > > > pc->mem_cgroup is not. > > > > > > We need lock_page_cgroup() anyway. > > > > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > > > I think we need to start documenting the locks the > > page_cgroup lock nests under. > > > Addin some comments on source code may be necessary. > > > If memcg_tasklist were a spinlock instead of mutex, could we use that > > instead of page_cgroup lock, since we care only about task migration? > > > > Hmm ? Another problem ? I can't catch what you ask. > move_account() is a function called by force_empty()->"move account to parent" IIUC, pc->mem_cgroup might change due to task migration and that is why we've added the page cgroup lock. If the race is between task migration and force_empty(), we can use the memcg_tasklist by making it a spinlock. -- Balbir -- 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() 2009-04-27 7:03 ` Balbir Singh @ 2009-04-27 7:08 ` KAMEZAWA Hiroyuki 2009-04-27 7:17 ` Balbir Singh 0 siblings, 1 reply; 7+ messages in thread From: KAMEZAWA Hiroyuki @ 2009-04-27 7:08 UTC (permalink / raw) To: balbir; +Cc: Daisuke Nishimura, Andrew Morton, linux-mm On Mon, 27 Apr 2009 12:33:42 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-27 15:59:53]: > > > On Mon, 27 Apr 2009 12:23:58 +0530 > > Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > > > * nishimura@mxp.nes.nec.co.jp <nishimura@mxp.nes.nec.co.jp> [2009-04-27 09:51:00]: > > > > > > > From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > > > > > > > memcg: fix try_get_mem_cgroup_from_swapcache() > > > > > > > > This is a bugfix for commit 3c776e64660028236313f0e54f3a9945764422df(included 2.6.30-rc1). > > > > Used bit of swapcache is solid under page lock, but considering move_account, > > > > pc->mem_cgroup is not. > > > > > > > > We need lock_page_cgroup() anyway. > > > > > > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > > > > > I think we need to start documenting the locks the > > > page_cgroup lock nests under. > > > > > Addin some comments on source code may be necessary. > > > > > If memcg_tasklist were a spinlock instead of mutex, could we use that > > > instead of page_cgroup lock, since we care only about task migration? > > > > > > > Hmm ? Another problem ? I can't catch what you ask. > > move_account() is a function called by force_empty()->"move account to parent" > > IIUC, pc->mem_cgroup might change due to task migration and that is > why we've added the page cgroup lock. "task" migration ? I think it's "page" migration. "page cgroup lock" is necessary because we have to modify 2 params (pc->flags and pc->mem_cgroup) at once. > If the race is between task > migration and force_empty(), we can use the memcg_tasklist by making > it a spinlock. > The race is between force_empty() and any other swap accounitng ops. This patch is good enough from my point of view. 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() 2009-04-27 7:08 ` KAMEZAWA Hiroyuki @ 2009-04-27 7:17 ` Balbir Singh 0 siblings, 0 replies; 7+ messages in thread From: Balbir Singh @ 2009-04-27 7:17 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: Daisuke Nishimura, Andrew Morton, linux-mm * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-27 16:08:46]: > On Mon, 27 Apr 2009 12:33:42 +0530 > Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-27 15:59:53]: > > > > > On Mon, 27 Apr 2009 12:23:58 +0530 > > > Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > > > > > * nishimura@mxp.nes.nec.co.jp <nishimura@mxp.nes.nec.co.jp> [2009-04-27 09:51:00]: > > > > > > > > > From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > > > > > > > > > memcg: fix try_get_mem_cgroup_from_swapcache() > > > > > > > > > > This is a bugfix for commit 3c776e64660028236313f0e54f3a9945764422df(included 2.6.30-rc1). > > > > > Used bit of swapcache is solid under page lock, but considering move_account, > > > > > pc->mem_cgroup is not. > > > > > > > > > > We need lock_page_cgroup() anyway. > > > > > > > > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > > > > > > > I think we need to start documenting the locks the > > > > page_cgroup lock nests under. > > > > > > > Addin some comments on source code may be necessary. > > > > > > > If memcg_tasklist were a spinlock instead of mutex, could we use that > > > > instead of page_cgroup lock, since we care only about task migration? > > > > > > > > > > Hmm ? Another problem ? I can't catch what you ask. > > > move_account() is a function called by force_empty()->"move account to parent" > > > > IIUC, pc->mem_cgroup might change due to task migration and that is > > why we've added the page cgroup lock. > > "task" migration ? I think it's "page" migration. > OK > "page cgroup lock" is necessary because we have to modify 2 params > (pc->flags and pc->mem_cgroup) at once. > Yes, definitely! > > If the race is between task > > migration and force_empty(), we can use the memcg_tasklist by making > > it a spinlock. > > > The race is between force_empty() and any other swap accounitng ops. > This patch is good enough from my point of view. Fair enough Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com> -- Balbir -- 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-04-27 7:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20090426231752.36498c90.d-nishimura@mtf.biglobe.ne.jp>
2009-04-27 0:51 ` [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache() Daisuke Nishimura
2009-04-27 1:14 ` KAMEZAWA Hiroyuki
2009-04-27 6:53 ` Balbir Singh
2009-04-27 6:59 ` KAMEZAWA Hiroyuki
2009-04-27 7:03 ` Balbir Singh
2009-04-27 7:08 ` KAMEZAWA Hiroyuki
2009-04-27 7:17 ` Balbir Singh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).