Re: [PATCH v5 04/11] mm: zswap: add range lookup for large-folio swapin

[Nhat Pham]

On Thu, Jul 23, 2026 at 9:43 AM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> > >> @@ -1595,13 +1611,19 @@ int zswap_load(struct folio *folio)
> > >>              return -ENOENT;
> > >>
> > >>      /*
> > >> -     * Large folios should not be swapped in while zswap is being used, as
> > >> -     * they are not properly handled. Zswap does not properly load large
> > >> -     * folios, and a large folio may only be partially in zswap.
> > >> +     * A large folio reaches zswap_load() only when its whole range is
> > >> +     * expected to be on disk: PMD swap-entry consumers split before
> > >> +     * calling into PMD-order swapin whenever any slot is still in zswap.
> > >> +     * Confirm the range is entirely absent from zswap and return -ENOENT
> > >> +     * so the caller reads it from disk; if a slot is unexpectedly still in
> > >> +     * zswap, fail the read rather than return partially-initialized data.
> > >>       */
> > >> -    if (WARN_ON_ONCE(folio_test_large(folio))) {
> > >> -            folio_unlock(folio);
> > >> -            return -EINVAL;
> > >> +    if (folio_test_large(folio)) {
> > >> +            if (zswap_is_present(swp, folio_nr_pages(folio))) {
> > >
> > > Is dropping the warning here intentional (for the folio_test_large() &&
> > > zswap_is_present() case)?
> > >
> >
> >
> > Yes, so we can end up in a race, which should be handled gracefully.
> >
> > For example, lets say we have zswap writeback enabled, which means we
> > can end up in a state where we have a PMD swap entry and 511 of the 512
> > slots have been written to disk, but 1 slot (slot X) is still in zswap.
> >
> > We can then have the following race:
> >
> >
> >   CPU A: PMD swap-in                  CPU B: split-PTE swap-in
> >   ------------------                 ------------------------
> >
> >   Checks swap cache: empty
> >
> >                                        Faults slot X
> >                                        Adds order-0 folio F
> >                                        zswap_load(F):
> >                                          removes X from zswap
> >                                          marks F dirty
> >
> >   Checks zswap range: empty
> >   A is preempted
> >
> >                                        Unmaps/reclaims F
> >                                        zswap_store(F):
> >                                          puts X back in zswap
> >                                        Removes F from swap cache
> >
> >   A resumes
> >   Allocates large swap-cache folio G
> >   zswap_load(G) finds X in zswap
>
> Why don't we check the zswap range after allocating a folio in the
> swap cache? I am assuming at this point we have the folio locked and
> the result should be stable?
>

+1. Is it possible to perform the check in __swap_cache_add_check?

That's what I did for vswap, per Kairui's suggestions. The cluster
lock is held from that point all the way to after the folio is
locked-and-loaded into swap cache, at which point no one can change
the backend under you.