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

[Alexandre Ghiti]

Hi Yosry,

On Tue, Aug 25, 2026 at 5:52 PM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> >
> 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?

Yes, I missed it!

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

Sure, will do in v5.

Thanks,

Alex

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