Re: [PATCH] memcg: stop warning on memcg_propagate_kmem

[Lord Glauber Costa of Sealand]

On 02/04/2013 12:36 PM, Michal Hocko wrote:
> On Mon 04-02-13 12:04:06, Glauber Costa wrote:
>> On 02/04/2013 11:57 AM, Michal Hocko wrote:
>>> On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
>>>> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
>>>> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
>>>> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
>>>> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>>>>
>>>> Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
>>>
>>> Acked-by: Michal Hocko <mhocko@xxxxxxx>
>>>
>>> Hmm, if you are not too tired then moving the function downwards to
>>> where it is called (memcg_init_kmem) will reduce the number of ifdefs.
>>> But this can wait for a bigger clean up which is getting due:
>>> git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
>>> 12
>>>
>>
>> The problem is that I was usually keeping things in clearly separated
>> blocks, like this :
>>
>> #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
>>         struct tcp_memcontrol tcp_mem;
>> #endif
>> #if defined(CONFIG_MEMCG_KMEM)
>>         /* analogous to slab_common's slab_caches list. per-memcg */
>>         struct list_head memcg_slab_caches;
>>         /* Not a spinlock, we can take a lot of time walking the list */
>>         struct mutex slab_caches_mutex;
>>         /* Index in the kmem_cache->memcg_params->memcg_caches array */
>>         int kmemcg_id;
>> #endif
>>
>> If it would be preferable to everybody, this could be easily rewritten as:
>>
>> #if defined(CONFIG_MEMCG_KMEM)
>> #if defined(CONFIG_INET)
>>         struct tcp_memcontrol tcp_mem;
>> #endif
>>         /* analogous to slab_common's slab_caches list. per-memcg */
>>         struct list_head memcg_slab_caches;
>>         /* Not a spinlock, we can take a lot of time walking the list */
>>         struct mutex slab_caches_mutex;
>>         /* Index in the kmem_cache->memcg_params->memcg_caches array */
>>         int kmemcg_id;
>> #endif
> 
> I was rather interested in reducing CONFIG_KMEM block, the above example
> doesn't bother me that much.
>  
>> This would allow us to collapse some blocks a bit down as well.
>>
>> It doesn't bother me *that* much, though.
> 
> Yes and a quick attempt shows that a clean up would bring a lot of
> churn.
> 
And some of it, because there are circular dependencies. So we would
have to start adding forward declarations here and there to make it all
work. That is part of the reason why I kept the blocks separate.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/