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