Re: [PATCH v4 3/3] mm: zswap: drop cold writeback folios via swap dropbehind

[Yosry Ahmed]

On Tue, Aug 25, 2026 at 6:55 AM Alexandre Ghiti <alex@xxxxxxxx> wrote:
>
> zswap writeback decompresses an entry into a fresh swap cache folio and
> writes it back. The folio is cold by construction, yet it is left on the
> LRU for reclaim to find and free later, wasting a reclaim scan and keeping
> cold memory resident longer than necessary.
>
> Allocate the folio off the LRU and mark it PG_dropbehind so the swap
> dropbehind path frees it from the swap cache once writeback completes.
>
> Suggested-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> Suggested-by: Nhat Pham <nphamcs@xxxxxxxxx>
> Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
> ---
>  mm/zswap.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 8163e6c5f76c..d16822a516e8 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1013,7 +1013,6 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
>          */
>         if (IS_ERR(folio))
>                 return PTR_ERR(folio);
> -       folio_add_lru(folio);
>
>         /*
>          * folio is locked, and the swapcache is now secured against
> @@ -1046,12 +1045,26 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
>         /* folio is up to date */
>         folio_mark_uptodate(folio);
>
> -       /* move it to the tail of the inactive list after end_writeback */
> -       folio_set_reclaim(folio);
> +       folio_set_dropbehind(folio);
> +
> +       /*
> +        * Drop our reference before starting writeback so the swap cache holds
> +        * the only one: the drop in folio_end_writeback() needs that for
> +        * remove_mapping_reclaim() to succeed, otherwise the folio is handed
> +        * back to reclaim instead.
> +        *
> +        * Nothing can free the folio in the meantime: we hold the folio lock
> +        * until writeback starts, PG_writeback then blocks swap cache removal,
> +        * and folio_end_writeback() takes its own reference before clearing
> +        * PG_writeback and donates it to the drop.
> +        */
> +       folio_put(folio);
>
>         /* start writeback */
>         __swap_writepage(folio, NULL);
>
> +       return 0;
> +
>  out:
>         if (ret) {

This check is now unneeded as ret should always be non-zero here, right?

We should probably also rename the label to indicate that it's only
the error path. Maybe "error" or "fail"?

>                 swap_cache_del_folio(folio);
> --
> 2.53.0-Meta
>
>