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

[Glauber Costa <glommer@parallels.com>]

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>