Re: [PATCH] mm/rmap: Always inline __folio_rmap_sanity_checks()

[David Hildenbrand <david@redhat.com>]

On 15.08.25 07:16, Lorenzo Stoakes wrote:
> On Thu, Aug 14, 2025 at 01:05:22PM -0700, Nathan Chancellor wrote:
>> Commit 5e901e249ad1 ("mm/rmap: convert "enum rmap_level" to "enum
>> pgtable_level"") changed VM_WARN_ON_ONCE, a run time warning, into
>> BUILD_BUG, a compile time error. After this adjustment, certain builds
>> with older versions of clang (such as arm64 allmodconfig) started
>> failing to build with:
>>
>>    In file included from mm/rmap.c:63:
>>    In file included from include/linux/ksm.h:14:
>>    include/linux/rmap.h:440:3: error: call to __compiletime_assert_890 declared with 'error' attribute: BUILD_BUG failed
>>                    BUILD_BUG();
>>                    ^
>>    ...
>>    <scratch space>:21:1: note: expanded from here
>>    __compiletime_assert_890
>>    ^
>>
>> While __folio_rmap_sanity_checks() is marked 'inline', the compiler may
>> not always honor it, such as when sanitizers or other instrumentation is
>> enabled.  If __folio_rmap_sanity_checks() is not inlined, there is no
>> way the compiler can eliminate the default cause.
>>
>> Mark __folio_rmap_sanity_checks() as __always_inline to allow the
>> BUILD_BUG() to work consistently, which clears up the error.

Thanks for reporting. Weird that no build bug stumbled over that so far 
(neither on the list nor on my private trees).

Maybe the latest change from Jann made the function bigger and the 
compiler more likely to not inline this function.

>>
>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> David needs to have a look, but afaict this seems reasonable,
> __folio_rmap-sanity_check() is only ever doing stuff if CONFIG_DEBUG_VM is
> set, and in that situation we don't even care if this would somehow be
> inefficient, but in any case modern compilers would inline it anyway.

Yes, all code is written such that we get specialized variants without 
runtime checks -- except __folio_rmap_sanity_checks() because that would
usually just get completely compiled out.

We could either have a WARN_ON_ONCE() instead of the BUILD_BUG() or, 
rally just __always_inline this function.

> 
> So LGTM and:
> 
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> 
>> ---
>> I assume this will be squashed into
>> mm-rmap-convert-enum-rmap_level-to-enum-pgtable_level.patch so no fixes
>> tag.

Yes, @Andrew please squash.

In any case

Acked-by: David Hildenbrand <david@redhat.com>

Thanks!

-- 
Cheers

David / dhildenb