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