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

[Daisuke Nishimura <d-nishimura@mtf.biglobe.ne.jp>]

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 ? :)
I think this line should be at the top of this function, and we should do like:

                case CHARGE_RETRY: /* not in OOM situation but retry */
			nr_oom_retries = MEM_CGROUP_RECLAIM_RETRIES;
			csize = PAGE_SIZE;
			break;

later.

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


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>