Received-SPF: pass (google.com: domain of linux-kernel+bounces-189062-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240524033819.1953587-1-yosryahmed@google.com> <ZlAQo0P4Z-dgVHn6@casper.infradead.org>
In-Reply-To: <ZlAQo0P4Z-dgVHn6@casper.infradead.org>
From: Yosry Ahmed <yosryahmed@google.com>
Date: Fri, 24 May 2024 12:53:59 -0700
Message-ID: <CAJD7tkaVFa24Yty=8J01OKkaFB-TDiKq1tj0GuOD2_TbU+13SQ@mail.gmail.com>
Subject: Re: [PATCH 0/3] mm: zswap: trivial folio conversions
To: Matthew Wilcox <willy@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>, Johannes Weiner <hannes@cmpxchg.org>, 
	Nhat Pham <nphamcs@gmail.com>, Chengming Zhou <chengming.zhou@linux.dev>, linux-mm@kvack.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, May 23, 2024 at 8:59=E2=80=AFPM Matthew Wilcox <willy@infradead.org=
> wrote:
>
> On Fri, May 24, 2024 at 03:38:15AM +0000, Yosry Ahmed wrote:
> > Some trivial folio conversions in zswap code.
>
> The three patches themselves look good.
>
> > The mean reason I included a cover letter is that I wanted to get
> > feedback on what other trivial conversions can/should be done in
> > mm/zswap.c (keeping in mind that only order-0 folios are supported
> > anyway).  These are the things I came across while searching for 'page'
> > in mm/zswap.c, and chose not to do anything about for now:
>
> I think there's a deeper question to answer before answering these
> questions, which is what we intend to do with large folios and zswap in
> the future.  Do we intend to split them?  Compress them as a large
> folio?  Compress each page in a large folio separately?  I can see an
> argument for choices 2 and 3, but I think choice 1 is going to be
> increasingly untenable.

Yeah I was kinda getting the small things out of the way so that zswap
is fully folio-ized, before we think about large folios. I haven't
given it a lot of thought, but here's what I have in mind.

Right now, I think most configs enable zswap will disable
CONFIG_THP_SWAP (otherwise all THPs will go straight to disk), so
let's assume that today we are splitting large folios before they go
to zswap (i.e. choice 1).

What we do next depends on how the core swap intends to deal with
large folios. My understanding based on recent developments is that we
intend to swapout large folios as a whole, but I saw some discussions
about splitting all large folios before swapping them out, or leaving
them whole but swapping them out in order-0 chunks.

I assume the rationale is that there is little benefit to keeping the
folios whole because they will most likely be freed soon anyway, but I
understand not wanting to spend time on splitting them, so swapping
them out in order-0 chunks makes some sense to me. It also dodges the
whole fragmentation issue.

If we do either of these things in the core swap code, then I think
zswap doesn't need to do anything to support large folios. If not,
then we need to make a choice between 2 (compress large folios) &
choice 3 (compress each page separately) as you mentioned.

Compressing large folios as a whole means that we need to decompress
them as a whole to read a single page, which I think could be very
inefficient in some cases or force us to swapin large folios. Unless
of course we end up in a world where we mostly swapin the same large
folios that we swapped out. Although there can be additional
compression savings from compressing large folios as a whole.

Hence, I think choice 3 is the most reasonable one, at least for the
short-term. I also think this is what zram does, but I haven't
checked. Even if we all agree on this, there are still questions that
we need to answer. For example, do we allocate zswap_entry's for each
order-0 chunk right away, or do we allocate a single zswap_entry for
the entire folio, and then "split" it during swapin if we only need to
read part of the folio?

Wondering what others think here.