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

[Balbir Singh <balbir@linux.vnet.ibm.com>]

Daisuke Nishimura wrote:
> 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 keeping the pc->flags removed as in the earlier patch, but something to
keep in mind as we review further changes to the flags field.

>> 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 :)
> 

Just sending it out, now, Thanks!

-- 
	Balbir

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