Re: [PATCH v7 5/8] mm: zswap: Compress and store a specific page in a folio.

[Johannes Weiner]

On Wed, Sep 25, 2024 at 01:49:03AM +0000, Sridhar, Kanchana P wrote:
> > From: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
> > I think it's more correct and efficient to update the atomic once
> > after all the pages are successfully compressed and stored.
> 
> Actually this would need to co-relate with the limits checking strategy,
> because the atomic is used there and needs to be as accurate as possible.

For the limit checks, we use the zpool counters, not zswap_stored_pages.

zswap_stored_pages is used in the zswap shrinker to guesstimate
pressure, so it's likely a good thing to only count entries that are
expected to stay, and not account the ones that might fail just yet.

> As far as the vmstat 'zswpout', the reason I left it as-is in my patchset
> was to be more indicative of the actual zswpout compute events that
> occurred (for things like getting the compressions count), regardless
> of whether or not the overall mTHP store was successful. If this vmstat
> needs to reflect only successful zswpout events (i.e., represent the zswap
> usage), I can fix it by updating it once only if the mTHP is stored successfully.

Yeah, that's fine as well.

I would suggest batching them both at the end of zswap_store().