* [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file @ 2012-09-11 8:08 Sachin Kamat 2012-09-11 9:52 ` Michal Hocko 0 siblings, 1 reply; 11+ messages in thread From: Sachin Kamat @ 2012-09-11 8:08 UTC (permalink / raw) To: cgroups, linux-mm Cc: sachin.kamat, Johannes Weiner, Michal Hocko, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton net/sock.h is included unconditionally at the beginning of the file. Hence, another conditional include is not required. Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Michal Hocko <mhocko@suse.cz> Cc: Balbir Singh <bsingharora@gmail.com> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> --- mm/memcontrol.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 795e525..d5e76f5 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -413,7 +413,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); -- 1.7.4.1 -- 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> ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-11 8:08 [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file Sachin Kamat @ 2012-09-11 9:52 ` Michal Hocko 2012-09-12 3:39 ` Sachin Kamat 2012-09-12 7:25 ` Michal Hocko 0 siblings, 2 replies; 11+ messages in thread From: Michal Hocko @ 2012-09-11 9:52 UTC (permalink / raw) To: Sachin Kamat Cc: cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton On Tue 11-09-12 13:38:54, Sachin Kamat wrote: > 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 --- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-11 9:52 ` Michal Hocko @ 2012-09-12 3:39 ` Sachin Kamat 2012-09-12 7:25 ` Michal Hocko 1 sibling, 0 replies; 11+ messages in thread From: Sachin Kamat @ 2012-09-12 3:39 UTC (permalink / raw) To: Michal Hocko Cc: cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton On 11 September 2012 15:22, Michal Hocko <mhocko@suse.cz> wrote: > On Tue 11-09-12 13:38:54, Sachin Kamat wrote: >> 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 Since you have compile tested this with different config options, your method looks better. Thanks. > --- > From 83c5a97e893b5379b7e93cfdc933d5e37756e70a Mon Sep 17 00:00:00 2001 > From: Michal Hocko <mhocko@suse.cz> > 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@linaro.org> > Signed-off-by: Michal Hocko <mhocko@suse.cz> > --- > 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 > }; > @@ -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 -- With warm regards, Sachin -- 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> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-11 9:52 ` Michal Hocko 2012-09-12 3:39 ` Sachin Kamat @ 2012-09-12 7:25 ` Michal Hocko 2012-09-12 8:50 ` Glauber Costa 1 sibling, 1 reply; 11+ messages in thread From: Michal Hocko @ 2012-09-12 7:25 UTC (permalink / raw) To: Sachin Kamat Cc: cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton 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: > On Tue 11-09-12 13:38:54, Sachin Kamat wrote: > > 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 <mhocko@suse.cz> > 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@linaro.org> > Signed-off-by: Michal Hocko <mhocko@suse.cz> > --- > 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 > }; > @@ -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 -- 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> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-12 7:25 ` Michal Hocko @ 2012-09-12 8:50 ` Glauber Costa 2012-09-12 12:56 ` Michal Hocko 0 siblings, 1 reply; 11+ messages in thread From: Glauber Costa @ 2012-09-12 8:50 UTC (permalink / raw) To: Michal Hocko Cc: Sachin Kamat, cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton 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: >> On Tue 11-09-12 13:38:54, Sachin Kamat wrote: >>> 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 <mhocko@suse.cz> >> 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@linaro.org> >> Signed-off-by: Michal Hocko <mhocko@suse.cz> >> --- >> 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. -- 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> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-12 8:50 ` Glauber Costa @ 2012-09-12 12:56 ` Michal Hocko 2012-09-12 13:09 ` Michal Hocko 0 siblings, 1 reply; 11+ messages in thread From: Michal Hocko @ 2012-09-12 12:56 UTC (permalink / raw) To: Glauber Costa Cc: Sachin Kamat, cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton 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: --- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-12 12:56 ` Michal Hocko @ 2012-09-12 13:09 ` Michal Hocko 2012-09-14 7:58 ` Sachin Kamat 0 siblings, 1 reply; 11+ messages in thread From: Michal Hocko @ 2012-09-12 13:09 UTC (permalink / raw) To: Glauber Costa Cc: Sachin Kamat, cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton 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... --- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-12 13:09 ` Michal Hocko @ 2012-09-14 7:58 ` Sachin Kamat 2012-09-14 8:27 ` Michal Hocko 0 siblings, 1 reply; 11+ messages in thread From: Sachin Kamat @ 2012-09-14 7:58 UTC (permalink / raw) To: Michal Hocko Cc: Glauber Costa, cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton Hi Michal, Has this patch been accepted? On 12 September 2012 18:39, Michal Hocko <mhocko@suse.cz> wrote: > 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@suse.cz> > 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@linaro.org> > Signed-off-by: Michal Hocko <mhocko@suse.cz> > --- > 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 -- With warm regards, Sachin -- 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> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-14 7:58 ` Sachin Kamat @ 2012-09-14 8:27 ` Michal Hocko 2012-09-14 8:32 ` Glauber Costa 0 siblings, 1 reply; 11+ messages in thread From: Michal Hocko @ 2012-09-14 8:27 UTC (permalink / raw) To: Sachin Kamat, Glauber Costa Cc: cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton On Fri 14-09-12 13:28:07, Sachin Kamat wrote: > Hi Michal, > > Has this patch been accepted? Not yet. I am waiting for Glauber to ack it. > > On 12 September 2012 18:39, Michal Hocko <mhocko@suse.cz> wrote: > > 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@suse.cz> > > 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@linaro.org> > > Signed-off-by: Michal Hocko <mhocko@suse.cz> > > --- > > 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 > > > > -- > With warm regards, > Sachin -- 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> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-14 8:27 ` Michal Hocko @ 2012-09-14 8:32 ` Glauber Costa 2012-09-14 9:56 ` Michal Hocko 0 siblings, 1 reply; 11+ messages in thread From: Glauber Costa @ 2012-09-14 8:32 UTC (permalink / raw) To: Michal Hocko Cc: Sachin Kamat, cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton On 09/14/2012 12:27 PM, Michal Hocko wrote: > On Fri 14-09-12 13:28:07, Sachin Kamat wrote: >> Hi Michal, >> >> Has this patch been accepted? > > Not yet. I am waiting for Glauber to ack it. > I am fine with the change, assuming you tested it, after you made the change I requested. >> >> On 12 September 2012 18:39, Michal Hocko <mhocko@suse.cz> wrote: >>> 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@suse.cz> >>> 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@linaro.org> >>> Signed-off-by: Michal Hocko <mhocko@suse.cz> >>> --- >>> 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 >> >> >> >> -- >> With warm regards, >> Sachin > -- 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> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file 2012-09-14 8:32 ` Glauber Costa @ 2012-09-14 9:56 ` Michal Hocko 0 siblings, 0 replies; 11+ messages in thread From: Michal Hocko @ 2012-09-14 9:56 UTC (permalink / raw) To: Glauber Costa Cc: Sachin Kamat, cgroups, linux-mm, Johannes Weiner, Balbir Singh, KAMEZAWA Hiroyuki, Andrew Morton On Fri 14-09-12 12:32:45, Glauber Costa wrote: > On 09/14/2012 12:27 PM, Michal Hocko wrote: > > On Fri 14-09-12 13:28:07, Sachin Kamat wrote: > >> Hi Michal, > >> > >> Has this patch been accepted? > > > > Not yet. I am waiting for Glauber to ack it. > > > > I am fine with the change, assuming you tested it, after you made the > change I requested. OK, I will repost the patch. -- 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> ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-09-14 9:56 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-11 8:08 [PATCH] mm/memcontrol.c: Remove duplicate inclusion of sock.h file Sachin Kamat 2012-09-11 9:52 ` Michal Hocko 2012-09-12 3:39 ` Sachin Kamat 2012-09-12 7:25 ` Michal Hocko 2012-09-12 8:50 ` Glauber Costa 2012-09-12 12:56 ` Michal Hocko 2012-09-12 13:09 ` Michal Hocko 2012-09-14 7:58 ` Sachin Kamat 2012-09-14 8:27 ` Michal Hocko 2012-09-14 8:32 ` Glauber Costa 2012-09-14 9:56 ` Michal Hocko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).