On Fri 14-09-12 12:45:05, Andrew Morton wrote:
> On Fri, 14 Sep 2012 14:08:49 +0200
> Michal Hocko <mhocko@xxxxxxx> wrote:
>
> > --- 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
>
> That wasn't a cleanup!
>
> Why not just unconditionally include them? That will impact compile
> time a teeny bit, but the code is cleaner.
>
> And it's safer, too - conditionally including header files make it more
> likely that people will accidentally break the build by not testing all
> relevant CONFIG_foo combinations.
OK, fair point. Updated patch below:
---
>From 3441c8faeefca4b882250bef1ba874889f931044 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@xxxxxxx>
Date: Tue, 11 Sep 2012 10:38:42 +0200
Subject: [PATCH 1/2] memcg: cleanup kmem tcp ifdefs
TCP kmem accounting is currently guarded by CONFIG_MEMCG_KMEM ifdefs but
the code is not used if !CONFIG_INET so we should rather test for both.
The same applies to net/sock.h, net/ip.h and net/tcp_memcontrol.h but
let's keep those outside of any ifdefs because it is considered safer
wrt. future maintainability.
Tested with
- CONFIG_INET && CONFIG_MEMCG_KMEM
- !CONFIG_INET && CONFIG_MEMCG_KMEM
- CONFIG_INET && !CONFIG_MEMCG_KMEM
- !CONFIG_INET && !CONFIG_MEMCG_KMEM
Changes since V3:
- include files unconditioanlly as per Andrew
Changes since V2:
- memcg_init_kmem and kmem_cgroup_destroy don't need CONFIG_INET
Changes since V1:
- depend on both CONFIG_INET and CONFIG_MEMCG_KMEM for both
mem_cgroup->tcp_mem and the sock specific code
Signed-off-by: Sachin Kamat <sachin.kamat@xxxxxxxxxx>
Signed-off-by: Michal Hocko <mhocko@xxxxxxx>
---
mm/memcontrol.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 795e525..cf5cb0b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -51,6 +51,7 @@
#include <linux/oom.h>
#include "internal.h"
#include <net/sock.h>
+#include <net/ip.h>
#include <net/tcp_memcontrol.h>
#include <asm/uaccess.h>
@@ -326,7 +327,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
};
@@ -412,9 +413,7 @@ 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>
+#if defined(CONFIG_INET) && defined(CONFIG_MEMCG_KMEM)
static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
void sock_update_memcg(struct sock *sk)
@@ -461,7 +460,6 @@ void sock_release_memcg(struct sock *sk)
}
}
-#ifdef CONFIG_INET
struct cg_proto *tcp_proto_cgroup(struct mem_cgroup *memcg)
{
if (!memcg || mem_cgroup_is_root(memcg))
@@ -470,10 +468,7 @@ struct cg_proto *tcp_proto_cgroup(struct mem_cgroup *memcg)
return &memcg->tcp_mem.cg_proto;
}
EXPORT_SYMBOL(tcp_proto_cgroup);
-#endif /* CONFIG_INET */
-#endif /* CONFIG_MEMCG_KMEM */
-#if defined(CONFIG_INET) && defined(CONFIG_MEMCG_KMEM)
static void disarm_sock_keys(struct mem_cgroup *memcg)
{
if (!memcg_proto_activated(&memcg->tcp_mem.cg_proto))
--
1.7.10.4
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>