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

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

[Nathan Chancellor]

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.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
I assume this will be squashed into
mm-rmap-convert-enum-rmap_level-to-enum-pgtable_level.patch so no fixes
tag.
---
 include/linux/rmap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 9d40d127bdb7..e8aff6d2deda 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -394,7 +394,7 @@ typedef int __bitwise rmap_t;
 /* The anonymous (sub)page is exclusive to a single process. */
 #define RMAP_EXCLUSIVE		((__force rmap_t)BIT(0))
 
-static inline void __folio_rmap_sanity_checks(const struct folio *folio,
+static __always_inline void __folio_rmap_sanity_checks(const struct folio *folio,
 		const struct page *page, int nr_pages, enum pgtable_level level)
 {
 	/* hugetlb folios are handled separately. */

---
base-commit: 6bee0462de1f9f4fa9400f153d3b0792c20d7111
change-id: 20250814-rmap-fix-build_bug-conversion-90441c036f42

Best regards,
--  
Nathan Chancellor <nathan@kernel.org>

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

[Lorenzo Stoakes]

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

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.
> ---
>  include/linux/rmap.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> index 9d40d127bdb7..e8aff6d2deda 100644
> --- a/include/linux/rmap.h
> +++ b/include/linux/rmap.h
> @@ -394,7 +394,7 @@ typedef int __bitwise rmap_t;
>  /* The anonymous (sub)page is exclusive to a single process. */
>  #define RMAP_EXCLUSIVE		((__force rmap_t)BIT(0))
>
> -static inline void __folio_rmap_sanity_checks(const struct folio *folio,
> +static __always_inline void __folio_rmap_sanity_checks(const struct folio *folio,
>  		const struct page *page, int nr_pages, enum pgtable_level level)
>  {
>  	/* hugetlb folios are handled separately. */
>
> ---
> base-commit: 6bee0462de1f9f4fa9400f153d3b0792c20d7111
> change-id: 20250814-rmap-fix-build_bug-conversion-90441c036f42
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>

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

[David Hildenbrand]

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