Re: [PATCH v3 2/3] mm/swap: refault on swap-in, not in the swap cache allocator

[Alexandre Ghiti]

Hi Usama,

On Wed, Aug 26, 2026 at 3:31 PM Usama Arif <usama.arif@xxxxxxxxx> wrote:
>
> >
> On Tue, 25 Aug 2026 19:24:52 +0200 Alexandre Ghiti <alex@xxxxxxxx> wrote:
>
> > The swap cache allocator evaluates the refault of every folio it
> > allocates.  zswap writeback also allocates through it: the shrinker puts
> > a buffer folio in the swap cache to write the compressed data out, and
> > that allocation is then counted as an anon refault (and, if the eviction
> > looks recent, as an activation) even though nothing faulted the page
> > back in.  On a workload that writes back continuously this inflates
> > workingset_refault_anon and workingset_activate_anon substantially.
> >
> > Move the refault evaluation out of the allocator and into the two
> > swap-in callers, which read the slot's shadow before the allocation
> > overwrites it.  zswap writeback keeps allocating the buffer, but no
> > longer reports a refault for it.
> >
> > The refault is evaluated before folio_add_lru(), as it was before this
> > patch, so workingset_refault() still sets PG_workingset/PG_active while
> > the folio is off the LRU: folio_add_lru() consumes both when it picks
> > the LRU list, and under MGLRU when it picks the generation.
> >
> > Fixes: aae466b0052e ("mm/swap: implement workingset detection for anonymous LRU")
> > Signed-off-by: Nhat Pham <nphamcs@xxxxxxxxx>
> > Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
> > ---
> >  mm/swap_state.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index bf8ff2d2dbf1..8046fea015c9 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -483,8 +483,6 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
> >
> >       /* memsw uncharges swap when folio is added to swap cache */
> >       memcg1_swapin(folio);
> > -     if (shadow)
> > -             workingset_refault(folio, shadow);
> >
> >       node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
> >       lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
> > @@ -646,17 +644,26 @@ static struct folio *swap_cache_read_folio(struct swap_io_ctx *ctx,
> >               pgoff_t ilx, bool readahead)
> >  {
> >       struct folio *folio;
> > +     void *shadow = NULL;
> >
> >       do {
> >               folio = swap_cache_get_folio(entry);
> >               if (folio)
> >                       return folio;
> > +             /*
> > +              * Capture the slot's shadow before the allocation overwrites it,
> > +              * so a fresh swap-in can be evaluated as a refault below.
> > +              */
> > +             shadow = swap_cache_get_shadow(entry);
> >               folio = __swap_cache_alloc_folio(entry, gfp, BIT(0), NULL, mpol, ilx);
>
> Can this refault against a shadow from a different cache generation?
> Another swap-in can replace this value, reclaim its folio, and install
> a newer shadow while this caller is allocating.  This caller can then
> win the locked insertion but still use the older snapshot.

I agree, you're right, I missed that. Thanks.

>
> What I think should be done here is have an optional shadow output to
> __swap_cache_alloc_folio() and return the value captured by the successful
> __swap_cache_add_check() under ci->lock and se that output in the swap-in
> path?

That seems right, I'll give it a try and report back.

Thanks,

Alex


>
> >       } while (PTR_ERR(folio) == -EEXIST);
> >
> >       if (IS_ERR_OR_NULL(folio))
> >               return NULL;
> >
> > +     if (shadow)
> > +             workingset_refault(folio, shadow);
> > +
> >       folio_add_lru(folio);
> >       swap_read_folio(ctx, folio);
> >       if (readahead) {
> > @@ -688,17 +695,26 @@ struct folio *swapin_sync(swp_entry_t entry, gfp_t gfp, unsigned long orders,
> >  {
> >       struct swap_io_ctx ctx = {};
> >       struct folio *folio;
> > +     void *shadow = NULL;
> >
> >       do {
> >               folio = swap_cache_get_folio(entry);
> >               if (folio)
> >                       return folio;
> > +             /*
> > +              * Capture the slot's shadow before the allocation overwrites it,
> > +              * so a fresh swap-in can be evaluated as a refault below.
> > +              */
> > +             shadow = swap_cache_get_shadow(entry);
> >               folio = __swap_cache_alloc_folio(entry, gfp, orders, vmf, mpol, ilx);
> >       } while (PTR_ERR(folio) == -EEXIST);
> >
> >       if (IS_ERR(folio))
> >               return folio;
> >
> > +     if (shadow)
> > +             workingset_refault(folio, shadow);
> > +
> >       folio_add_lru(folio);
> >       swap_read_folio(&ctx, folio);
> >       swap_read_submit(&ctx);
> > --
> > 2.53.0-Meta
> >
> >
>