Received-SPF: pass (google.com: domain of linux-kernel+bounces-206857-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: <20240606184818.1566920-1-yosryahmed@google.com>
 <84d78362-e75c-40c8-b6c2-56d5d5292aa7@redhat.com> <CAJD7tkZH9C21nx75W9Erun=oUvmad5ujmDyGYWRRHEwPCCizUw@mail.gmail.com>
 <7507d075-9f4d-4a9b-836c-1fbb2fbd2257@redhat.com> <CAGsJ_4w_dEyLsPhdJCtRMReXDD116p+U979Yk-8EBu=zx5FWgQ@mail.gmail.com>
 <CAJD7tkYnZCvwUSTAx-N-08b+mVF4yHEdDTbp9oqYrVQXX8M1Cw@mail.gmail.com>
 <CAGsJ_4wAYbofjg99iEr7cEAptH9Ve9WcrSXho8A-Sw54vb8Msw@mail.gmail.com>
 <9374758d-9f81-4e4f-8405-1f972234173e@redhat.com> <CAJD7tkZBzSB_6pAZP0n0nq+W=J1XKQGFzZZLzPmSH0apwaqTNg@mail.gmail.com>
 <CAGsJ_4ywmVbO+nrbcxz9YMdzi_y5qSUG3Cg=3oz3kNpQ6gg6iA@mail.gmail.com>
 <CAJD7tkZkChE_bGJCnf3u0493PZOe1=+Y8S_nq2=j1uP-BtZ9Mg@mail.gmail.com> <CAGsJ_4zU0fVj8M-qKnQAKoXdrQrBuQchs_yZ32=e2uY80kvxxA@mail.gmail.com>
In-Reply-To: <CAGsJ_4zU0fVj8M-qKnQAKoXdrQrBuQchs_yZ32=e2uY80kvxxA@mail.gmail.com>
From: Yosry Ahmed <yosryahmed@google.com>
Date: Fri, 7 Jun 2024 17:32:13 -0700
Message-ID: <CAJD7tkZH8gHouYM=GT-njsKmV7=wD6bxSKn5zEwQsE_GAUzsfQ@mail.gmail.com>
Subject: Re: [PATCH] mm: zswap: add VM_BUG_ON() if large folio swapin is attempted
To: Barry Song <21cnbao@gmail.com>
Cc: David Hildenbrand <david@redhat.com>, 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>, 
	Chris Li <chrisl@kernel.org>, Ryan Roberts <ryan.roberts@arm.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 4:28=E2=80=AFPM Barry Song <21cnbao@gmail.com> wrote=
:
>
> On Sat, Jun 8, 2024 at 7:17=E2=80=AFAM Yosry Ahmed <yosryahmed@google.com=
> wrote:
> >
> > [..]
> > > > One problem is that even if zswap was never enabled, the warning wi=
ll
> > > > be emitted just if CONFIG_ZSWAP is on. Perhaps we need a variable o=
r
> > > > static key if zswap was "ever" enabled.
> > > >
> > > > Barry, I suspect your is_zswap_enabled() check is deficient for
> > > > similar reasons, zswap could have been enabled before then became
> > > > disabled.
> > >
> > > I don't understand this. if zswap was enabled before but is disabled =
when
> > > I am loading data, will I get corrupted data before zswap was once en=
abled?
> > > If not, it seems nothing important.
> >
> > If zswap was enabled and then disabled, some pages may still be in
> > zswap. We do not load the pages from zswap when it is disabled, we
> > just stop storing new pages.
> >
> > So if you just rely in checking whether zswap is enabled at swapin
> > time to decide whether to use large folios, you may end up with a
> > situation where zswap is disabled, yet parts of the large folio you
> > are trying to swapin (or all of it) is in zswap.
> >
> > This is why I think we'll need to track whether zswap was ever enabled
> > instead (or if a page was ever stored).
>
> Thanks! It doesn't seem good. Do we have a simple way to clean zswap
> when it is disabled? seems not easy? Just like we do swapoff, or disable
> cache, we ensure they are clean - this is a real "disable".

I think it's just simpler, and disabling zswap at runtime is not that
common. Keep in mind that if zswap being disabled takes time, then
we'll want to handle races between zswap being disabled and incoming
swapins, it will probably end up being a state machine or so.

Without a valid use case, I think the complexity is not justified.
It's probably simpler to just track if zswap was ever used, and
disable large folio swapin. This should be a transitional state
anyway.