Re: [PATCH v3] mm: bypass swap readahead for zswap

[Barry Song]

On Mon, Nov 2, 2026 at 3:55 PM Alexandre Ghiti <alexghiti@xxxxxxxx> wrote:
>
> Hi Barry,
>
> I'm back from holidays, sorry for the late answer.
>
> On Fri, Oct 23, 2026 at 11:16 PM Barry Song (Xiaomi) <baohua@xxxxxxxxxx> wrote:
> >
> > >
> > On Wed, Oct 7, 2026 at 11:39 PM Alexandre Ghiti <alex@xxxxxxxx> wrote:
> > >
> > > Commit 0bcac06f27d7 ("mm, swap: skip swapcache for swapin of synchronous
> > > device") made SWP_SYNCHRONOUS_IO devices (e.g. zram) skip swap readahead.
> > >
> > > zswap is the same kind of in-memory, synchronous backend as zram, not a
> > > swap device flagged SWP_SYNCHRONOUS_IO so it still goes through
> > > swapin_readahead().
> > >
> > > Here are the results from bypassing readahead for zswap too: it was
> > > measured with a kernel build (make -j16) in a memcg, zswap=zstd, shrinker
> > > off, on Sapphire Rapids and 3 iterations.
> > >
> > >   768M memcg (sustained swap thrash):
> > >     metric                 mm-new    + bypass    delta
> > >     build time (s)          405.0       341.7    -15.6%
> > >     zswap-in (GB)            79.5        53.0     -33%
> > >     zswap-out (GB)          144.8       115.6     -20%
> > >     swap readahead (pages)  6.79M       0.45M     -93%
> > >     swap_ra hit (%)          72.1        89.9     +18pp
> > >
> > >   1G memcg (light pressure, build not memory-bound):
> > >     metric                 mm-new    + bypass    delta
> > >     build time (s)          177.7       176.0    ~same (no regression)
> > >     zswap-in (GB)            10.2         7.5     -26%
> > >     zswap-out (GB)           27.7        25.1      -9%
> > >     swap readahead (pages)  1.07M       0.08M     -93%
> > >     swap_ra hit (%)          68.6        87.2     +19pp
> > >
> > > Similar gains were observed on an AMD EPYC 7D13 host.
> > >
> > > The gain is from no longer prefetching pages that are pointless for an
> > > in-memory backend: readahead inflates anon residency and thrashes the
> > > page cache (file pages get evicted and re-read), lengthens each fault by
> > > synchronously (de)compressing a cluster of neighbours, and adds
> > > compression traffic when those extra pages are reclaimed.
> > >
> > > Bypassing swap readahead for zswap therefore makes sense.
> > >
> > > Suggested-by: Usama Arif <usama.arif@xxxxxxxxx>
> > > Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
> > > ---
> >
> > Hi Alexandre,
> >
> > One thing on my mind is that for synchronous devices, we always free
> > the swapcache during swap-in, thus releasing the compressed copy and
> > metadata as well. We could do something similar when a zswap entry
> > is found.
> >
> > I wonder if we should do that as well. I don't have time to
> > investigate this further myself, but I wonder if you'd be interested
> > in looking into it.
> >
> >     mm: free swapcache on zswap hits in do_swap_page()
> >
> >     We already do this for zram and other synchronous devices. The same
> >     should apply to zswap hits. Freeing the swapcache will allow the
> >     compressed copy and metadata to be released earlier.
> >
> >     Suggested-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 55b5a84d1016..d22c779fda93 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -4711,17 +4711,18 @@ static inline bool should_try_to_free_swap(struct swap_info_struct *si,
> >                                            struct folio *folio,
> >                                            struct vm_area_struct *vma,
> >                                            bool exclusive,
> > -                                          unsigned int fault_flags)
> > +                                          unsigned int fault_flags,
> > +                                          bool sync_swap)
> >  {
> >         if (!folio_test_swapcache(folio))
> >                 return false;
> >         /*
> > -        * Always try to free swap cache for SWP_SYNCHRONOUS_IO devices. Swap
> > +        * Always try to free swap cache for synchronize swap entries. Swap
> >          * cache can help save some IO or memory overhead, but these devices
> >          * are fast, and meanwhile, swap cache pinning the slot deferring the
> >          * release of metadata or fragmentation is a more critical issue.
> >          */
> > -       if (data_race(si->flags & SWP_SYNCHRONOUS_IO))
> > +       if (sync_swap)
> >                 return true;
> >         if (mem_cgroup_swap_full(folio) || (vma->vm_flags & VM_LOCKED) ||
> >             folio_test_mlocked(folio))
> > @@ -4977,6 +4978,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >         rmap_t rmap_flags = RMAP_NONE;
> >         bool exclusive = false;
> >         bool rwp_restore = false;
> > +       bool sync_swap = false;
> >         softleaf_t entry;
> >         pte_t pte;
> >         vm_fault_t ret = 0;
> > @@ -5059,7 +5061,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >                 swap_update_readahead(folio, vma, vmf->address);
> >         if (!folio) {
> >                 /* Swapin bypasses readahead for SWP_SYNCHRONOUS_IO devices and zswap */
> > -               if (swap_entry_synchronous(si, entry))
> > +               sync_swap = swap_entry_synchronous(si, entry);
> > +               if (sync_swap)
> >                         folio = swapin_sync(entry, GFP_HIGHUSER_MOVABLE,
> >                                             thp_swapin_suitable_orders(vmf) | BIT(0),
> >                                             vmf, NULL, 0);
> > @@ -5325,7 +5328,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >          * Do it after mapping, so raced page faults will likely see the folio
> >          * in swap cache and wait on the folio lock.
> >          */
> > -       if (should_try_to_free_swap(si, folio, vma, exclusive, vmf->flags))
> > +       if (should_try_to_free_swap(si, folio, vma, exclusive, vmf->flags, sync_swap))
> >                 folio_free_swap(folio);
> >
> >         folio_unlock(folio);
> >
>
> zswap already gets rid of the compressed data but indeed keeps the
> metadata on swapin, we discussed that with Nhat 2 weeks ago :)

Oh, yes, I missed this.
        /*
         * We are reading into the swapcache, invalidate zswap entry.
         * The swapcache is the authoritative owner of the page and
         * its mappings, and the pressure that results from having two
         * in-memory copies outweighs any benefits of caching the
         * compression work.
         */
        folio_mark_dirty(folio);
        xa_erase(tree, offset);
        zswap_entry_free(entry);

For zram, it's a different story. It depends on
the final swapcache release and
swap_slot_free_notify().

I suspect we could also do this by freeing the compressed
copy immediately while the zram read is in progress. However,
since zram is a block device and can also be accessed
directly through its block device node, this might somehow
break the expected semantics, although it could benefit
performance.

>
> I have a different patch for that: zswap marks a swapped-in entry as
> dirty (so it gets swapped-out again) so I was thinking we could use
> this property to free swapcache metadata in that case. I'm pasting the
> diff here so you can tell me what you think. Anyway, to answer your
> question, yes I'm taking this over but before I send that upstream, I
> need to test/benchmark this approach. It relates to
> https://lore.kernel.org/linux-mm/20261022145615.2184343-1-alex@xxxxxxxx/
> where poking holes in the swap table currently results in inefficient
> IO grouping.

I'm not quite sure we want to free the swapcache metadata, as
it could also be useful, for example, by avoiding the cost
of allocating a swap slot and adding the folio to the
swapcache when it is swapped out again.

So I am really not sure if it is correct though.

>
> diff --git a/mm/internal.h b/mm/internal.h
> index 2112576a70a2..0c4598326d2f 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -626,10 +626,13 @@ static inline bool
> should_try_to_free_swap(struct swap_info_struct *si,
>                 return true;
>
>         /*
> -        * Free the swapcache only if we are the exclusive user and
> -        * this is a write fault.
> +        * Free the swapcache only if we are the exclusive user and this is a
> +        * write fault, or the folio is dirty: its copy in the swap slot is
> +        * stale then (zswap loads are exclusive and dirty the folio), so the
> +        * slot only holds swap space.
>          */
> -       return (fault_flags & FAULT_FLAG_WRITE) && exclusive;
> +       return exclusive && ((fault_flags & FAULT_FLAG_WRITE) ||
> +                            folio_test_dirty(folio));
>  }
>
>  void folio_rotate_reclaimable(struct folio *folio);

Best,
Barry