Re: [PATCH v2] mm: zswap: remove nr_zswap_stored atomic

[Johannes Weiner]

On Fri, Mar 22, 2024 at 12:10:01AM +0000, Yosry Ahmed wrote:
> nr_stored was introduced by commit b5ba474f3f51 ("zswap: shrink zswap
> pool based on memory pressure") as a per zswap_pool counter of the
> number of stored pages that are not same-filled pages. It is used in
> zswap_shrinker_count() to scale the number of freeable compressed pages
> by the compression ratio.  That is, to reduce the amount of writeback
> from zswap with higher compression ratios as the ROI from IO diminishes.
> 
> Later on, commit bf9b7df23cb3 ("mm/zswap: global lru and shrinker shared
> by all zswap_pools") made the shrinker global (not per zswap_pool), and
> replaced nr_stored with nr_zswap_stored (initially introduced as
> zswap.nr_stored), which is now a global counter.
> 
> The counter is now awfully close to zswap_stored_pages. The only
> difference is that the latter also includes same-filled pages. Also,
> when memcgs are enabled, we use memcg_page_state(memcg, MEMCG_ZSWAPPED),
> which includes same-filled pages anyway (i.e.  equivalent to
> zswap_stored_pages).
> 
> Use zswap_stored_pages instead in zswap_shrinker_count() to keep things
> consistent whether memcgs are enabled or not, and add a comment about
> the number of freeable pages possibly being scaled down more than it
> should if we have lots of same-filled pages (i.e. inflated compression
> ratio).
> 
> Remove nr_zswap_stored and one atomic operation in the store and free
> paths.
> 
> Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
> Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx>

Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>