Re: [BUGFIX][PATCH] memcg: fix try_get_mem_cgroup_from_swapcache()

[Balbir Singh <balbir@linux.vnet.ibm.com>]

* 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>