On Fri 23-11-12 14:37:05, Glauber Costa wrote:

On 11/23/2012 02:33 PM, Michal Hocko wrote:

quoted

On Fri 23-11-12 13:33:36, Glauber Costa wrote:

quoted

On 11/23/2012 01:20 PM, Michal Hocko wrote:

quoted

On Thu 22-11-12 14:29:50, Glauber Costa wrote:
[...]

quoted

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 05b87aa..46f7cfb 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c

[...]

quoted

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

This sounds to generic and it doesn't say what kind of debugging you
get. CONFIG_MEMCG_ASYNC_DESTROY_DEBUG on the other hand can be pretty
explicit that it is aimed at debugging memory leaks caused by async
memcg destruction.

quoted

Ohh, you are right you are using kmem_cache name for those. Sorry for
the confusion
 

quoted

quoted

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.

Deal

quoted

quoted

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

So please make it configurable, off by default and be explicit about its
usefulness.

Thanks
-- 
Michal Hocko
SUSE Labs

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