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

[Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>]

On Wed 12-09-12 14:56:47, Michal Hocko wrote:
> On Wed 12-09-12 12:50:41, Glauber Costa wrote:
> [...]
> > >> 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.
>  
>  You are right of course. Updated patch bellow:

Bahh. And I managed to send a different patch than I tested...
---
From 0617ff7114bdf424160a8f1533784c837d426ec2 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>
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 <sachin.kamat-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>
---
 mm/memcontrol.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 795e525..1a217b4 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -50,8 +50,12 @@
 #include <linux/cpu.h>
 #include <linux/oom.h>
 #include "internal.h"
+
+#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
 #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
+#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
 	struct tcp_memcontrol tcp_mem;
 #endif
 };
@@ -413,8 +417,6 @@ struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
 
 /* Writing them here to avoid exposing memcg's inner layout */
 #ifdef CONFIG_MEMCG_KMEM
-#include <net/sock.h>
-#include <net/ip.h>
 
 static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
 void sock_update_memcg(struct sock *sk)
-- 
1.7.10.4

-- 
Michal Hocko
SUSE Labs