linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Chris Down <chris@chrisdown.name>
To: Michal Hocko <mhocko@kernel.org>
Cc: Qian Cai <cai@lca.pw>,
	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 15:28:14 +0000	[thread overview]
Message-ID: <20191217152814.GB136178@chrisdown.name> (raw)
In-Reply-To: <20191217151931.GD7272@dhcp22.suse.cz>

Michal Hocko writes:
>On Tue 17-12-19 15:09:21, Chris Down wrote:
>[...]
>> (Side note: I'm moderately baffled that a tightly scoped __maybe_unused is
>> considered sinister but somehow disabling -Wunused-function is on the table
>> :-))
>
>Well, I usually do not like to see __maybe_unused because that is prone
>to bit-rot and loses its usefulness. Looking into the recent git logs
>most -Wunused-function led to the code removal (which is really good
>but the compiler is likely to do that already so the overall impact is
>not that large) or more ifdefery. I do not really see many instance of
>__maybe_unused.

Hmm, but __maybe_unused is easy to find and document the reasons behind nearby, 
and then reevaluate at some later time. On the other hand, it's much *harder* 
to reevaluate which functions actually are unused in the long term if we remove 
-Wunused-function, because enabling it to find candidates will result in an 
incredibly amount of noise from those who have missed unused functions 
previously due to the lack of the warning.

Maybe Qian is right and we should just ignore such patches, but I think that 
comes with its own risks that we will alienate perfectly well intentioned new 
contributors to mm without them having any idea why we did that.


  reply	other threads:[~2019-12-17 15:28 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
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 [this message]
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=20191217152814.GB136178@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