Re: [PATCH] memcg: use ID in page_cgroup

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

On Tue, 24 Aug 2010 01:35:37 -0700
Greg Thelen <gthelen@google.com> wrote:

> > rcu_read_lock() is just for delaying to discard object, not for avoiding
> > racy updates. All _updates_ requires proper lock or speculative logic as
> > atomic_inc_not_zero() etc... Basically, RCU is for avoiding use-after-free.
> 
> 
> Thanks for the info.  Referring to your original patch:
> > @@ -2014,11 +2025,11 @@ static int mem_cgroup_move_account(struc
> >  {
> >  	int ret = -EINVAL;
> >  	lock_page_cgroup(pc);
> > -	if (PageCgroupUsed(pc) && pc->mem_cgroup == from) {
> > +	if (PageCgroupUsed(pc) && id_to_memcg(pc->mem_cgroup, true) == from) {
> >  		__mem_cgroup_move_account(pc, from, to, uncharge);
> >  		ret = 0;
> >  	}
> > -	unlock_page_cgroup(pc);
> > +	rcu_read_unlock();
> > 
> 
> It seems like mem_cgroup_move_account() is not balanced.  Why is
> lock_page_cgroup(pc) used to lock but rcu_read_unlock() used to unlock?
> 

Nice catch. It's bug. It seems my eyes were corrupted..
Will be fixed in the next version. Sorry for bad code.

Thanks,
-Kame