Re: [RFC][1/3] memcg clean up try charge

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

On Tue, 1 Jun 2010 23:19:14 +0900
Daisuke Nishimura <d-nishimura@mtf.biglobe.ne.jp> wrote:

> On Tue, 1 Jun 2010 18:24:06 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > 
> > mem_cgroup_try_charge() has a big loop (doesn't fits in screee) and seems to be
> > hard to read. Most of routines are for slow paths. This patch moves codes out
> > from the loop and make it clear what's done.
> > 
> I like this cleanup :)
> 
> I have some comments for now.
> 
> > -	while (1) {
> > -		int ret = 0;
> > -		unsigned long flags = 0;
> > +	while (ret != CHARGE_OK) {
> > +		int nr_oom_retries = MEM_CGROUP_RECLAIM_RETRIES;
> reset nr_oom_retries at the beginning of every loop ? :)

This initialization is done only once ;) 

> I think this line should be at the top of this function, and we should do like:
> 
But ok, will do that.


>                 case CHARGE_RETRY: /* not in OOM situation but retry */
> 			nr_oom_retries = MEM_CGROUP_RECLAIM_RETRIES;
> 			csize = PAGE_SIZE;
> 			break;
> 
> later.
> 
Hmmmmmmm. ok.


> > +		case CHARGE_NOMEM: /* OOM routine works */
> >  			if (!oom)
> >  				goto nomem;
> > -			if (mem_cgroup_handle_oom(mem_over_limit, gfp_mask)) {
> > -				nr_retries = MEM_CGROUP_RECLAIM_RETRIES;
> > -				continue;
> > -			}
> > -			/* When we reach here, current task is dying .*/
> > -			css_put(&mem->css);
> > +			/* If !oom, we never return -ENOMEM */
> s/!oom/oom ?   
> 

yes.

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>