linux-kernel - Re: [PATCH] mm: memcontrol.c: move mem_cgroup_id_get_many under CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191217135440.GB58496@chrisdown.name>
Date:   Tue, 17 Dec 2019 13:54:40 +0000
From:   Chris Down <chris@...isdown.name>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        cgroups@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: memcontrol.c: move mem_cgroup_id_get_many under
 CONFIG_MMU

Hi Kuninori,

Michal Hocko writes:
>On Tue 17-12-19 15:47:40, Kuninori Morimoto wrote:
>> From: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
>>
>> mem_cgroup_id_get_many() is used under CONFIG_MMU.
>
>Not really. It is used when SWAP is enabled currently. But it is not
>really bound to the swap functionality by any means. It just happens
>that we do not have other users currently. We might put it under
>CONFIG_SWAP but I do not really think it is a big improvement.

Agreed, I think we shouldn't wrap this in preprocessor conditionals it since 
it's entirely possible it will end up used elsewhere and we'll end up with a 
mess of #ifdefs.

>> This patch moves it to under CONFIG_MMU.
>> We will get below warning without this patch
>> if .config doesn't have CONFIG_MMU.
>>
>> 	LINUX/mm/memcontrol.c:4814:13: warning: 'mem_cgroup_id_get_many'\
>> 		defined but not used [-Wunused-function]
>> 	static void mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n)
>> 	^~~~~~~~~~~~~~~~~~~~~~
>
>Is this warning really a big deal? The function is not used, alright,
>and the compiler will likely just drop it.

Let's just add __maybe_unused, since it seems like what we want in this 
scenario -- it avoids new users having to enter preprocessor madness, while 
also not polluting the build output.

Once you've done that, I'll send over my ack. :-)

Thanks.