Re: Low overhead patches for the memory cgroup controller (v4)

[Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>]

On Mon, 15 Jun 2009 08:52:56 +0530, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> Daisuke Nishimura wrote:
> > On Mon, 15 Jun 2009 08:30:06 +0530, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> >> Daisuke Nishimura wrote:
> >>
> >>>  	pc->mem_cgroup = mem;
> >>>  	smp_wmb();
> >>> -	pc->flags = pcg_default_flags[ctype];
> >> pc->flags needs to be reset here, otherwise we have the danger the carrying over
> >> older bits. I'll merge your changes and test.
> >>
> > hmm, why ?
> > 
> > I do in my patch:
> > 
> > +	switch (ctype) {
> > +	case MEM_CGROUP_CHARGE_TYPE_CACHE:
> > +	case MEM_CGROUP_CHARGE_TYPE_SHMEM:
> > +		SetPageCgroupCache(pc);
> > +		SetPageCgroupUsed(pc);
> > +		break;
> > +	case MEM_CGROUP_CHARGE_TYPE_MAPPED:
> > +		ClearPageCgroupCache(pc);
> > +		SetPageCgroupUsed(pc);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > 
> 
> Yes, I did that in the older code, what I was suggesting was just an additional
> step to ensure that in the future if we add new flags, we don't end up with a
> long list of initializations and clearing or if we forget to clear pc->flags and
> reuse the page_cgroup, it might be a problem. My message was confusing, it
> should have been resetting the pc->flags will provide protection for any future
> addition of flags.
> 
O.K. I see your point.

But we shouldn't touch PCG_ACCT_LRU flag here. IIUC, that's why we abandon
pcg_default_flags[]. Please take care of it.

> I am testing your patch which is the modified version of v3 with your changes
> and have your signed-off-by in it as well as I post v5. Is that OK?
> 
Sure :)


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>