linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Chris Down <chris@chrisdown.name>
To: Qian Cai <cai@lca.pw>
Cc: Michal Hocko <mhocko@kernel.org>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	cgroups@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: memcontrol.c: move mem_cgroup_id_get_many under CONFIG_MMU
Date: Tue, 17 Dec 2019 14:37:20 +0000	[thread overview]
Message-ID: <20191217143720.GB131030@chrisdown.name> (raw)
In-Reply-To: <392D7C59-5538-4A9B-8974-DB0B64880C2C@lca.pw>

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.)


  reply	other threads:[~2019-12-17 14:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-17  6:47 Kuninori Morimoto
2019-12-17  9:53 ` Michal Hocko
2019-12-17 13:54   ` Chris Down
2019-12-17 14:16     ` Qian Cai
2019-12-17 14:37       ` Chris Down [this message]
2019-12-17 15:09         ` Qian Cai
2019-12-17 14:46       ` Michal Hocko
2019-12-17 15:04         ` Qian Cai
2019-12-17 15:13           ` Michal Hocko
2019-12-17 15:17             ` Qian Cai
2019-12-17 15:09         ` Chris Down
2019-12-17 15:19           ` Michal Hocko
2019-12-17 15:28             ` Chris Down
2019-12-17 15:32               ` Chris Down
2019-12-17 15:34               ` Qian Cai
2019-12-17 15:46               ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191217143720.GB131030@chrisdown.name \
    --to=chris@chrisdown.name \
    --cc=akpm@linux-foundation.org \
    --cc=cai@lca.pw \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=vdavydov.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox