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

[Nhat Pham]

On Thu, Nov 5, 2026 at 8:51 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")
> Suggested-by: Kairui Song <kasong@xxxxxxxxxxx>
> Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>

LGTM for the most part:

Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx>

Quick comment below:

> ---
>  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);
> +}

Side note - I wonder if it's worth it to also prefer non-full clusters
for zswap as well?

It's a bit tricky, but I suppose you can do a best-effort check of the
folio's cgroup's zswappability?