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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20191217143720.GB131030@chrisdown.name>
Date:   Tue, 17 Dec 2019 14:37:20 +0000
From:   Chris Down <chris@...isdown.name>
To:     Qian Cai <cai@....pw>
Cc:     Michal Hocko <mhocko@...nel.org>,
        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

Qian Cai writes:
>__maybe_unused should only be used in the last resort as it mark the compiler 
>to catch the real issues in the future. In this case, it might be better just 
>ignore it as only non-realistic compiling test would use !CONFIG_MMU in this 
>case.

While that's true, I'd rather not end up with getting more patches based on 
tests like these. On balance the risk of adding __maybe_unused here with a note 
to remove it later seems better than having to reply to every patch removing 
warnings :-)

I struggle to imagine a real issue this would catch that wouldn't already be 
caught by other means. If it's just the risks of dead code, that seems equally 
risky as taking time away from reviewers.

We should probably also review the coding style doc again, since this looks 
suspect:

>If you have a function or variable which may potentially go unused in a
>particular configuration, and the compiler would warn about its definition
>going unused, mark the definition as __maybe_unused rather than wrapping it in
>a preprocessor conditional.  (However, if a function or variable *always* goes
>unused, delete it.)