On Fri, Jun 7, 2024 at 9:08 PM Mika Penttilä <mpenttil@xxxxxxxxxx> wrote:
>
> On 6/8/24 05:36, Yosry Ahmed wrote:
> > diff --git a/mm/zswap.c b/mm/zswap.c
> > index b9b35ef86d9be..ebb878d3e7865 100644
> > --- a/mm/zswap.c
> > +++ b/mm/zswap.c
> > @@ -1557,6 +1557,26 @@ bool zswap_load(struct folio *folio)
> >
> > VM_WARN_ON_ONCE(!folio_test_locked(folio));
> >
> > + /*
> > + * 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.
> > + *
> > + * If any of the subpages are in zswap, reading from disk would result
> > + * in data corruption, so return true without marking the folio uptodate
> > + * so that an IO error is emitted (e.g. do_swap_page() will sigfault).
> > + *
> > + * Otherwise, return false and read the folio from disk.
> > + */
> > + if (folio_test_large(folio)) {
> > + if (xa_find(tree, &offset,
> > + offset + folio_nr_pages(folio) - 1, XA_PRESENT)) {
> > + WARN_ON_ONCE(1);
> > + return true;
> > + }
>
> How does that work? Should it be xa_find_after() to not always find
> current entry?
By "current entry" I believe you mean the entry corresponding to
"offset" (i.e. the first subpage of the folio). At this point, we
haven't checked if that offset has a corresponding entry in zswap or
not. It may be on disk, or zwap may be disabled.
>
> And does it still mean those subsequent entries map to same folio?
If I understand correctly, a folio in the swapcache has contiguous
swap offsets for its subpages. So I am assuming that the large folio
swapin case will adhere to that (i.e. we only swapin a large folio if
the swap offsets are contiguous). Did I misunderstand something here?
>
>
> --Mika
>
>
