Re: [PATCH V2] memcg: add mlock statistic in memory.stat

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

(2012/04/20 15:39), Ying Han wrote:

> On Thu, Apr 19, 2012 at 11:16 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> (2012/04/20 14:57), Ying Han wrote:
>>
>>> On Thu, Apr 19, 2012 at 5:37 PM, KAMEZAWA Hiroyuki
>>> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>>> (2012/04/19 22:12), Johannes Weiner wrote:
>>>>> Plus this code runs for ALL uncharges, the unlikely() and preliminary
>>>>> flag testing don't make it okay.  It's bad that we have this in the
>>>>> allocator, but at least it would be good to hook into that branch and
>>>>> not add another one.
>>>>>
>>>>> pc->mem_cgroup stays intact after the uncharge.  Could we make the
>>>>> memcg removal path wait on the mlock counter to drop to zero instead
>>>>> and otherwise keep Ying's version?
>>>>>
>>>>
>>>>
>>>> handling problem in ->destroy() path ? Hmm, it will work against use-after-free.
>>>
>>>> But accounting problem which may be caused by mem_cgroup_lru_add_list() cannot
>>>> be handled, which overwrites pc->mem_cgroup.
>>>
>>> Kame, can you clarify that? What the mem_cgroup_lru_add_list() has
>>> anything to do w/ this problem?
>>>
>>
>>
>> It overwrites pc->mem_cgroup. Then, Assume a task in cgroup "A".
>>
>>        1. page is charged.       pc->mem_cgroup = A + Used bit.
>>        2. page is set Mlocked.   A's mlock-counter += 1
>>        3. page is uncharged      - Used bit.
>>        4. page is added to lru   pc->mem_cgroup = root
>>        5. page is freed          root's mlock-coutner -=1,
>>
>> Then, A's mlock-counter +1, root's mlock-counter -1 IF free_pages()
>> really handle mlocked pages...
> 
> Hmm, now the question is whether the TestClearPageMlock() should only
> happen between step 2 and step 3. If so, the mlock stat will be
> updated correctly.
> 


Yes, I think it's true. TestClearPageMlock() should happen
betwen 2 and 3, I believe.


>> Sigh...."This shouldn't happen"!!!!!
>>
>> How about adding warning to free_page() path and remove your current hook ?
> 
> That does make thing a lot simpler.. I will wait a bit in case someone
> remember a counter example?
> 


Sure. Thank you for your works.

Thanks,
-Kame

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>