Re: [PATCH v2] memcg: clean up networking headers file inclusion

Re: [PATCH v2] memcg: clean up networking headers file inclusion

[Glauber Costa]

On 09/14/2012 04:01 PM, Michal Hocko wrote:
> On Fri 14-09-12 15:35:50, Glauber Costa wrote:
>> On 09/14/2012 03:34 PM, Michal Hocko wrote:
>>> On Fri 14-09-12 15:21:29, Glauber Costa wrote:
>>>> On 09/14/2012 03:21 PM, Michal Hocko wrote:
>>>>> Hi,
>>>>> so I did some more changes to ifdefery of sock kmem part. The patch is
>>>>> below. 
>>>>> Glauber please have a look at it. I do not think any of the
>>>>> functionality wrapped inside CONFIG_MEMCG_KMEM without CONFIG_INET is
>>>>> reusable for generic CONFIG_MEMCG_KMEM, right?
>>>> Almost right.
>>>>
>>>>
>>>>
>>>>>  }
>>>>>  
>>>>>  /* 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);
>>>>
>>>> This one is. ^^^^
>>>
>>> But this is just a forward declaration. And btw. it makes my compiler
>>> complain about:
>>> mm/memcontrol.c:421: warning: ‘mem_cgroup_is_root’ declared inline after being called
>>> mm/memcontrol.c:421: warning: previous declaration of ‘mem_cgroup_is_root’ was here
>>>
>>> But I didn't care much yet. It is probaly that my compiler is too old to
>>> be clever about this.
>>>
>> Weird, this code is in tree for a long time.
> 
> Yes I think it is just compiler issue. Anyway the trivial patch bellow
> does the trick.

That seems to be alright, and it works for me as well.


> ---
> From 28a803e6e07c5b7ae6b88d7b26801666781aa899 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.cz>
> Date: Fri, 14 Sep 2012 13:56:32 +0200
> Subject: [PATCH] memcg: move mem_cgroup_is_root upwards
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> kmem code uses this function and it is better to not use forward
> declarations for static inline functions as some (older) compilers don't
> like it:
> 
> gcc version 4.3.4 [gcc-4_3-branch revision 152973] (SUSE Linux)
> 
> mm/memcontrol.c:421: warning: ‘mem_cgroup_is_root’ declared inline after being called
> mm/memcontrol.c:421: warning: previous declaration of ‘mem_cgroup_is_root’ was here
> 
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> ---
>  mm/memcontrol.c |   11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 0cd25e9..df69552 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -415,10 +415,14 @@ struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
>  	return container_of(s, struct mem_cgroup, css);
>  }
>  
> +static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
> +{
> +	return (memcg == root_mem_cgroup);
> +}
> +
>  /* Writing them here to avoid exposing memcg's inner layout */
>  #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)
>  {
>  	if (mem_cgroup_sockets_enabled) {
> @@ -1014,11 +1018,6 @@ void mem_cgroup_iter_break(struct mem_cgroup *root,
>  	     iter != NULL;				\
>  	     iter = mem_cgroup_iter(NULL, iter, NULL))
>  
> -static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
> -{
> -	return (memcg == root_mem_cgroup);
> -}
> -
>  void mem_cgroup_count_vm_event(struct mm_struct *mm, enum vm_event_item idx)
>  {
>  	struct mem_cgroup *memcg;
> 

--
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 v3] memcg: clean up networking headers file inclusion

[Glauber Costa]

On 09/14/2012 04:08 PM, Michal Hocko wrote:
> On Fri 14-09-12 15:35:50, Glauber Costa wrote:
> [...]
>> So, *right now* this code is used only for inet code, so I won't oppose
>> your patch on this basis. I'll reuse it for kmem, but I am happy to just
>> rebase it.
> 
> Hmm, I guess I was too strict after all. memcg_init_kmem doesn't need
> CONFIG_INET gueard as both mem_cgroup_sockets_{init,destroy} are defined
> empty for !CONFIG_INET. All other functions guarded in INET&&KMEM combo
> seem to be networking specific.
> Updated patch bellow:
> ---
> From 4dca5e135b4dcc08464bbd70761d094f99ed83b1 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 and CONFIG_INET are not selected which is not
> necessary.
> While we are at it, let's clean up KMEM sock code ifdefs to require both
> CONFIG_KMEM and CONFIG_INET as it doesn't make much sense to compile
> this code if there is no possible user for it.
> 
> Tested with
> - CONFIG_INET && CONFIG_MEMCG_KMEM
> - !CONFIG_INET && CONFIG_MEMCG_KMEM
> - CONFIG_INET && !CONFIG_MEMCG_KMEM
> - !CONFIG_INET && !CONFIG_MEMCG_KMEM
> 
> 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@linaro.org>
> Signed-off-by: Michal Hocko <mhocko@suse.cz>

Seems safe now. Since the config matrix can get tricky, and we have no
pressing time issues with this, I would advise to give it a day in
Fengguang's magic system before merging it. Just put it in a temp branch
in korg and let it do the job.

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

[PATCH v2] memcg: clean up networking headers file inclusion

[Michal Hocko]

Hi,
so I did some more changes to ifdefery of sock kmem part. The patch is
below. 
Glauber please have a look at it. I do not think any of the
functionality wrapped inside CONFIG_MEMCG_KMEM without CONFIG_INET is
reusable for generic CONFIG_MEMCG_KMEM, right?
---

Re: [PATCH v2] memcg: clean up networking headers file inclusion

[Glauber Costa]

On 09/14/2012 03:21 PM, Michal Hocko wrote:
> Hi,
> so I did some more changes to ifdefery of sock kmem part. The patch is
> below. 
> Glauber please have a look at it. I do not think any of the
> functionality wrapped inside CONFIG_MEMCG_KMEM without CONFIG_INET is
> reusable for generic CONFIG_MEMCG_KMEM, right?
Almost right.



>  }
>  
>  /* 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);

This one is. ^^^^

--
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 v2] memcg: clean up networking headers file inclusion

[Michal Hocko]

On Fri 14-09-12 15:21:29, Glauber Costa wrote:
> On 09/14/2012 03:21 PM, Michal Hocko wrote:
> > Hi,
> > so I did some more changes to ifdefery of sock kmem part. The patch is
> > below. 
> > Glauber please have a look at it. I do not think any of the
> > functionality wrapped inside CONFIG_MEMCG_KMEM without CONFIG_INET is
> > reusable for generic CONFIG_MEMCG_KMEM, right?
> Almost right.
> 
> 
> 
> >  }
> >  
> >  /* 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);
> 
> This one is. ^^^^

But this is just a forward declaration. And btw. it makes my compiler
complain about:
mm/memcontrol.c:421: warning: a??mem_cgroup_is_roota?? declared inline after being called
mm/memcontrol.c:421: warning: previous declaration of a??mem_cgroup_is_roota?? was here

But I didn't care much yet. It is probaly that my compiler is too old to
be clever about this.
-- 
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 v2] memcg: clean up networking headers file inclusion

[Glauber Costa]

On 09/14/2012 03:34 PM, Michal Hocko wrote:
> On Fri 14-09-12 15:21:29, Glauber Costa wrote:
>> On 09/14/2012 03:21 PM, Michal Hocko wrote:
>>> Hi,
>>> so I did some more changes to ifdefery of sock kmem part. The patch is
>>> below. 
>>> Glauber please have a look at it. I do not think any of the
>>> functionality wrapped inside CONFIG_MEMCG_KMEM without CONFIG_INET is
>>> reusable for generic CONFIG_MEMCG_KMEM, right?
>> Almost right.
>>
>>
>>
>>>  }
>>>  
>>>  /* 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);
>>
>> This one is. ^^^^
> 
> But this is just a forward declaration. And btw. it makes my compiler
> complain about:
> mm/memcontrol.c:421: warning: ‘mem_cgroup_is_root’ declared inline after being called
> mm/memcontrol.c:421: warning: previous declaration of ‘mem_cgroup_is_root’ was here
> 
> But I didn't care much yet. It is probaly that my compiler is too old to
> be clever about this.
> 
Weird, this code is in tree for a long time.
So, *right now* this code is used only for inet code, so I won't oppose
your patch on this basis. I'll reuse it for kmem, but I am happy to just
rebase it.


--
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 v2] memcg: clean up networking headers file inclusion

[Michal Hocko]

On Fri 14-09-12 15:35:50, Glauber Costa wrote:
> On 09/14/2012 03:34 PM, Michal Hocko wrote:
> > On Fri 14-09-12 15:21:29, Glauber Costa wrote:
> >> On 09/14/2012 03:21 PM, Michal Hocko wrote:
> >>> Hi,
> >>> so I did some more changes to ifdefery of sock kmem part. The patch is
> >>> below. 
> >>> Glauber please have a look at it. I do not think any of the
> >>> functionality wrapped inside CONFIG_MEMCG_KMEM without CONFIG_INET is
> >>> reusable for generic CONFIG_MEMCG_KMEM, right?
> >> Almost right.
> >>
> >>
> >>
> >>>  }
> >>>  
> >>>  /* 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);
> >>
> >> This one is. ^^^^
> > 
> > But this is just a forward declaration. And btw. it makes my compiler
> > complain about:
> > mm/memcontrol.c:421: warning: a??mem_cgroup_is_roota?? declared inline after being called
> > mm/memcontrol.c:421: warning: previous declaration of a??mem_cgroup_is_roota?? was here
> > 
> > But I didn't care much yet. It is probaly that my compiler is too old to
> > be clever about this.
> > 
> Weird, this code is in tree for a long time.

Yes I think it is just compiler issue. Anyway the trivial patch bellow
does the trick.
---

[PATCH v3] memcg: clean up networking headers file inclusion

[Michal Hocko]

On Fri 14-09-12 15:35:50, Glauber Costa wrote:
[...]
> So, *right now* this code is used only for inet code, so I won't oppose
> your patch on this basis. I'll reuse it for kmem, but I am happy to just
> rebase it.

Hmm, I guess I was too strict after all. memcg_init_kmem doesn't need
CONFIG_INET gueard as both mem_cgroup_sockets_{init,destroy} are defined
empty for !CONFIG_INET. All other functions guarded in INET&&KMEM combo
seem to be networking specific.
Updated patch bellow:
---

Re: [PATCH v2] memcg: clean up networking headers file inclusion

[Michal Hocko]

On Fri 14-09-12 12:05:36, Glauber Costa wrote:
> On 09/14/2012 04:01 PM, Michal Hocko wrote:
> > On Fri 14-09-12 15:35:50, Glauber Costa wrote:
> >> On 09/14/2012 03:34 PM, Michal Hocko wrote:
> >>> On Fri 14-09-12 15:21:29, Glauber Costa wrote:
> >>>> On 09/14/2012 03:21 PM, Michal Hocko wrote:
> >>>>> Hi,
> >>>>> so I did some more changes to ifdefery of sock kmem part. The patch is
> >>>>> below. 
> >>>>> Glauber please have a look at it. I do not think any of the
> >>>>> functionality wrapped inside CONFIG_MEMCG_KMEM without CONFIG_INET is
> >>>>> reusable for generic CONFIG_MEMCG_KMEM, right?
> >>>> Almost right.
> >>>>
> >>>>
> >>>>
> >>>>>  }
> >>>>>  
> >>>>>  /* 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);
> >>>>
> >>>> This one is. ^^^^
> >>>
> >>> But this is just a forward declaration. And btw. it makes my compiler
> >>> complain about:
> >>> mm/memcontrol.c:421: warning: a??mem_cgroup_is_roota?? declared inline after being called
> >>> mm/memcontrol.c:421: warning: previous declaration of a??mem_cgroup_is_roota?? was here
> >>>
> >>> But I didn't care much yet. It is probaly that my compiler is too old to
> >>> be clever about this.
> >>>
> >> Weird, this code is in tree for a long time.
> > 
> > Yes I think it is just compiler issue. Anyway the trivial patch bellow
> > does the trick.
> 
> That seems to be alright, and it works for me as well.

OK, I will post it on top of the last version.

> 
> 
> > ---
> > From 28a803e6e07c5b7ae6b88d7b26801666781aa899 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.cz>
> > Date: Fri, 14 Sep 2012 13:56:32 +0200
> > Subject: [PATCH] memcg: move mem_cgroup_is_root upwards
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> > 
> > kmem code uses this function and it is better to not use forward
> > declarations for static inline functions as some (older) compilers don't
> > like it:
> > 
> > gcc version 4.3.4 [gcc-4_3-branch revision 152973] (SUSE Linux)
> > 
> > mm/memcontrol.c:421: warning: a??mem_cgroup_is_roota?? declared inline after being called
> > mm/memcontrol.c:421: warning: previous declaration of a??mem_cgroup_is_roota?? was here
> > 
> > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> > ---
> >  mm/memcontrol.c |   11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 0cd25e9..df69552 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -415,10 +415,14 @@ struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
> >  	return container_of(s, struct mem_cgroup, css);
> >  }
> >  
> > +static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
> > +{
> > +	return (memcg == root_mem_cgroup);
> > +}
> > +
> >  /* Writing them here to avoid exposing memcg's inner layout */
> >  #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)
> >  {
> >  	if (mem_cgroup_sockets_enabled) {
> > @@ -1014,11 +1018,6 @@ void mem_cgroup_iter_break(struct mem_cgroup *root,
> >  	     iter != NULL;				\
> >  	     iter = mem_cgroup_iter(NULL, iter, NULL))
> >  
> > -static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
> > -{
> > -	return (memcg == root_mem_cgroup);
> > -}
> > -
> >  void mem_cgroup_count_vm_event(struct mm_struct *mm, enum vm_event_item idx)
> >  {
> >  	struct mem_cgroup *memcg;
> > 
> 

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

[PATCH] memcg: clean up networking headers file inclusion

[Michal Hocko]

And here is another trivial patch on top of the previous patch.
I am not sure whether we care about such old compilers but the change
itself makes some sense on its own.
---

Re: [PATCH v3] memcg: clean up networking headers file inclusion

[Michal Hocko]

On Fri 14-09-12 12:14:30, Glauber Costa wrote:
> On 09/14/2012 04:08 PM, Michal Hocko wrote:
> > On Fri 14-09-12 15:35:50, Glauber Costa wrote:
> > [...]
> >> So, *right now* this code is used only for inet code, so I won't oppose
> >> your patch on this basis. I'll reuse it for kmem, but I am happy to just
> >> rebase it.
> > 
> > Hmm, I guess I was too strict after all. memcg_init_kmem doesn't need
> > CONFIG_INET gueard as both mem_cgroup_sockets_{init,destroy} are defined
> > empty for !CONFIG_INET. All other functions guarded in INET&&KMEM combo
> > seem to be networking specific.
> > Updated patch bellow:
> > ---
> > From 4dca5e135b4dcc08464bbd70761d094f99ed83b1 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 and CONFIG_INET are not selected which is not
> > necessary.
> > While we are at it, let's clean up KMEM sock code ifdefs to require both
> > CONFIG_KMEM and CONFIG_INET as it doesn't make much sense to compile
> > this code if there is no possible user for it.
> > 
> > Tested with
> > - CONFIG_INET && CONFIG_MEMCG_KMEM
> > - !CONFIG_INET && CONFIG_MEMCG_KMEM
> > - CONFIG_INET && !CONFIG_MEMCG_KMEM
> > - !CONFIG_INET && !CONFIG_MEMCG_KMEM
> > 
> > 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@linaro.org>
> > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> 
> Seems safe now. Since the config matrix can get tricky, and we have no
> pressing time issues with this, I would advise to give it a day in
> Fengguang's magic system before merging it. Just put it in a temp branch
> in korg and let it do the job.

OK done. It is cleanups/memcg-sock-include.

Fengguang, do you think we can (ab)use your build test coverity to test
git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git cleanups/memcg-sock-include

Thanks a lot!

I will repost both patches later (and hopefully I will not forget about
other people on the CC list list like 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 v3] memcg: clean up networking headers file inclusion

[Fengguang Wu]

> > Seems safe now. Since the config matrix can get tricky, and we have no
> > pressing time issues with this, I would advise to give it a day in
> > Fengguang's magic system before merging it. Just put it in a temp branch
> > in korg and let it do the job.
> 
> OK done. It is cleanups/memcg-sock-include.
> 
> Fengguang, do you think we can (ab)use your build test coverity to test
> git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git cleanups/memcg-sock-include
> 
> Thanks a lot!

Feel free to take advantage of it to your heart's content!  Actually
one of my biggest joy of working (hard) is to see users being able to
make utmost use of the resulted system or feature :)

The tests will auto start shortly after you push the branch.

Thanks,
Fengguang

--
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 v3] memcg: clean up networking headers file inclusion

[Michal Hocko]

On Fri 14-09-12 20:41:24, Wu Fengguang wrote:
> > > Seems safe now. Since the config matrix can get tricky, and we have no
> > > pressing time issues with this, I would advise to give it a day in
> > > Fengguang's magic system before merging it. Just put it in a temp branch
> > > in korg and let it do the job.
> > 
> > OK done. It is cleanups/memcg-sock-include.
> > 
> > Fengguang, do you think we can (ab)use your build test coverity to test
> > git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git cleanups/memcg-sock-include
> > 
> > Thanks a lot!
> 
> Feel free to take advantage of it to your heart's content!  Actually
> one of my biggest joy of working (hard) is to see users being able to
> make utmost use of the resulted system or feature :)

Thanks a lot and I really appraciate that!

> The tests will auto start shortly after you push the branch.

OK, the branch should be there so it probably need few minutes to sync.

> 
> Thanks,
> Fengguang

-- 
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 v3] memcg: clean up networking headers file inclusion

[Andrew Morton]

On Fri, 14 Sep 2012 14:08:49 +0200
Michal Hocko <mhocko@suse.cz> 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.

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

[PATCH v4] memcg: cleanup kmem tcp ifdefs

[Michal Hocko]

On Fri 14-09-12 12:45:05, Andrew Morton wrote:
> On Fri, 14 Sep 2012 14:08:49 +0200
> Michal Hocko <mhocko@suse.cz> 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:
---