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

[Andrew Morton <akpm@linux-foundation.org>]

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,
_

Kemeng, can you please review that comment "Drop the last ref...". 
From its positioning it appears to be describing swap_is_last_ref(),
but swap_is_last_ref() doesn't do those things.