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().
