Re: [PATCH v2 1/1] mm, swap: only prefer nonfull clusters on synchronous devices

[Alexandre Ghiti]

Hi Kairui,

On Sat, Nov 7, 2026 at 11:59 AM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> >
> On Fri, Nov 6, 2026 at 12:05 AM Alexandre Ghiti <alex@xxxxxxxx> wrote:
> >
> > Commit 9a42aed48421 ("mm, swap: prefer nonfull over free clusters") made
> > allocations fill the nonfull clusters of their order before taking a
> > free one, on every device without discard. Since commit 3cedb745d354
> > ("mm: clarify the folio_free_swap() for do_swap_page()"), an exclusive
> > write fault frees the swap slot, so a workload that rewrites swapped out
> > memory leaves holes in full clusters, which then go back on the nonfull
> > list. Reclaim allocates slots one folio at a time, so a batch of folios
> > gets scattered over those holes, its writes can't be merged and it is
> > written with many small requests.
> >
> > So partially revert commit 9a42aed48421 ("mm, swap: prefer nonfull over
> > free clusters") and prefer free clusters again, except on synchronous
> > devices like zram: they don't merge I/O, so the contiguity of the slots
> > doesn't matter to them, and filling the holes first keeps the free
> > clusters for large folios.
> >
> > Fixes: 9a42aed48421 ("mm, swap: prefer nonfull over free clusters")
>
> Do we need Cc stable? I guess no because 3cedb745d354 is new?

I think it should go to stable since it was merged in 7.3, I'm adding
the tag below (if it works that way in mm):

Cc: stable@xxxxxxxxxxxxxxx

>
> > Suggested-by: Kairui Song <kasong@xxxxxxxxxxx>
> > Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
> > ---
> >  mm/swapfile.c | 19 +++++++++++++------
> >  1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/mm/swapfile.c b/mm/swapfile.c
> > index 189f04d8838f..1c1b5d0b724a 100644
> > --- a/mm/swapfile.c
> > +++ b/mm/swapfile.c
> > @@ -973,6 +973,17 @@ static bool __swap_cluster_alloc_entries(struct swap_info_struct *si,
> >         return true;
> >  }
> >
> > +/*
> > + * Prefer a new cluster over a nonfull one if the device needs discard, to
> > + * spread out the writes, or if it is not synchronous, to keep the slots
> > + * of a swapout batch contiguous so that its writes can be merged.
> > + */
> > +static bool swap_prefer_free_cluster(struct swap_info_struct *si)
> > +{
> > +       return (si->flags & SWP_PAGE_DISCARD) ||
> > +              !(si->flags & SWP_SYNCHRONOUS_IO);
> > +}
>
> I actually wonder if we just want !si->flags & SWP_SYNCHRONOUS_IO
> here, but still reasonable to keep the two checks here.
>
> > +
> >  /* Try use a new cluster for current CPU and allocate from it. */
> >  static unsigned int alloc_swap_scan_cluster(struct swap_info_struct *si,
> >                                             struct swap_cluster_info *ci,
> > @@ -1143,11 +1154,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
> >         }
> >
> >  new_cluster:
> > -       /*
> > -        * If the device need discard, prefer new cluster over nonfull
> > -        * to spread out the writes.
> > -        */
> > -       if (si->flags & SWP_PAGE_DISCARD) {
> > +       if (swap_prefer_free_cluster(si)) {
>
> That's a pretty nice helper, thanks!
>
> The patch itself looks good to me.
>
> Acked-by: Kairui Song <kasong@xxxxxxxxxxx>

Thanks,

Alex

>