Re: [PATCH next-20130703] net: sock: Add ifdef CONFIG_MEMCG_KMEM for mem_cgroup_sockets_{init,destroy}

On Wed, Jul 3, 2013 at 5:20 PM, Michal Hocko [off-list ref] wrote:

On Wed 03-07-13 20:51:00, Li Zefan wrote:
[...]

quoted

[PATCH] memcg: fix build error if CONFIG_MEMCG_KMEM=n

Fix this build error:

mm/built-in.o: In function `mem_cgroup_css_free':
memcontrol.c:(.text+0x5caa6): undefined reference to
'mem_cgroup_sockets_destroy'

Reported-by: Fengguang Wu <redacted>
Reported-by: Stephen Rothwell <redacted>
Signed-off-by: Li Zefan <redacted>

I am seeing the same thing I just didn't get to reporting it.
The other approach is not bad as well but I find this tiny better
because mem_cgroup_css_free should care only about a single cleanup
function for whole kmem. If that one needs to do tcp kmem specific
cleanup then it should be done inside kmem_cgroup_css_offline.

As said in my other mail, for me this makes sense as it is a followup.

But, still I don't know why sock.c has is own mem_cgroup_sockets_{init,destroy}.
It's interesting, that include/net/sock.h is included by sock.c, too.
If they are different, why do both function use the same name?

Andrew could you add this as
memcg-use-css_get-put-when-charging-uncharging-kmem-fix.patch, please?

*-fix-2 as there is *-fix already around.

- Sedat -

http://ozlabs.org/~akpm/mmotm/broken-out/memcg-use-css_get-put-when-charging-uncharging-kmem-fix.patch

Acked-by: Michal Hocko <redacted>

Thanks

quoted

---
 mm/memcontrol.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 234f311..59ea6f9 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c

@@ -5876,6 +5876,11 @@ static int memcg_init_kmem(struct mem_cgroup *memcg, struct cgroup_subsys *ss)
      return mem_cgroup_sockets_init(memcg, ss);
 }

+static void memcg_destroy_kmem(struct mem_cgroup *memcg)
+{
+     mem_cgroup_sockets_destroy(memcg);
+}
+
 static void kmem_cgroup_css_offline(struct mem_cgroup *memcg)
 {
      if (!memcg_kmem_is_active(memcg))

@@ -5915,6 +5920,10 @@ static int memcg_init_kmem(struct mem_cgroup *memcg, struct cgroup_subsys *ss)
      return 0;
 }

+static void memcg_destroy_kmem(struct mem_cgroup *memcg)
+{
+}
+
 static void kmem_cgroup_css_offline(struct mem_cgroup *memcg)
 {
 }

@@ -6312,8 +6321,7 @@ static void mem_cgroup_css_free(struct cgroup *cont)
 {
      struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);

-     mem_cgroup_sockets_destroy(memcg);
-
+     memcg_destroy_kmem(memcg);
      __mem_cgroup_free(memcg);
 }

--

1.8.0.2

quoted

---
[ v2: git dislikes lines beginning with hash ('#'). ]

 include/net/sock.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index ea6206c..ad4bf7f 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h

@@ -71,6 +71,7 @@
 struct cgroup;
 struct cgroup_subsys;
 #ifdef CONFIG_NET
+#ifdef CONFIG_MEMCG_KMEM

#if defined(CONFIG_NET) && defined(CONFIG_MEMCG_KMEM)

quoted

 int mem_cgroup_sockets_init(struct mem_cgroup *memcg, struct cgroup_subsys *ss);
 void mem_cgroup_sockets_destroy(struct mem_cgroup *memcg);
 #else

@@ -83,7 +84,8 @@ static inline
 void mem_cgroup_sockets_destroy(struct mem_cgroup *memcg)
 {
 }
-#endif
+#endif /* CONFIG_NET */
+#endif /* CONFIG_MEMCG_KMEM */
 /*
  * This structure really needs to be cleaned up.
  * Most of it is for TCP, and not used by any of

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

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