On 11/23/2012 02:33 PM, Michal Hocko wrote:
> On Fri 23-11-12 13:33:36, Glauber Costa wrote:
>> On 11/23/2012 01:20 PM, Michal Hocko wrote:
>>> On Thu 22-11-12 14:29:50, Glauber Costa wrote:
>>> [...]
>>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>>> index 05b87aa..46f7cfb 100644
>>>> --- a/mm/memcontrol.c
>>>> +++ b/mm/memcontrol.c
>>> [...]
>>>> @@ -349,6 +366,33 @@ struct mem_cgroup {
>>>> #endif
>>>> };
>>>>
>>>> +#if defined(CONFIG_MEMCG_KMEM) || defined(CONFIG_MEMCG_SWAP)
>>>
>>> Can we have a common config for this something like CONFIG_MEMCG_ASYNC_DESTROY
>>> which would be selected if either of the two (or potentially others)
>>> would be selected.
>>> Also you are saying that the feature is only for debugging purposes so
>>> it shouldn't be on by default probably.
>>>
>>
>> I personally wouldn't mind. But the big value I see from it is basically
>> being able to turn it off. For all the rest, we would have to wrap it
>> under one of those config options anyway...
>
> Sure you would need to habe mem_cgroup_dangling_FOO wrapped by the
> correct one anyway but that still need to live inside a bigger ifdef and
> naming all the FOO is awkward. Besides that one
> CONFIG_MEMCG_ASYNC_DESTROY_DEBUG could have a Kconfig entry and so be
> enabled separately.
>
How about a more general memcg debug option like CONFIG_MEMCG_DEBUG?
Do you foresee more facilities we could enable under this?
> Ohh, you are right you are using kmem_cache name for those. Sorry for
> the confusion
>
>>> And finally it would be really nice if you described what is the
>>> exported information good for. Can I somehow change the current state
>>> (e.g. force freeing those objects so that the memcg can finally pass out
>>> in piece)?
>>>
>> I am open, but I would personally like to have this as a view-only
>> interface,
>
> And I was not proposing to make it RW. I am just missing a description
> that would explain: "Ohh well, the file says there are some dangling
> memcgs. Should I report a bug or sue somebody or just have a coffee and
> wait some more?"
>
People should pay me beer if the number of pending caches is odd, and
pay you beer if the number is even. If the number is 0, we both get it.
>> just so we suspect a leak occurs, we can easily see what is
>> the dead memcg contribution to it. It shows you where the data come
>> from, and if you want to free it, you go search for subsystem-specific
>> ways to force a free should you want.
>
> Yes, I can imagine its usefulness for developers but I do not see much
> of an use for admins yet. So I am a bit hesitant for this being on by
> default.
>
Fully agreed. I am implementing this because Kame suggested. I promptly
agreed because I remembered how many times I asked myself "Who is
holding this?" and had to go put some printks all over...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>