Re: [PATCH v5 2/3] mm: swap: drop dropbehind swap cache folios on writeback completion

[Alexandre Ghiti]

Hi Barry,

On Mon, Sep 14, 2026 at 1:01 AM Barry Song <baohua@xxxxxxxxxx> wrote:
>
> >
> On Fri, Sep 11, 2026 at 8:16 PM Alexandre Ghiti <alex@xxxxxxxx> wrote:
> >
> > A PG_dropbehind folio is dropped from its cache once writeback completes
> > rather than left for reclaim to find later; this is implemented for file
> > folios in folio_end_dropbehind(). Extend it to swap cache folios.
> >
> > The drop takes the folio and swap cluster locks and may sleep, so it
> > cannot run in interrupt context. Set BIO_COMPLETE_IN_TASK on the write,
> > as the file dropbehind paths do, and drop the folio directly from
> > folio_end_writeback().
> >
> > Suggested-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> > Suggested-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> > Suggested-by: Nhat Pham <nphamcs@xxxxxxxxx>
> > Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx>
> > Reviewed-by: Kunwu Chan <kunwu.chan@xxxxxxxxx>
> > Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
> > ---
> [...]
>
> > diff --git a/mm/page_io.c b/mm/page_io.c
> > index 88962571cb93..52eae99de6e3 100644
> > --- a/mm/page_io.c
> > +++ b/mm/page_io.c
> > @@ -602,6 +602,15 @@ static void swap_bdev_submit_write(struct swap_io_ctx *ctx)
> >                 submit_bio_wait(bio);
> >                 end_swap_bio_write(bio);
> >         } else {
> > +               int p;
> > +
> > +               for (p = 0; p < sio->nr_bvecs; p++) {
> > +                       if (folio_test_dropbehind(bvec_folio(&sio->bvecs[p]))) {
> > +                               bio_set_flag(bio, BIO_COMPLETE_IN_TASK);
>
> Thanks very much, Alexandre. I find the code is really much,
> much better, but I’m still struggling to understand why we need a
> sleepable context after Wenjian’s commit:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?id=848d2ce2fce15fbdc083fbf9691bfa72911033c4
>
> With this question:
>
> https://lore.kernel.org/linux-mm/CAGsJ_4wj6XvPB+QF+_X1BJ_8Y2sZEEFAw6RaJJz-cHaDWLkxaQ@xxxxxxxxxxxxxx/T/#u
>
> In short, it still feels a little strange to me that we need a
> sleepable context, especially now that we no longer call
> `folio_unmap_invalidate()` for unmapped folios.

The thing is the zswap writeback folios are not on lru: that's because
if we put them on lru at allocation, the perpcu lru cache holds a
reference and it prevents remove_mapping() from freeing the folio (I
measured that with on-lru folios, the freed rate collapses from 99.9%
to ~3%).

So if we use the folio_trylock() approach, we'd need to put the folio
back on lru otherwise it would be "lost". But I don't think we can do
that while someone else holds the folio lock.

That's why I use folio_lock() which can sleep and then needs the
sleepable context.

What do you think?

Thanks,

Alex

>
> Best Regards
> Barry
>