Re: [PATCH 6/9] memcg: use css_tryget()

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Thu, 18 Dec 2008 10:50:27 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Tue, 16 Dec 2008 18:17:39 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > 
> > From:KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Use css_tryget() in memcg.
> > 
> > css_tryget() newly is added and we can know css is alive or not and
> > get refcnt of css in very safe way.
> > ("alive" here means "rmdir/destroy" is not called.)
> > 
> > This patch replaces css_get() to css_tryget(), where I cannot explain
> > why css_get() is safe. And removes memcg->obsolete flag.
> > 
> > Changelog (v0) -> (v1):
> >   - fixed css_ref leak bug at swap-in.
> > 
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> (snip)
> 
> > +/*
> > + * While swap-in, try_charge -> commit or cancel, the page is locked.
> > + * And when try_charge() successfully returns, one refcnt to memcg without
> > + * struct page_cgroup is aquired. This refcnt will be cumsumed by
> > + * "commit()" or removed by "cancel()"
> > + */
> >  int mem_cgroup_try_charge_swapin(struct mm_struct *mm,
> >  				 struct page *page,
> >  				 gfp_t mask, struct mem_cgroup **ptr)
> >  {
> >  	struct mem_cgroup *mem;
> >  	swp_entry_t     ent;
> > +	int ret;
> >  
> >  	if (mem_cgroup_disabled())
> >  		return 0;
> > @@ -1089,10 +1115,15 @@ int mem_cgroup_try_charge_swapin(struct 
> >  	ent.val = page_private(page);
> >  
> >  	mem = lookup_swap_cgroup(ent);
> > -	if (!mem || mem->obsolete)
> > +	if (!mem)
> > +		goto charge_cur_mm;
> > +	if (!css_tryget(&mem->css))
> >  		goto charge_cur_mm;
> I haven't noticed the bug here which existed in RFC version.
> 
> Actually, I found a problem at rmdir(cannot remove directry because of refcnt leak)
> in testing RFC version, and have been digging it.
> I've confirmed it is fixed in this version.
> 
> This version looks good to me and I think this patch is definitely needed
> to remove buggy "obsolete" flag.
> 
> 	Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> 
ya, thanks again :)

-Kame

> 
> Thanks,
> Daisuke Nishimura.
> 

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