Re: [PATCH v2] mm/swapops,rmap: remove should-never-be-compiled codes.

["David Hildenbrand (Red Hat)" <david@kernel.org>]

On 12/30/25 22:35, Andrew Morton wrote:
> On Wed, 31 Dec 2025 10:28:25 +1300 Barry Song <21cnbao@gmail.com> wrote:
> 
>> On Wed, Dec 31, 2025 at 8:50 AM David Hildenbrand (Red Hat)
>> <david@kernel.org> wrote:
>>>
>>>
>>>> -                     break;
>>>> +#ifdef CONFIG_NO_PAGE_MAPCOUNT
>>>> +             last = atomic_add_negative(-1, &folio->_entire_mapcount);
>>>> +             if (level == PGTABLE_LEVEL_PMD && last)
>>>> +                     nr_pmdmapped = folio_large_nr_pages(folio);
>>>> +             nr = folio_dec_return_large_mapcount(folio, vma);
>>>> +             if (!nr) {
>>>> +                     /* Now completely unmapped. */
>>>> +                     nr = folio_large_nr_pages(folio);
>>>> +             } else {
>>>> +                     partially_mapped = last &&
>>>> +                             nr < folio_large_nr_pages(folio);
>>>> +                     nr = 0;
>>>
>>> The whole code was written to avoid ifdefs.
>>
>> The #ifdefs are only for -O0 builds, which are never a
>> mainline requirement. However, dropping functions that
>> contain nothing but a BUILD_BUG() seems reasonable?
> 
> Let's not make -O0 an objective, please.  We often make assumptions
> about dead code elimination in order to keep kernel code more pleasing
> to read and to maintain.
> 
>> If those functions were actually required, the compiler
>> would fail anyway. In that case, there doesn’t seem to be
>> much value in keeping wrapper functions whose sole purpose
>> is to contain a single BUILD_BUG().
> 
> I agree with that part of the patch - if the thing only does a
> BUILD_BUG() then let's simply remove it.  If somehow the compiler tries
> to reference the now-not-present function then we'll get an error message
> anyway.
> 

The change

	+#ifdef CONFIG_NO_PAGE_MAPCOUNT
	+#define folio_inc_return_large_mapcount(folio, vma) \
	+	folio_add_return_large_mapcount(folio, 1, vma)
	 #define folio_dec_return_large_mapcount(folio, vma) \
	 	folio_sub_return_large_mapcount(folio, 1, vma)
	+#endif

In the patch is wrong. It must be CONFIG_MM_ID.



So if the following makes compilers happy, fine with me:

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index daa92a58585d9..9d782826e8579 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -336,6 +336,11 @@ static __always_inline int folio_sub_return_large_mapcount(struct folio *folio,
         return new_mapcount_val + 1;
  }
  #define folio_sub_large_mapcount folio_sub_return_large_mapcount
+
+#define folio_inc_return_large_mapcount(folio, vma) \
+       folio_add_return_large_mapcount(folio, 1, vma)
+#define folio_dec_return_large_mapcount(folio, vma) \
+       folio_sub_return_large_mapcount(folio, 1, vma)
  #else /* !CONFIG_MM_ID */
  /*
   * See __folio_rmap_sanity_checks(), we might map large folios even without
@@ -354,33 +359,17 @@ static inline void folio_add_large_mapcount(struct folio *folio,
         atomic_add(diff, &folio->_large_mapcount);
  }
  
-static inline int folio_add_return_large_mapcount(struct folio *folio,
-               int diff, struct vm_area_struct *vma)
-{
-       BUILD_BUG();
-}
-
  static inline void folio_sub_large_mapcount(struct folio *folio,
                 int diff, struct vm_area_struct *vma)
  {
         atomic_sub(diff, &folio->_large_mapcount);
  }
-
-static inline int folio_sub_return_large_mapcount(struct folio *folio,
-               int diff, struct vm_area_struct *vma)
-{
-       BUILD_BUG();
-}
  #endif /* CONFIG_MM_ID */
  
  #define folio_inc_large_mapcount(folio, vma) \
         folio_add_large_mapcount(folio, 1, vma)
-#define folio_inc_return_large_mapcount(folio, vma) \
-       folio_add_return_large_mapcount(folio, 1, vma)
  #define folio_dec_large_mapcount(folio, vma) \
         folio_sub_large_mapcount(folio, 1, vma)
-#define folio_dec_return_large_mapcount(folio, vma) \
-       folio_sub_return_large_mapcount(folio, 1, vma)
  
  /* RMAP flags, currently only relevant for some anon rmap operations. */
  typedef int __bitwise rmap_t;



Use a config with !CONFIG_TRANSPARENT_HUGEPAGE:

  $ LANG=C make mm/rmap.o
   CALL    scripts/checksyscalls.sh
   DESCEND objtool
   INSTALL libsubcmd_headers
   DESCEND bpf/resolve_btfids
   INSTALL libsubcmd_headers
   CC      mm/rmap.o
mm/rmap.c: In function '__folio_add_rmap':
mm/rmap.c:1249:30: error: implicit declaration of function 'folio_add_return_large_mapcount'; did you mean 'folio_add_large_mapcount'? [-Wimplicit-function-declaration]
  1249 |                         nr = folio_add_return_large_mapcount(folio, orig_nr_pages, vma);
       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       |                              folio_add_large_mapcount
mm/rmap.c:1274:30: error: implicit declaration of function 'folio_inc_return_large_mapcount'; did you mean 'folio_inc_large_mapcount'? [-Wimplicit-function-declaration]
  1274 |                         nr = folio_inc_return_large_mapcount(folio, vma);
       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       |                              folio_inc_large_mapcount
mm/rmap.c: In function '__folio_remove_rmap':
mm/rmap.c:1673:30: error: implicit declaration of function 'folio_sub_return_large_mapcount'; did you mean 'folio_sub_large_mapcount'? [-Wimplicit-function-declaration]
  1673 |                         nr = folio_sub_return_large_mapcount(folio, nr_pages, vma);
       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       |                              folio_sub_large_mapcount
mm/rmap.c:1702:30: error: implicit declaration of function 'folio_dec_return_large_mapcount'; did you mean 'folio_dec_large_mapcount'? [-Wimplicit-function-declaration]
  1702 |                         nr = folio_dec_return_large_mapcount(folio, vma);
       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       |                              folio_dec_large_mapcount
make[3]: *** [scripts/Makefile.build:287: mm/rmap.o] Error 1
make[2]: *** [scripts/Makefile.build:556: mm] Error 2
make[1]: *** [/home/dhildenb/git/linux/Makefile:2054: .] Error 2
make: *** [Makefile:248: __sub-make] Error 2



-- 
Cheers

David