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

Subsystems: networking [general], networking [sockets], the rest

STALE4832d

11 messages, 3 authors, 2013-07-03 · open the first message on its own page

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

From: Sedat Dilek <hidden>
Date: 2013-07-03 12:20:10

When "CONFIG_MEMCG_KMEM=n" I see this in my build-log:

 LD      init/built-in.o
mm/built-in.o: In function `mem_cgroup_css_free':
memcontrol.c:(.text+0x5caa6): undefined reference to `mem_cgroup_sockets_destroy'
make[2]: *** [vmlinux] Error 1

Inspired by the ifdef for mem_cgroup_sockets_{init,destroy} here...

[ net/core/sock.c ]

 #ifdef CONFIG_MEMCG_KMEM
 int mem_cgroup_sockets_init()
 ...
 void mem_cgroup_sockets_destroy()
 ...
 #endif

...I did the the same for both in "include/net/sock.h".

This fixes the issue for me in next-20130703.

Signed-off-by: Sedat Dilek <redacted>
---
[ 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
 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
-- 
1.8.3.2

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

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

From: Li Zefan <hidden>
Date: 2013-07-03 12:51:36

On 2013/7/3 20:19, Sedat Dilek wrote:
When "CONFIG_MEMCG_KMEM=n" I see this in my build-log:

 LD      init/built-in.o
mm/built-in.o: In function `mem_cgroup_css_free':
memcontrol.c:(.text+0x5caa6): undefined reference to `mem_cgroup_sockets_destroy'
make[2]: *** [vmlinux] Error 1

Inspired by the ifdef for mem_cgroup_sockets_{init,destroy} here...

[ net/core/sock.c ]

 #ifdef CONFIG_MEMCG_KMEM
 int mem_cgroup_sockets_init()
 ...
 void mem_cgroup_sockets_destroy()
 ...
 #endif

...I did the the same for both in "include/net/sock.h".

This fixes the issue for me in next-20130703.

Signed-off-by: Sedat Dilek <redacted>
Maybe it's better to add memcg_destroy_kmem(), to pair with
memcg_init_kmem().

This patch can be folded into "memcg: use css_get/put when charging/uncharging kmem"

=======================

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


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

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

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

From: Sedat Dilek <hidden>
Date: 2013-07-03 13:09:14

On Wed, Jul 3, 2013 at 2:51 PM, Li Zefan [off-list ref] wrote:
On 2013/7/3 20:19, Sedat Dilek wrote:
quoted
When "CONFIG_MEMCG_KMEM=n" I see this in my build-log:

 LD      init/built-in.o
mm/built-in.o: In function `mem_cgroup_css_free':
memcontrol.c:(.text+0x5caa6): undefined reference to `mem_cgroup_sockets_destroy'
make[2]: *** [vmlinux] Error 1

Inspired by the ifdef for mem_cgroup_sockets_{init,destroy} here...

[ net/core/sock.c ]

 #ifdef CONFIG_MEMCG_KMEM
 int mem_cgroup_sockets_init()
 ...
 void mem_cgroup_sockets_destroy()
 ...
 #endif

...I did the the same for both in "include/net/sock.h".

This fixes the issue for me in next-20130703.

Signed-off-by: Sedat Dilek <redacted>
Maybe it's better to add memcg_destroy_kmem(), to pair with
memcg_init_kmem().

This patch can be folded into "memcg: use css_get/put when charging/uncharging kmem"
Hi Li Zefan,

Can you or a guru from netdev explain me why there exists
mem_cgroup_sockets_init() and mem_cgroup_sockets_destroy() in

1. net/core/sock.c <--- AFAICS this includes below sock.h, too.
2. net/core/sock.h <--- mm/memcontrol.c is including this one.

Make me less confused.

And can you explain why your approach is "better" to do the change in
memcontrol.c than in sock.h (see sock.c).

Thanks in advance.

- Sedat -
quoted hunk
=======================

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

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

From: Sedat Dilek <hidden>
Date: 2013-07-03 13:10:59

On Wed, Jul 3, 2013 at 3:09 PM, Sedat Dilek [off-list ref] wrote:
On Wed, Jul 3, 2013 at 2:51 PM, Li Zefan [off-list ref] wrote:
quoted
On 2013/7/3 20:19, Sedat Dilek wrote:
quoted
When "CONFIG_MEMCG_KMEM=n" I see this in my build-log:

 LD      init/built-in.o
mm/built-in.o: In function `mem_cgroup_css_free':
memcontrol.c:(.text+0x5caa6): undefined reference to `mem_cgroup_sockets_destroy'
make[2]: *** [vmlinux] Error 1

Inspired by the ifdef for mem_cgroup_sockets_{init,destroy} here...

[ net/core/sock.c ]

 #ifdef CONFIG_MEMCG_KMEM
 int mem_cgroup_sockets_init()
 ...
 void mem_cgroup_sockets_destroy()
 ...
 #endif

...I did the the same for both in "include/net/sock.h".

This fixes the issue for me in next-20130703.

Signed-off-by: Sedat Dilek <redacted>
Maybe it's better to add memcg_destroy_kmem(), to pair with
memcg_init_kmem().

This patch can be folded into "memcg: use css_get/put when charging/uncharging kmem"
Hi Li Zefan,

Can you or a guru from netdev explain me why there exists
mem_cgroup_sockets_init() and mem_cgroup_sockets_destroy() in

1. net/core/sock.c <--- AFAICS this includes below sock.h, too.
2. net/core/sock.h <--- mm/memcontrol.c is including this one.
*** include/net/sock.h ***

- Sedat -
Make me less confused.

And can you explain why your approach is "better" to do the change in
memcontrol.c than in sock.h (see sock.c).

Thanks in advance.

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

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

From: Sedat Dilek <hidden>
Date: 2013-07-03 13:20:47

On Wed, Jul 3, 2013 at 2:51 PM, Li Zefan [off-list ref] wrote:
On 2013/7/3 20:19, Sedat Dilek wrote:
quoted
When "CONFIG_MEMCG_KMEM=n" I see this in my build-log:

 LD      init/built-in.o
mm/built-in.o: In function `mem_cgroup_css_free':
memcontrol.c:(.text+0x5caa6): undefined reference to `mem_cgroup_sockets_destroy'
make[2]: *** [vmlinux] Error 1

Inspired by the ifdef for mem_cgroup_sockets_{init,destroy} here...

[ net/core/sock.c ]

 #ifdef CONFIG_MEMCG_KMEM
 int mem_cgroup_sockets_init()
 ...
 void mem_cgroup_sockets_destroy()
 ...
 #endif

...I did the the same for both in "include/net/sock.h".

This fixes the issue for me in next-20130703.

Signed-off-by: Sedat Dilek <redacted>
Maybe it's better to add memcg_destroy_kmem(), to pair with
memcg_init_kmem().

This patch can be folded into "memcg: use css_get/put when charging/uncharging kmem"
That text can be placed into the below changelog as it explains what
the root-cause is.
"memcg: use css_get/put when charging/uncharging kmem" dropped 1st
into Linux-next with next-20130703.

Please, test your changes also with CONFIG_MEMCG_KMEM=n next time.
Thanks!
=======================

[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>
Reported-by: Sedat Dilek <redacted>
Tested-by: Sedat Dilek <redacted>
quoted hunk
Signed-off-by: Li Zefan <redacted>
---
 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>

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

From: Michal Hocko <hidden>
Date: 2013-07-03 15:21:03

On Wed 03-07-13 20:51:00, Li Zefan wrote:
[...]
[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.

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

Acked-by: Michal Hocko <redacted>

Thanks
quoted hunk
---
 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


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

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

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

From: Sedat Dilek <hidden>
Date: 2013-07-03 15:53:25

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>

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

From: Michal Hocko <hidden>
Date: 2013-07-03 16:00:03

On Wed 03-07-13 17:53:21, Sedat Dilek wrote:
On Wed, Jul 3, 2013 at 5:20 PM, Michal Hocko [off-list ref] wrote:
quoted
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}.
That is the only definition AFAICS (except for !CONFIG_NET where it
expands to NOOP). Please note that memcg_init_kmem is a common kmem
initializator and it needs to be prepared for !CONFIG_NET.

The same applies to _destroy.
Makes more sense now?

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

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

From: Sedat Dilek <hidden>
Date: 2013-07-03 16:11:31

On Wed, Jul 3, 2013 at 5:59 PM, Michal Hocko [off-list ref] wrote:
On Wed 03-07-13 17:53:21, Sedat Dilek wrote:
quoted
On Wed, Jul 3, 2013 at 5:20 PM, Michal Hocko [off-list ref] wrote:
quoted
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}.
That is the only definition AFAICS (except for !CONFIG_NET where it
expands to NOOP). Please note that memcg_init_kmem is a common kmem
initializator and it needs to be prepared for !CONFIG_NET.

The same applies to _destroy.
Makes more sense now?
So, that stuff comes originally from the net-tree.

I understand the !CONFIG_NET case, but lack the understanding why
memcontrol.c needs _destroy.
Can you explain that (sorry /me is no mm-geek)?

- Sedat -

[1] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/net/core/sock.c?id=next-20130703#n147
[2] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/include/net/sock.h?id=next-20130703#n73
[...]
--
Michal Hocko
SUSE Labs

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

From: Michal Hocko <hidden>
Date: 2013-07-03 16:34:33

On Wed 03-07-13 18:11:28, Sedat Dilek wrote:
On Wed, Jul 3, 2013 at 5:59 PM, Michal Hocko [off-list ref] wrote:
quoted
On Wed 03-07-13 17:53:21, Sedat Dilek wrote:
quoted
On Wed, Jul 3, 2013 at 5:20 PM, Michal Hocko [off-list ref] wrote:
quoted
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}.
That is the only definition AFAICS (except for !CONFIG_NET where it
expands to NOOP). Please note that memcg_init_kmem is a common kmem
initializator and it needs to be prepared for !CONFIG_NET.

The same applies to _destroy.
Makes more sense now?
So, that stuff comes originally from the net-tree.
No, it all came from tcp kmem accounting. It is a memcg thingy and I
guess it was placed into sock.c because it depends on some static
symbols there (e.g. proto_list_mutex).
I understand the !CONFIG_NET case, but lack the understanding why
memcontrol.c needs _destroy.
Because it is memcg specific and it has to be called when a group is
destroyed.
Can you explain that (sorry /me is no mm-geek)?

- Sedat -

[1] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/net/core/sock.c?id=next-20130703#n147
[2] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/include/net/sock.h?id=next-20130703#n73
quoted
[...]
--
Michal Hocko
SUSE Labs
-- 
Michal Hocko
SUSE Labs

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

From: Sedat Dilek <hidden>
Date: 2013-07-03 16:42:44

On Wed, Jul 3, 2013 at 6:34 PM, Michal Hocko [off-list ref] wrote:
On Wed 03-07-13 18:11:28, Sedat Dilek wrote:
quoted
On Wed, Jul 3, 2013 at 5:59 PM, Michal Hocko [off-list ref] wrote:
quoted
On Wed 03-07-13 17:53:21, Sedat Dilek wrote:
quoted
On Wed, Jul 3, 2013 at 5:20 PM, Michal Hocko [off-list ref] wrote:
quoted
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}.
That is the only definition AFAICS (except for !CONFIG_NET where it
expands to NOOP). Please note that memcg_init_kmem is a common kmem
initializator and it needs to be prepared for !CONFIG_NET.

The same applies to _destroy.
Makes more sense now?
So, that stuff comes originally from the net-tree.
No, it all came from tcp kmem accounting. It is a memcg thingy and I
guess it was placed into sock.c because it depends on some static
symbols there (e.g. proto_list_mutex).
memcg thingies should belong to mm-tree :-).
quoted
I understand the !CONFIG_NET case, but lack the understanding why
memcontrol.c needs _destroy.
Because it is memcg specific and it has to be called when a group is
destroyed.
I looked again into my local GIT tree where I applied Li Zefan's patch.
memcg_destroy_kmem() makes sense with existing memcg_init_kmem().

I have now a better picture, but it's balck/white not coloured :-).

Thanks for your patience and explanations.

- Sedat -
quoted
Can you explain that (sorry /me is no mm-geek)?

- Sedat -

[1] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/net/core/sock.c?id=next-20130703#n147
[2] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/include/net/sock.h?id=next-20130703#n73
quoted
[...]
--
Michal Hocko
SUSE Labs
--
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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help