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

[SeongJae Park <sj@kernel.org>]

On Fri, 8 Aug 2025 16:37:15 -0700 Nhat Pham <nphamcs@gmail.com> wrote:

> On Thu, Aug 7, 2025 at 4:54 PM SeongJae Park <sj@kernel.org> wrote:
> >
> > On Thu, 7 Aug 2025 16:03:54 -0700 Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > > On Thu, Aug 7, 2025 at 11:16 AM SeongJae Park <sj@kernel.org> wrote:
[...]
> > > Also, can we fix the counter value?
> > >
> > > I assume we want:
> > >
> > > else if (comp_ret || dlen = PAGE_SIZE)
> > >      zswap_reject_compress_fail++;
> > >
> > > or something like that.
> >
> > I'm not very clearly getting your point.
> >
> > I was thinking we should increase the counter if we "reject" the page (does not
> > save the content in the zpool) due to failing at compressing the page's content
> > into a size smaller than PAGE_SIZE.  This patch implements the behavior.
> >
> > Am I missing a mis-implementation of the behavior in this patch, or the
> > behavior is not what you think it should be?  More elaboration of your point
> > would be helpful for me.
> 
> Ah yeah, maybe "reject compress fail" is not a good name here. But
> sometimes I like to know how many times we fail to compress, even if
> we do save them.

Thank you for clarifying, that makes sense to me.

> 
> We can rename it to just "zswap_compress_fail", but that's breaking
> API, so it's kind of annoying. Maybe "zswap_stored_uncompressed_pages"
> suffices (see comment below).

The suggested name sounds good to me.

> 
> Johannes, any suggestions on what to do here?

+1

> 
> >
> > >
> > > And what happened to the incompressible page stored counter? :)
> >
> > I don't get what counter you are asking about.  Could you please elaborate?
> 
> I meant "zswap_stored_uncompressed_pages" in your RFC v1.

Thank you for kindly elaborating this.  I implemented that not to provide an
additional observability, but only for keeping zswap_total_pages() account the
pages including the uncompressed pages, though, since the version was not using
zpool.  The internal counter has dropped from RFC v2, since we started using
zpool, thanks to feedbacks from reviewers including you.

> 
> That could give us a nice breakdown of how much memory in zswap is
> actually compressed memory, and how much is uncompressed.

I agree it could be useful information.  Unless others raise different
opinions, I will implement this in the next version, with your suggested name.



Thanks,
SJ

[...]