On Tue, 1 Jun 2010 23:19:14 +0900
Daisuke Nishimura <d-nishimura@xxxxxxxxxxxxxxxxx> wrote:
> On Tue, 1 Jun 2010 18:24:06 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote:
>
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>