linux-kernel - Re: [RESEND PATCH v2 1/1] mm/rmap: add CONFIG_MM_ID guard for folio_test_large_maybe_mapped

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] [day] [month] [year] [list]

Message-ID: <fa4c886f-81d5-4679-bba3-4357f1d9c6ff@redhat.com>
Date: Tue, 22 Apr 2025 09:00:54 +0200
From: David Hildenbrand <david@...hat.com>
To: Lance Yang <lance.yang@...ux.dev>,
 Andrew Morton <akpm@...ux-foundation.org>
Cc: mingzhe.yang@...com, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 Lance Yang <ioworker0@...il.com>
Subject: Re: [RESEND PATCH v2 1/1] mm/rmap: add CONFIG_MM_ID guard for
 folio_test_large_maybe_mapped_shared()

On 22.04.25 06:35, Lance Yang wrote:
> April 22, 2025 at 3:22 AM, "Andrew Morton" <akpm@...ux-foundation.org> wrote:
> 
> 
> 
>>
>> On Mon, 21 Apr 2025 05:13:03 +0000 "Lance Yang" <lance.yang@...ux.dev> wrote:
>>
>>>
>>> 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.
>>>
>>>   
>>>
>>>   The reason we can't simply add #ifdef CONFIG_MM_ID around folio_test_large_maybe_mapped_shared()
>>>
>>>   is because its caller folio_maybe_mapped_shared() relies on IS_ENABLED(CONFIG_MM_ID).
>>>
>>>   
>>>
>>>   If we do, with CONFIG_TRANSPARENT_HUGEPAGE=N, we'll hit compilation errors like:
>>>
>>>   
>>>
>>>   ./include/linux/mm.h: In function ‘folio_maybe_mapped_shared’:
>>>
>>>   ./include/linux/mm.h:2337:16: error: implicit declaration of function ‘folio_test_large_maybe_mapped_shared’; did you mean ‘folio_maybe_mapped_shared’? [-Werror=implicit-function-declaration]
>>>
>>>   2337 | return folio_test_large_maybe_mapped_shared(folio);
>>>
>>>   | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>>   | folio_maybe_mapped_shared
>>>
>>>   cc1: some warnings being treated as errors
>>>
>>
>> That's OK - provide a declaration of folio_maybe_mapped_shared() but no
>>
>> definition. So the compiled-out code can be compiled and the linker
>>
>> will confirm that it's never actually called.
>>
> 
> Got it, that works as well ;)
> 
> So if David is cool with it, I'll send out the new version like this:
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index d3909cb1e576..a762e4b4eab4 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -1230,10 +1230,15 @@ static inline int folio_has_private(const struct folio *folio)
>   	return !!(folio->flags & PAGE_FLAGS_PRIVATE);
>   }
> 
> +#ifdef CONFIG_MM_ID
>   static inline bool folio_test_large_maybe_mapped_shared(const struct folio *folio)
>   {
>   	return test_bit(FOLIO_MM_IDS_SHARED_BITNUM, &folio->_mm_ids);
>   }
> +#else
> +bool folio_test_large_maybe_mapped_shared(const struct folio *folio);
> +#endif

Fine with me. At this point, I do prefer inlining the function, though.

-- 
Cheers,

David / dhildenb