Re: [RFC][PATCH] memcg remove css_get/put per pages v2

[KAMEZAWA Hiroyuki]

On Thu, 10 Jun 2010 11:34:24 +0900
Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx> wrote:

> I can't find any trivial bugs from my review at the moment.
> I'll do some tests.
> 

Thank you for review.

> Some minor commens.
> 
> On Wed, 9 Jun 2010 15:59:40 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote:
> > Still RFC, added lkml to CC: list.
> > ==
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> > 
> > Now, memory cgroup increments css(cgroup subsys state)'s reference
> > count per a charged page. And the reference count is kept until
> > the page is uncharged. But this has 2 bad effect. 
> > 
> >  1. Because css_get/put calls atoimic_inc()/dec, heavy call of them
> >     on large smp will not scale well.
> >  2. Because css's refcnt cannot be in a state as "ready-to-release",
> >     cgroup's notify_on_release handler can't work with memcg.
> > 
> > This is a trial to remove css's refcnt per a page. Even if we remove
> > refcnt, pre_destroy() does enough synchronization.
> > 
> > After this patch, it seems css_get() is still called in try_charge().
> > But the logic is.
> > 
> >   1. task_lock(mm->owner)
> There is no task_lock() in this version :)
> 
yes.


> (snip)
> > @@ -4219,7 +4252,6 @@ static int mem_cgroup_do_precharge(unsig
> >  		mc.precharge += count;
> >  		VM_BUG_ON(test_bit(CSS_ROOT, &mem->css.flags));
> >  		WARN_ON_ONCE(count > INT_MAX);
> > -		__css_get(&mem->css, (int)count);
> >  		return ret;
> >  	}
> >  one_by_one:
> You can remove VM_BUG_ON() and WARN_ON_ONCE() here, too.
> 
ok.


> > @@ -4469,8 +4501,6 @@ static void mem_cgroup_clear_mc(void)
> >  			 */
> >  			res_counter_uncharge(&mc.to->res,
> >  						PAGE_SIZE * mc.moved_swap);
> > -			VM_BUG_ON(test_bit(CSS_ROOT, &mc.to->css.flags));
> > -			__css_put(&mc.to->css, mc.moved_swap);
> >  		}
> >  		/* we've already done mem_cgroup_get(mc.to) */
> >  
> > 
> And, you can remove "WARN_ON_ONCE(mc.moved_swap > INT_MAX)" at the beginning
> of this block, too.
> 
Hmm. ok.
Ah...them, this patch fixes the limitation by "css's refcnt is int" problem.
sounds nice.

Thanks,
-Kmae

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/