Thread (11 messages) 11 messages, 3 authors, 2012-09-14

Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file

From: Glauber Costa <hidden>
Date: 2012-09-12 08:54:04
Also in: linux-mm

On 09/12/2012 11:25 AM, Michal Hocko wrote:
Just realized that Glauber is not in the CC list. Glauber, could you
have a look? The thread started here:
http://www.spinics.net/lists/linux-mm/msg41725.html

Thanks!

On Tue 11-09-12 11:52:00, Michal Hocko wrote:
quoted
On Tue 11-09-12 13:38:54, Sachin Kamat wrote:
quoted
net/sock.h is included unconditionally at the beginning of the file.
Hence, another conditional include is not required.
I guess we can do little bit better. What do you think about the
following?  I have compile tested this with:
- CONFIG_INET=y && CONFIG_MEMCG_KMEM=n
- CONFIG_MEMCG_KMEM=y
---
From 83c5a97e893b5379b7e93cfdc933d5e37756e70a Mon Sep 17 00:00:00 2001
From: Michal Hocko <redacted>
Date: Tue, 11 Sep 2012 10:38:42 +0200
Subject: [PATCH] memcg: clean up networking headers file inclusion

Memory controller doesn't need anything from the networking stack unless
CONFIG_MEMCG_KMEM is selected.
Now we are including net/sock.h and net/tcp_memcontrol.h unconditionally
which is not necessary. Moreover struct mem_cgroup contains tcp_mem even
if CONFIG_MEMCG_KMEM is not selected which is not necessary.

Signed-off-by: Sachin Kamat <redacted>
Signed-off-by: Michal Hocko <redacted>
---
 mm/memcontrol.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 795e525..85ec9ff 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -50,8 +50,12 @@
 #include <linux/cpu.h>
 #include <linux/oom.h>
 #include "internal.h"
+
+#ifdef CONFIG_MEMCG_KMEM
 #include <net/sock.h>
+#include <net/ip.h>
 #include <net/tcp_memcontrol.h>
+#endif
 
 #include <asm/uaccess.h>
 
@@ -326,7 +330,7 @@ struct mem_cgroup {
 	struct mem_cgroup_stat_cpu nocpu_base;
 	spinlock_t pcp_counter_lock;
 
-#ifdef CONFIG_INET
+#ifdef CONFIG_MEMCG_KMEM
 	struct tcp_memcontrol tcp_mem;
 #endif
 };
If you are changing this, why not test for both? This field will be
useless with inet disabled. I usually don't like conditional in
structures (note that the "kmem" res counter in my patchsets is not
conditional to KMEM!!), but since the decision was made to make this one
conditional, I think INET is a much better test. I am fine with both though.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help