All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Lance Yang <ioworker0@gmail.com>
Cc: mingzhe.yang@ly.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	David Hildenbrand <david@redhat.com>,
	Lance Yang <lance.yang@linux.dev>
Subject: Re: [RESEND PATCH v2 1/1] mm/rmap: add CONFIG_MM_ID guard for folio_test_large_maybe_mapped_shared()
Date: Sun, 20 Apr 2025 16:29:25 -0700	[thread overview]
Message-ID: <20250420162925.2c58c018defee9ee192be553@linux-foundation.org> (raw)
In-Reply-To: <20250418152228.20545-1-lance.yang@linux.dev>

On Fri, 18 Apr 2025 23:22:28 +0800 Lance Yang <ioworker0@gmail.com> wrote:

> From: Lance Yang <lance.yang@linux.dev>
> 
> To prevent folio_test_large_maybe_mapped_shared() from being used without
> CONFIG_MM_ID, we add a compile-time check rather than wrapping it in
> '#ifdef', avoiding even more #ifdef in callers that already use
> IS_ENABLED(CONFIG_MM_ID).
> 
> Also, we used plenty of IS_ENABLED() on purpose to keep the code free of
> '#ifdef' mess.

I dunno, this just seems really whacky.

> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -1232,6 +1232,8 @@ static inline int folio_has_private(const struct folio *folio)
>  
>  static inline bool folio_test_large_maybe_mapped_shared(const struct folio *folio)
>  {
> +	/* This function should never be called without CONFIG_MM_ID enabled. */

A correcter comment would be "This function should never be compiled
without CONFIG_MM_ID enabled".

Which lets the cat out of the bag.  Why the heck is it being compiled
with CONFIG_MM_ID=n??  We have tools to prevent that.

Can we just slap "#ifdef CONFIG_MM_ID" around the whole function?  It
should have no callers, right?  If the linker ends up complaining then
something went wrong.


  reply	other threads:[~2025-04-20 23:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-18 15:22 [RESEND PATCH v2 1/1] mm/rmap: add CONFIG_MM_ID guard for folio_test_large_maybe_mapped_shared() Lance Yang
2025-04-20 23:29 ` Andrew Morton [this message]
2025-04-21  5:13   ` Lance Yang
2025-04-21  5:16     ` Lance Yang
2025-04-21  7:17       ` David Hildenbrand
2025-04-21  7:50         ` Lance Yang
2025-04-21 19:22     ` Andrew Morton
2025-04-22  4:35       ` Lance Yang
2025-04-22  7:00         ` David Hildenbrand
2025-04-22 23:20           ` Andrew Morton

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=20250420162925.2c58c018defee9ee192be553@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=ioworker0@gmail.com \
    --cc=lance.yang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingzhe.yang@ly.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.