Re: [linux-next:master 1753/1936] mm/swapfile.c:1517:20: warning: function 'swap_is_last_ref' is not needed and will not be emitted

[Baoquan He <bhe@redhat.com>]

On 04/09/25 at 09:54am, SeongJae Park wrote:
> Hello,
> 
> On Tue, 8 Apr 2025 17:39:59 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Tue, 8 Apr 2025 18:06:11 +0800 kernel test robot <lkp@intel.com> wrote:
> > 
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > > head:   7702d0130dc002bab2c3571ddb6ff68f82d99aea
> > > commit: 6bb001b6b64ec20d900d0d81294a0f031af59c3e [1753/1936] mm: swap: enable swap_entry_range_free() to drop any kind of last ref
> > > config: hexagon-randconfig-001-20250408 (https://download.01.org/0day-ci/archive/20250408/202504081841.jbL7yL0r-lkp@intel.com/config)
> > > compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 92c93f5286b9ff33f27ff694d2dc33da1c07afdd)
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250408/202504081841.jbL7yL0r-lkp@intel.com/reproduce)
> > > 

Hi lkp team,

When I tried to reproduce, I got below failure. Please help check what's
wrong with it. Thanks in advance.

[~]# COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang-21 ~/lkp-tests/kbuild/make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
Compiler will be installed in /root/0day
Cannot find clang-21 under https://cdn.kernel.org/pub/tools/llvm/files
Please set new URL env variable and rerun
  * crosstool provided by kernel org:
    export URL=https://cdn.kernel.org/pub/tools/llvm/files
  * crosstool provided by 0-Day CI:
    export URL=https://download.01.org/0day-ci/cross-package
clang crosstool install failed
Install clang compiler failed
setup_crosstool failed

> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202504081841.jbL7yL0r-lkp@intel.com/
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > > >> mm/swapfile.c:1517:20: warning: function 'swap_is_last_ref' is not needed and will not be emitted [-Wunneeded-internal-declaration]
> > 
> > It's annoying that gcc warns about this.
> > 
> > >     1517 | static inline bool swap_is_last_ref(unsigned char count)
> > >          |                    ^~~~~~~~~~~~~~~~
> > >    1 warning generated.
> > > 
> > > 
> > > vim +/swap_is_last_ref +1517 mm/swapfile.c
> > > 
> > >   1512	
> > >   1513	/*
> > >   1514	 * Drop the last ref(1, SWAP_HAS_CACHE or SWAP_MAP_SHMEM) of swap entries,
> > >   1515	 * caller have to ensure all entries belong to the same cgroup and cluster.
> > >   1516	 */
> > > > 1517	static inline bool swap_is_last_ref(unsigned char count)
> > >   1518	{
> > >   1519		return (count == SWAP_HAS_CACHE) || (count == 1) ||
> > >   1520		       (count == SWAP_MAP_SHMEM);
> > >   1521	}
> > >   1522	
> > 
> > : static inline bool swap_is_last_ref(unsigned char count)
> > : {
> > : 	return (count == SWAP_HAS_CACHE) || (count == 1) ||
> > : 	       (count == SWAP_MAP_SHMEM);
> > : }
> > : 
> > : static void swap_entries_free(struct swap_info_struct *si,
> > : 			      struct swap_cluster_info *ci,
> > : 			      swp_entry_t entry, unsigned int nr_pages)
> > : {
> > : 	unsigned long offset = swp_offset(entry);
> > : 	unsigned char *map = si->swap_map + offset;
> > : 	unsigned char *map_end = map + nr_pages;
> > : 
> > : 	/* It should never free entries across different clusters */
> > : 	VM_BUG_ON(ci != offset_to_cluster(si, offset + nr_pages - 1));
> > : 	VM_BUG_ON(cluster_is_empty(ci));
> > : 	VM_BUG_ON(ci->count < nr_pages);
> > : 
> > : 	ci->count -= nr_pages;
> > : 	do {
> > : 		VM_BUG_ON(!swap_is_last_ref(*map));
> > 
> > 
> > And it's annoying that VM_BUG_ON() doesn't reference its arg when
> > CONFIG_DEBUG_VM=n.
> > 
> > I'll add this:
> > 
> > --- a/mm/swapfile.c~mm-swap-enable-swap_entry_range_free-to-drop-any-kind-of-last-ref-fix
> > +++ a/mm/swapfile.c
> > @@ -1514,11 +1514,13 @@ fallback:
> >   * Drop the last ref(1, SWAP_HAS_CACHE or SWAP_MAP_SHMEM) of swap entries,
> >   * caller have to ensure all entries belong to the same cgroup and cluster.
> >   */
> > +#ifdef CONFIG_DEBUG_VM
> >  static inline bool swap_is_last_ref(unsigned char count)
> >  {
> >  	return (count == SWAP_HAS_CACHE) || (count == 1) ||
> >  	       (count == SWAP_MAP_SHMEM);
> >  }
> > +#endif
> >  
> >  static void swap_entries_free(struct swap_info_struct *si,
> >  			      struct swap_cluster_info *ci,
> > _
> 
> This makes my build fails when CONFIG_DEBUG_VM is unset as below.
> 
>     linux/mm/swapfile.c: In function ‘swap_entries_free’:
>     linux/mm/swapfile.c:1565:28: error: implicit declaration of function ‘swap_is_last_ref’; did you mean ‘swap_is_last_map’? [-Werror=implicit-function-declaration]
>      1565 |                 VM_BUG_ON(!swap_is_last_ref(*map));
>           |                            ^~~~~~~~~~~~~~~~
>     linux/include/linux/build_bug.h:30:63: note: in definition of macro ‘BUILD_BUG_ON_INVALID’
>        30 | #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
>           |                                                               ^
>     linux/mm/swapfile.c:1565:17: note: in expansion of macro ‘VM_BUG_ON’
>      1565 |                 VM_BUG_ON(!swap_is_last_ref(*map));
>           |                 ^~~~~~~~~
>     cc1: some warnings being treated as errors
> 
> Maybe we can open-code the condition, or add a macro implementation of
> swap_is_last_ref() for !CONFIG_DEBUG_VM case?

I think we shold do the similar thing as this commit does:
6e8e04291d81 mm/zsmalloc: add __maybe_unused attribute for is_first_zpdesc()

Just add a __maybe_unused attribute to mute the warning. I haven't
tested it because the reproducer failed in my side.

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 2887ed5d34cc..08b52346cc39 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1539,7 +1539,7 @@ static bool swap_entries_put_map_nr(struct swap_info_struct *si,
  * Drop the last ref(1, SWAP_HAS_CACHE or SWAP_MAP_SHMEM) of swap entries,
  * caller have to ensure all entries belong to the same cgroup and cluster.
  */
-static inline bool swap_is_last_ref(unsigned char count)
+static inline bool __maybe_unused swap_is_last_ref(unsigned char count)
 {
 	return (count == SWAP_HAS_CACHE) || (count == 1) ||
 	       (count == SWAP_MAP_SHMEM);