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

[SeongJae Park <sj@kernel.org>]

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)
> > 
> > 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?


Thanks,
SJ

[...]