Re: [PATCH v2] mm/zswap: store <PAGE_SIZE compression failed page as-is

[Nhat Pham <nphamcs@gmail.com>]

On Fri, Aug 15, 2025 at 3:34 PM Chris Li <chrisl@kernel.org> wrote:
>
> On Wed, Aug 13, 2025 at 11:33 AM Nhat Pham <nphamcs@gmail.com> wrote:
> > > I know Hugh has some idea to store incompressible pages in the swap
> > > cache as well. Hugh?
> >
> > I've also proposed that approach internally - keeping the page in
> > swapcache, while adding them to the zswap LRU for writeback to disk
> > (and so that we do not consider them for zswap again in the future).
> >
> > But after a while, we decided against it, mostly due to the complexity
> > of the solution. On the zswap side, we need to distinguish between the
>
> Google actually has an internal patch to keep incompressible pages in
> separate LRU out of zswap. But that breaks the zswap LRU order as
> well. If there is interest and I can find the time, I can send it out
> for note comparison purposes. I do see the value of maintaining the
> LRU in the zswap tier as a whole.

It would be very valuable. Much appreciated, Chris!

And yes, we also discussed that approach too. What I described above
was an attempt to get the best-of-all-world.

There's a couple of desirata:

1. Maintain LRU ordering, in case we want to do zswap writeback.
2. Do not retry incompressible pages, until they are accessed.
3. Minimize the cost of page fault (memcpy, page allocation/free), as
much as possible.
4. Do not retry incompressible pages, until they are dirtied (a
stronger guarantee than 2).
5. Keep the stored data migratable.

In the end, it got too complicated. So we decided to go for 1, 2, and
5, with this approach. Regarding 3, this is still an improvement over
vanilla zswap (which writes to disk, and tends to have an even higher
cost of page fault).

I believe Google's approach gets us 2, 3, 4.


>
> > ordinary struct zswap_entry and the struct page on zswap's LRU list.
> > Externally, we need to handle moving a page currently in the zswap LRU
> > to the main memory anon LRUs too.
> >
> > Migration is another concern. Zswap needs to be notified that the
> > "backend" of a zswap entry has changed underneath it. Not impossible,
> > but again that's just more surgery.
>
> Ack. We might need to get that operation inside zsmalloc.
>
> >
> > So we decided to start with a simple solution (this one), and iterate
> > as issues cropped up. At least then, we have production justifications
> > for any future improvements.
>
> Ack.
>
> Chris