Re: [PATCH v3 11/15] mm, swap: use the swap table for the swap cache and switch API

[Kairui Song <ryncsn@gmail.com>]

On Fri, Sep 12, 2025 at 5:31 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Thu, Sep 11, 2025 at 10:54:19AM +0800, Lance Yang wrote:
> >
> >
> > On 2025/9/11 10:48, Kairui Song wrote:
> > > On Thu, Sep 11, 2025 at 10:34 AM Lance Yang <lance.yang@linux.dev> wrote:
> > > >
> > > > On Thu, Sep 11, 2025 at 10:27 AM Lance Yang <lance.yang@linux.dev> wrote:
> > > > >
> > > > > Hi Kairui,
> > > > >
> > > > > I'm hitting a build error with allnoconfig:
> > > > >
> > > > > In file included from mm/shmem.c:44:
> > > > > mm/swap.h: In function ‘folio_index’:
> > > > > mm/swap.h:462:24: error: implicit declaration of function
> > > > > ‘swp_offset’; did you mean ‘pmd_offset’?
> > > > > [-Wimplicit-function-declaration]
> > > > > 462 | return swp_offset(folio->swap);
> > > > >
> > > > > It looks like a header might be missing in mm/swap.h. Please let me know
> > > > > if you need any more information.
> > > >
> > > > Confirmed that just adding #include <linux/swapops.h> into mm/swap.h fixes it.
> > > >
> > > > diff --git a/mm/swap.h b/mm/swap.h
> > > > index ad339547ee8c..271e8c560fcc 100644
> > > > --- a/mm/swap.h
> > > > +++ b/mm/swap.h
> > > > @@ -3,6 +3,7 @@
> > > >   #define _MM_SWAP_H
> > > >
> > > >   #include <linux/atomic.h> /* for atomic_long_t */
> > > > +#include <linux/swapops.h>
> > > >   struct mempolicy;
> > > >   struct swap_iocb;
> > > >
> > > > Cheers,
> > > > Lance
> > > >
> > >
> > > Hi Lance,
> > >
> > > You are testing V2 not V3 right? The build error is in V2 and I can
> > > confirm that. But In V3 I've added "ifdef CONFIG_SWAP" for the
> > > swp_offset usage in swap.h. I've just tested allnoconfig and it
> > > works fine on aarch64 and x86_64.
> > >
> > > V2: https://lore.kernel.org/linux-mm/20250905191357.78298-12-ryncsn@gmail.com/
> >
> >
> > Ah, I was testing V2. My apologies for the noise!
> >
>
> OK so my bug is different then :)
>
> I think the issue is that you're not properly checking for:
>
> #ifdef CONFIG_MMU
> ...
> #endif
>
> This is v3 (I see the #ifdef CONFIG_SWAP) btw.
>
> So swapops.h is wrapped by #ifdef CONFIG_MMU which is why the declaration is
> missing.
>
> The below fixpatch fixes things for me.
>
> Cheers, Lorenzo
>
> ----8<----
> From 5a3969a438af9c33422a45fe813a44068d784b2f Mon Sep 17 00:00:00 2001
> From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Date: Fri, 12 Sep 2025 10:28:40 +0100
> Subject: [PATCH] fix
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
>  arch/sparc/include/asm/pgtable_64.h |  3 +++
>  mm/swap.h                           | 37 +++++++++++++++++------------
>  2 files changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
> index b8000ce4b59f..b06f55915653 100644
> --- a/arch/sparc/include/asm/pgtable_64.h
> +++ b/arch/sparc/include/asm/pgtable_64.h
> @@ -1050,6 +1050,9 @@ int page_in_phys_avail(unsigned long paddr);
>
>  int remap_pfn_range(struct vm_area_struct *, unsigned long, unsigned long,
>                    unsigned long, pgprot_t);
> +void remap_pfn_range_prepare(struct vm_area_desc *desc, unsigned long pfn);
> +int remap_pfn_range_complete(struct vm_area_struct *vma, unsigned long addr,
> +               unsigned long pfn, unsigned long size, pgprot_t pgprot);
>
>  void adi_restore_tags(struct mm_struct *mm, struct vm_area_struct *vma,
>                      unsigned long addr, pte_t pte);
> diff --git a/mm/swap.h b/mm/swap.h
> index caff4fe30fc5..18651687fcd1 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -53,7 +53,7 @@ enum swap_cluster_flags {
>         CLUSTER_FLAG_MAX,
>  };
>
> -#ifdef CONFIG_SWAP
> +#if defined(CONFIG_SWAP) && defined(CONFIG_MMU)
>  #include <linux/swapops.h> /* for swp_offset */
>  #include <linux/blk_types.h> /* for bio_end_io_t */
>
> @@ -317,7 +317,26 @@ static inline int non_swapcache_batch(swp_entry_t entry, int max_nr)
>         return i;
>  }
>
> +/**
> + * folio_index - File index of a folio.
> + * @folio: The folio.
> + *
> + * For a folio which is either in the page cache or the swap cache,
> + * return its index within the address_space it belongs to.  If you know
> + * the folio is definitely in the page cache, you can look at the folio's
> + * index directly.
> + *
> + * Return: The index (offset in units of pages) of a folio in its file.
> + */
> +static inline pgoff_t folio_index(struct folio *folio)
> +{
> +       if (unlikely(folio_test_swapcache(folio)))
> +               return swp_offset(folio->swap);
> +       return folio->index;
> +}
> +
>  #else /* CONFIG_SWAP */
> +
>  struct swap_iocb;
>  static inline struct swap_cluster_info *swap_cluster_lock(
>         struct swap_info_struct *si, pgoff_t offset, bool irq)
> @@ -442,24 +461,12 @@ static inline int non_swapcache_batch(swp_entry_t entry, int max_nr)
>  {
>         return 0;
>  }
> -#endif /* CONFIG_SWAP */
>
> -/**
> - * folio_index - File index of a folio.
> - * @folio: The folio.
> - *
> - * For a folio which is either in the page cache or the swap cache,
> - * return its index within the address_space it belongs to.  If you know
> - * the folio is definitely in the page cache, you can look at the folio's
> - * index directly.
> - *
> - * Return: The index (offset in units of pages) of a folio in its file.
> - */
>  static inline pgoff_t folio_index(struct folio *folio)
>  {
> -       if (unlikely(folio_test_swapcache(folio)))
> -               return swp_offset(folio->swap);
>         return folio->index;
>  }
>
> +#endif
> +
>  #endif /* _MM_SWAP_H */
> --
> 2.51.0

Thanks for the info. But I think V3 should be good?

CONFIG_SWAP depends on CONFIG_MMU so if you have !CONFIG_MMU, you must
have !CONFIG_SWAP.

I didn't adjust the header in V3 and fixed it differently from what
Lance is suggesting, V3 wraps swp_offset with CONFIG_SWAP. It should
covers !CONFIG_MMU case too.

The folio_index in V3 (this patch) looks like this:

static inline pgoff_t folio_index(struct folio *folio)
{
#ifdef CONFIG_SWAP
        if (unlikely(folio_test_swapcache(folio)))
               return swp_offset(folio->swap);
#endif
        return folio->index;
}

I just tested !MMU and it seems fine.