Received-SPF: pass (google.com: domain of linux-kernel+bounces-208662-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240608023654.3513385-1-yosryahmed@google.com> <2821278f-bc94-c147-d0fe-8cc52dbdccb1@redhat.com>
In-Reply-To: <2821278f-bc94-c147-d0fe-8cc52dbdccb1@redhat.com>
From: Yosry Ahmed <yosryahmed@google.com>
Date: Mon, 10 Jun 2024 10:35:54 -0700
Message-ID: <CAJD7tkYepf6z1Q7HiSRAc7-S9OwYCphx=9NcyHxtW0YrkFv0sQ@mail.gmail.com>
Subject: Re: [PATCH v2] mm: zswap: handle incorrect attempts to load of large folios
To: =?UTF-8?Q?Mika_Penttil=C3=A4?= <mpenttil@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, Johannes Weiner <hannes@cmpxchg.org>, 
	Nhat Pham <nphamcs@gmail.com>, Chengming Zhou <chengming.zhou@linux.dev>, 
	Baolin Wang <baolin.wang@linux.alibaba.com>, Barry Song <21cnbao@gmail.com>, 
	Chris Li <chrisl@kernel.org>, Ryan Roberts <ryan.roberts@arm.com>, 
	David Hildenbrand <david@redhat.com>, Matthew Wilcox <willy@infradead.org>, linux-mm@kvack.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 7, 2024 at 9:08=E2=80=AFPM Mika Penttil=C3=A4 <mpenttil@redhat.=
com> 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 use=
d, as
> > +      * they are not properly handled. Zswap does not properly load la=
rge
> > +      * folios, and a large folio may only be partially in zswap.
> > +      *
> > +      * If any of the subpages are in zswap, reading from disk would r=
esult
> > +      * in data corruption, so return true without marking the folio u=
ptodate
> > +      * so that an IO error is emitted (e.g. do_swap_page() will sigfa=
ult).
> > +      *
> > +      * 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_PRESEN=
T)) {
> > +                     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
>
>