Received-SPF: pass (google.com: domain of linux-kernel+bounces-206834-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
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>
In-Reply-To: <CAJD7tkZkChE_bGJCnf3u0493PZOe1=+Y8S_nq2=j1uP-BtZ9Mg@mail.gmail.com>
From: Barry Song <21cnbao@gmail.com>
Date: Sat, 8 Jun 2024 06:28:21 +0800
Message-ID: <CAGsJ_4zU0fVj8M-qKnQAKoXdrQrBuQchs_yZ32=e2uY80kvxxA@mail.gmail.com>
Subject: Re: [PATCH] mm: zswap: add VM_BUG_ON() if large folio swapin is attempted
To: Yosry Ahmed <yosryahmed@google.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 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 will
> > > be emitted just if CONFIG_ZSWAP is on. Perhaps we need a variable or
> > > 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 wh=
en
> > I am loading data, will I get corrupted data before zswap was once enab=
led?
> > 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".

Thanks
Barry