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

[Jianyue Wu]

On Tue, Aug 11, 2026 at 8:20 AM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> 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?
>
Thanks, good point, I'll use entry->handle as the condition:
       return rcu_dereference_protected(zswap_pools[entry->pool_idx],


entry->handle /* live entry pins pool */);

Best regards,
Jianyue