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@xxxxxxx>
> 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@xxxxxxx>
> ---
> 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@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href