linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [linux-next:master 1753/1936] mm/swapfile.c:1517:20: warning: function 'swap_is_last_ref' is not needed and will not be emitted
@ 2025-04-08 10:06 kernel test robot
  2025-04-09  0:39 ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: kernel test robot @ 2025-04-08 10:06 UTC (permalink / raw)
  To: Kemeng Shi
  Cc: llvm, oe-kbuild-all, Andrew Morton, Linux Memory Management List,
	Tim Chen, Baoquan He

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]
    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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-08 10:06 [linux-next:master 1753/1936] mm/swapfile.c:1517:20: warning: function 'swap_is_last_ref' is not needed and will not be emitted kernel test robot
@ 2025-04-09  0:39 ` Andrew Morton
  2025-04-09  2:24   ` Baoquan He
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Andrew Morton @ 2025-04-09  0:39 UTC (permalink / raw)
  To: kernel test robot
  Cc: Kemeng Shi, llvm, oe-kbuild-all, Linux Memory Management List,
	Tim Chen, Baoquan He

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.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-09  0:39 ` Andrew Morton
@ 2025-04-09  2:24   ` Baoquan He
  2025-04-09  6:25   ` Kemeng Shi
  2025-04-09 16:54   ` SeongJae Park
  2 siblings, 0 replies; 10+ messages in thread
From: Baoquan He @ 2025-04-09  2:24 UTC (permalink / raw)
  To: Andrew Morton, Kemeng Shi
  Cc: kernel test robot, llvm, oe-kbuild-all,
	Linux Memory Management List, Tim Chen

On 04/08/25 at 05:39pm, Andrew Morton 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,
> _
> 
> 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.

Indeed, maybe below change can be squashed into above patch from Andrew.

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 2887ed5d34cc..baaf8ad29a6b 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1536,8 +1536,8 @@ 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.
+ * Check if it's the last ref of swap entry in the freeing path.
+ * Qualified vlaue includes 1, SWAP_HAS_CACHE or SWAP_MAP_SHMEM.
  */
 static inline bool swap_is_last_ref(unsigned char count)
 {



^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-09  0:39 ` Andrew Morton
  2025-04-09  2:24   ` Baoquan He
@ 2025-04-09  6:25   ` Kemeng Shi
  2025-04-09 16:54   ` SeongJae Park
  2 siblings, 0 replies; 10+ messages in thread
From: Kemeng Shi @ 2025-04-09  6:25 UTC (permalink / raw)
  To: Andrew Morton, kernel test robot
  Cc: llvm, oe-kbuild-all, Linux Memory Management List, Tim Chen, Baoquan He



on 4/9/2025 8:39 AM, Andrew Morton 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:
Thanks for fixing 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.
> 
The comment was originally for swap_entries_free() where swap_is_last_ref()
is factored out from. The swap_is_last_ref() is only used in swap_entries_free(),
so I treat these two functions as a single unit and placed the comment in front
of swap_is_last_ref().
If this introduces confusion, I think we can simply move this comment to front of
swap_entries_free() to address this.

Thanks,
Kemeng



^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-09  0:39 ` Andrew Morton
  2025-04-09  2:24   ` Baoquan He
  2025-04-09  6:25   ` Kemeng Shi
@ 2025-04-09 16:54   ` SeongJae Park
  2025-04-09 22:45     ` Baoquan He
  2025-04-10  0:42     ` Andrew Morton
  2 siblings, 2 replies; 10+ messages in thread
From: SeongJae Park @ 2025-04-09 16:54 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SeongJae Park, kernel test robot, Kemeng Shi, llvm,
	oe-kbuild-all, Linux Memory Management List, Tim Chen,
	Baoquan He

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

[...]


^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-09 16:54   ` SeongJae Park
@ 2025-04-09 22:45     ` Baoquan He
  2025-04-10  0:26       ` SeongJae Park
  2025-04-13  2:47       ` Li, Philip
  2025-04-10  0:42     ` Andrew Morton
  1 sibling, 2 replies; 10+ messages in thread
From: Baoquan He @ 2025-04-09 22:45 UTC (permalink / raw)
  To: SeongJae Park, kernel test robot
  Cc: Andrew Morton, Kemeng Shi, llvm, oe-kbuild-all,
	Linux Memory Management List, Tim Chen

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-09 22:45     ` Baoquan He
@ 2025-04-10  0:26       ` SeongJae Park
  2025-04-13  2:47       ` Li, Philip
  1 sibling, 0 replies; 10+ messages in thread
From: SeongJae Park @ 2025-04-10  0:26 UTC (permalink / raw)
  To: Baoquan He
  Cc: SeongJae Park, kernel test robot, Andrew Morton, Kemeng Shi,
	llvm, oe-kbuild-all, Linux Memory Management List, Tim Chen

Hi Baoquan,

On Thu, 10 Apr 2025 06:45:45 +0800 Baoquan He <bhe@redhat.com> wrote:

[...]
> 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);

I confirmed that this fixes my !CONFIG_DEBUG_VM case build failure on the
latest mm-new.  Thank you for quickly sharing your thought and this fix.


Thanks,
SJ


^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-09 16:54   ` SeongJae Park
  2025-04-09 22:45     ` Baoquan He
@ 2025-04-10  0:42     ` Andrew Morton
  2025-04-10  6:34       ` Kemeng Shi
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2025-04-10  0:42 UTC (permalink / raw)
  To: SeongJae Park
  Cc: kernel test robot, Kemeng Shi, llvm, oe-kbuild-all,
	Linux Memory Management List, Tim Chen, Baoquan He

On Wed,  9 Apr 2025 09:54:52 -0700 SeongJae Park <sj@kernel.org> wrote:

> > I'll add this:
> > 
> 
> ...
>
> This makes my build fails when CONFIG_DEBUG_VM is unset as below.

Uh, OK.  I'll leave this to Kemeng to address, please.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-10  0:42     ` Andrew Morton
@ 2025-04-10  6:34       ` Kemeng Shi
  0 siblings, 0 replies; 10+ messages in thread
From: Kemeng Shi @ 2025-04-10  6:34 UTC (permalink / raw)
  To: Andrew Morton, SeongJae Park
  Cc: kernel test robot, llvm, oe-kbuild-all,
	Linux Memory Management List, Tim Chen, Baoquan He



on 4/10/2025 8:42 AM, Andrew Morton wrote:
> On Wed,  9 Apr 2025 09:54:52 -0700 SeongJae Park <sj@kernel.org> wrote:
> 
>>> I'll add this:
>>>
>>
>> ...
>>
>> This makes my build fails when CONFIG_DEBUG_VM is unset as below.
> 
> Uh, OK.  I'll leave this to Kemeng to address, please.
> 
Sure. Thanks to Baoquan and SeongJae for providing the solution and
testing it. I will address build warning by adding __maybe_unused
as Baoquan menthioned and update confusion comment before
swap_is_last_ref().

Thanks,
Kemeng



^ permalink raw reply	[flat|nested] 10+ messages in thread

* 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
  2025-04-09 22:45     ` Baoquan He
  2025-04-10  0:26       ` SeongJae Park
@ 2025-04-13  2:47       ` Li, Philip
  1 sibling, 0 replies; 10+ messages in thread
From: Li, Philip @ 2025-04-13  2:47 UTC (permalink / raw)
  To: Baoquan He
  Cc: Andrew Morton, Kemeng Shi, llvm, oe-kbuild-all,
	Linux Memory Management List, Tim Chen, SeongJae Park, lkp


> Subject: 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
> 
> 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,

Hi Baoquan, sorry for late reply. This is an issue in the bot that when clang-21 is not yet released. Early
on we tried to add a reminder like below (suggested by another similar case).

	# clang-21 might not be officially released. Try a lower version if installation fails.

We will add this reminder asap. Thus when clang-21 fails to install, clang-20 could be an alternative
to reproduce.

Sorry for inconvenience.

Thanks

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-04-13  2:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-08 10:06 [linux-next:master 1753/1936] mm/swapfile.c:1517:20: warning: function 'swap_is_last_ref' is not needed and will not be emitted kernel test robot
2025-04-09  0:39 ` Andrew Morton
2025-04-09  2:24   ` Baoquan He
2025-04-09  6:25   ` Kemeng Shi
2025-04-09 16:54   ` SeongJae Park
2025-04-09 22:45     ` Baoquan He
2025-04-10  0:26       ` SeongJae Park
2025-04-13  2:47       ` Li, Philip
2025-04-10  0:42     ` Andrew Morton
2025-04-10  6:34       ` Kemeng Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox