Re: [PATCH RFC v2 2/2] mm/zswap: reference the pool by index to shrink struct zswap_entry

[Yosry Ahmed]

On Fri, Jul 31, 2026 at 08:32:48AM +0800, Jianyue Wu wrote:
> struct zswap_entry is one allocation per stored page, so its size is pure
> overhead.  It currently embeds an 8-byte pool pointer, even though the
> live pools now sit in a small fixed array indexed by a u8 slot number.
> 
> Replace the per-entry pool pointer with that u8 slot index and resolve it
> through a zswap_entry_pool() helper.  A live entry holds a reference to
> its pool, so the slot cannot be reused under it; the lookup therefore
> needs no RCU read-side section (rcu_dereference_protected(..., true)).
> 
> The u8 fits in the padding after the bool referenced field, shrinking the
> entry from 56 to 48 bytes on x86_64.  This raises objs_per_slab from 73
> to 85 and saves about 2MiB of metadata per 1GiB of data held in zswap.
> 
> Suggested-by: Chris Li <chrisl@xxxxxxxxxx>
> Signed-off-by: Jianyue Wu <wujianyue000@xxxxxxxxx>
> ---
>  mm/zswap.c | 33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/zswap.c b/mm/zswap.c
> index b203934d3be8..d4f4db2999f2 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -190,7 +190,7 @@ static struct shrinker *zswap_shrinker;
>   *              writeback logic. The entry is only reclaimed by the writeback
>   *              logic if referenced is unset. See comments in the shrinker
>   *              section for context.
> - * pool - the zswap_pool the entry's data is in
> + * pool_idx - slot of the zswap_pool that the entry's data is in.
>   * handle - zsmalloc allocation handle that stores the compressed page data
>   * objcg - the obj_cgroup that the compressed memory is charged to
>   * lru - handle to the pool's lru used to evict pages.
> @@ -199,12 +199,22 @@ struct zswap_entry {
>  	swp_entry_t swpentry;
>  	unsigned int length;
>  	bool referenced;
> -	struct zswap_pool *pool;
> +	u8 pool_idx;
>  	unsigned long handle;
>  	struct obj_cgroup *objcg;
>  	struct list_head lru;
>  };
>  
> +static struct zswap_pool *zswap_entry_pool(struct zswap_entry *entry)
> +{
> +	/*
> +	 * A live entry holds a reference to its pool, so the slot cannot be
> +	 * cleared or reused under it.  This is not an RCU read-side walk.
> +	 */
> +	return rcu_dereference_protected(zswap_pools[entry->pool_idx],
> +					 true /* entry pins pool */);

Probably doesn't matter in practice, but maybe entry->handle or
something instead of 'true' to make it clear we are checking for an
"active" entry?

> +}
> +
>  static struct xarray *zswap_trees[MAX_SWAPFILES];
>  static unsigned int nr_zswap_trees[MAX_SWAPFILES];
>  
> @@ -807,9 +817,13 @@ static void zswap_entry_cache_free(struct zswap_entry *entry)
>   */
>  static void zswap_entry_free(struct zswap_entry *entry)
>  {
> +	struct zswap_pool *pool = zswap_entry_pool(entry);
> +
>  	zswap_lru_del(&zswap_list_lru, entry);
> -	zs_free(entry->pool->zs_pool, entry->handle);
> -	zswap_pool_put(entry->pool);
> +	if (!WARN_ON_ONCE(!pool)) {
> +		zs_free(pool->zs_pool, entry->handle);
> +		zswap_pool_put(pool);
> +	}
>  	if (entry->objcg) {
>  		obj_cgroup_uncharge_zswap(entry->objcg, entry->length);
>  		obj_cgroup_put(entry->objcg);
> @@ -966,12 +980,15 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
>  
>  static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>  {
> -	struct zswap_pool *pool = entry->pool;
> +	struct zswap_pool *pool = zswap_entry_pool(entry);
>  	struct scatterlist input[2]; /* zsmalloc returns an SG list 1-2 entries */
>  	struct scatterlist output;
>  	struct crypto_acomp_ctx *acomp_ctx;
>  	int ret = 0, dlen;
>  
> +	if (WARN_ON_ONCE(!pool))
> +		return false;
> +
>  	acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
>  	mutex_lock(&acomp_ctx->mutex);
>  	zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length);
> @@ -1007,7 +1024,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>  	pr_alert_ratelimited("Decompression error from zswap (%d:%lu %s %u->%d)\n",
>  						swp_type(entry->swpentry),
>  						swp_offset(entry->swpentry),
> -						entry->pool->tfm_name,
> +						pool->tfm_name,
>  						entry->length, dlen);
>  	return false;
>  }
> @@ -1500,7 +1517,7 @@ static bool zswap_store_page(struct page *page,
>  	 *    The publishing order matters to prevent writeback from seeing
>  	 *    an incoherent entry.
>  	 */
> -	entry->pool = pool;
> +	entry->pool_idx = pool->idx;
>  	entry->swpentry = page_swpentry;
>  	entry->objcg = objcg;
>  	entry->referenced = true;
> @@ -1806,7 +1823,7 @@ static int zswap_setup(void)
>  	struct zswap_pool *pool;
>  	int ret;
>  
> -	/* Slot indices are stored in a u8 (pool->idx). */
> +	/* Slot indices are stored in a u8 (pool->idx and entry->pool_idx). */
>  	BUILD_BUG_ON(ZSWAP_MAX_POOLS - 1 > U8_MAX);
>  
>  	zswap_entry_cache = KMEM_CACHE(zswap_entry, 0);
> 
> -- 
> 2.43.0
>