Re: [PATCH 20/28] mm, swap: check swap table directly for checking cache

[Kairui Song <ryncsn@gmail.com>]

On Thu, Jun 19, 2025 at 6:38 PM Baoquan He <bhe@redhat.com> wrote:
>
> On 05/15/25 at 04:17am, Kairui Song wrote:
> > From: Kairui Song <kasong@tencent.com>
> >
> > Instead of looking at the swap map, check swap table directly to tell if
> > a swap entry has cache. Prepare for remove SWAP_HAS_CACHE.
>
> But you actually check both the swap table entry and swap map entry in
> this patch, or do I miss anything?

Hi, Baoquan

>
> E.g
>
> if (!swap_count(si->swap_map[offset]) && swp_te_is_folio(swp_te))

Yes, the count info is still in the swap_map now, I'm only converting
the HAS_CACHE check to use swp_te_t here. We'll remove swap_map in
later patches and use the swp_te_t solely to get both info.

The reason some checks are added to check the swap_count is that:
Before this patch, `swap_map[offset] == SWAP_HAS_CACHE` implies the
count is zero too. So if HAS_CACHE is moved to swp_te_t, we still need
to check the count separately. The overhead will be gone very soon in
a later patch.

>
> >
> > Signed-off-by: Kairui Song <kasong@tencent.com>
> > ---
> >  mm/memory.c     | 12 +++++------
> >  mm/swap.h       |  6 ++++++
> >  mm/swap_state.c | 11 ++++++++++
> >  mm/swapfile.c   | 54 +++++++++++++++++++++++--------------------------
> >  4 files changed, 48 insertions(+), 35 deletions(-)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index a70624a55aa2..a9a548575e72 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -4314,15 +4314,15 @@ static struct folio *__alloc_swap_folio(struct vm_fault *vmf)
> >  }
> >
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > -static inline int non_swapcache_batch(swp_entry_t entry, int max_nr)
> > +static inline int non_swapcache_batch(swp_entry_t entry, unsigned int max_nr)
> >  {
> > -     struct swap_info_struct *si = swp_info(entry);
> > -     pgoff_t offset = swp_offset(entry);
> > -     int i;
> > +     unsigned int i;
> >
> >       for (i = 0; i < max_nr; i++) {
> > -             if ((si->swap_map[offset + i] & SWAP_HAS_CACHE))
> > -                     return i;
> > +             /* Page table lock pins the swap entries / swap device */
> > +             if (swap_cache_check_folio(entry))
> > +                     break;
> > +             entry.val++;
> >       }
> >
> >       return i;
> > diff --git a/mm/swap.h b/mm/swap.h
> > index 467996dafbae..2ae4624a0e48 100644
> > --- a/mm/swap.h
> > +++ b/mm/swap.h
> > @@ -186,6 +186,7 @@ static inline struct address_space *swap_address_space(swp_entry_t entry)
> >  extern struct folio *swap_cache_get_folio(swp_entry_t entry);
> >  extern struct folio *swap_cache_add_folio(swp_entry_t entry, struct folio *folio,
> >                                         void **shadow, bool swapin);
> > +extern bool swap_cache_check_folio(swp_entry_t entry);
> >  extern void *swap_cache_get_shadow(swp_entry_t entry);
> >  /* Below helpers requires the caller to lock the swap cluster. */
> >  extern void __swap_cache_del_folio(swp_entry_t entry,
> > @@ -395,6 +396,11 @@ static inline void *swap_cache_get_shadow(swp_entry_t end)
> >       return NULL;
> >  }
> >
> > +static inline bool swap_cache_check_folio(swp_entry_t entry)
> > +{
> > +     return false;
> > +}
> > +
> >  static inline unsigned int folio_swap_flags(struct folio *folio)
> >  {
> >       return 0;
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index c8bb16835612..ea6a1741db5c 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -266,6 +266,17 @@ struct folio *swap_cache_get_folio(swp_entry_t entry)
> >       return folio;
> >  }
> >
> > +/*
> > + * Check if a swap entry has folio cached, may return false positive.
> > + * Caller must hold a reference of the swap device or pin it in other ways.
> > + */
> > +bool swap_cache_check_folio(swp_entry_t entry)
> > +{
> > +     swp_te_t swp_te;
> > +     swp_te = __swap_table_get(swp_cluster(entry), swp_offset(entry));
> > +     return swp_te_is_folio(swp_te);
> > +}
> > +
> >  /*
> >   * If we are the only user, then try to free up the swap cache.
> >   *
> > diff --git a/mm/swapfile.c b/mm/swapfile.c
> > index ef233466725e..0f2a499ff2c9 100644
> > --- a/mm/swapfile.c
> > +++ b/mm/swapfile.c
> > @@ -181,15 +181,19 @@ static long swap_usage_in_pages(struct swap_info_struct *si)
> >  #define TTRS_FULL            0x4
> >
> >  static bool swap_only_has_cache(struct swap_info_struct *si,
> > -                           unsigned long offset, int nr_pages)
> > +                             struct swap_cluster_info *ci,
> > +                             unsigned long offset, int nr_pages)
> >  {
> >       unsigned char *map = si->swap_map + offset;
> >       unsigned char *map_end = map + nr_pages;
> > +     swp_te_t entry;
> >
> >       do {
> > +             entry = __swap_table_get(ci, offset);
>
> entry is not used in swap_only_has_cache() in this patch.

Thanks, it used in a later patch so I must move it here accidently
during a rebase, will defer this change to later patch.

>
> >               VM_BUG_ON(!(*map & SWAP_HAS_CACHE));
> > -             if (*map != SWAP_HAS_CACHE)
> > +             if (*map)
> >                       return false;
> > +             offset++;
> >       } while (++map < map_end);
> >
> >       return true;
> ......snip...
>
>