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

[wale zhang <wale.zhang.ftd@gmail.com>]

On Wed, Dec 31, 2025 at 5:59 AM David Hildenbrand (Red Hat)
<david@kernel.org> wrote:
>
> 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.

Hello David,

folio_inc_return_large_mapcount and folio_dec_return_large_mapcount
are only used when CONFIG_NO_PAGE_MAPCOUNT is defined.

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

Apart from the solution as below, I can't think of any other ways. Do
you have any good ideas?
+#ifdef CONFIG_NO_PAGE_MAPCOUNT
-               if (IS_ENABLED(CONFIG_NO_PAGE_MAPCOUNT)) {

> 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

Thanks
Wale