Re: [PATCH v1 7/8] mm: zswap: Optimally place members of struct zswap_pool, struct zswap_entry.

[Kanchana P. Sridhar]

On Wed, Oct 14, 2026 at 8:43 AM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> On Thu, Oct 08, 2026 at 11:28:13AM -0700, Kanchana P. Sridhar wrote:
> > Place and align the members of struct zswap_pool and struct zswap_entry
> > such that members used together in compress/decompress hot paths are
> > able to share the same cacheline. Minimize holes within these structs
> > while doing so.
> >
> > Signed-off-by: Kanchana P. Sridhar <kanchanapsridhar2026@xxxxxxxxx>
> > ---
> >  mm/zswap.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/zswap.c b/mm/zswap.c
> > index 8ae67fd10522..ade84ac4076e 100644
> > --- a/mm/zswap.c
> > +++ b/mm/zswap.c
> > @@ -160,11 +160,11 @@ struct crypto_acomp_ctx {
> >  struct zswap_pool {
> >       struct zs_pool *zs_pool;
> >       struct crypto_acomp_ctx __percpu *acomp_ctx;
> > -     u8 compr_batch_size;
> >       struct percpu_ref ref;
> > -     struct rcu_work release_rwork;
> > -     struct hlist_node node;
> >       u8 idx;
> > +     u8 compr_batch_size;
> > +     struct hlist_node node;
> > +     struct rcu_work release_rwork;
>
> compr_batch_size is added by this series, why not just add it before idx
> to begin with?

In patch 0006, where compr_batch_size is introduced, I was attempting
to place it in the same cacheline as the acomp_ctx. This increased the
size of struct zswap_pool to 248 bytes. I later optimized this in
patch 0007, bringing back the size to 240 with the addition of
compr_batch_size.

As things stand:

Baseline mm-unstable:

struct zswap_pool {
        struct zs_pool *           zs_pool;              /*     0     8 */
        struct crypto_acomp_ctx *  acomp_ctx;            /*     8     8 */
        struct percpu_ref          ref;                  /*    16    16 */
        struct rcu_work            release_rwork
__attribute__((__aligned__(8))); /*    32    56 */
        /* --- cacheline 1 boundary (64 bytes) was 24 bytes ago --- */
        struct hlist_node          node;                 /*    88    16 */
        u8                         idx;                  /*   104     1 */
        char                       tfm_name[128];        /*   105   128 */

        /* size: 240, cachelines: 4, members: 7 */
        /* padding: 7 */
        /* forced alignments: 1 */
        /* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));

My latest version makes changes to struct zswap_pool in 2 separate
patches: the first introduces compr_batch_size without changing the
size, the second optimizes for hot-path members' co-location on the
same cacheline:

struct zswap_pool {
  struct zs_pool *zs_pool;
  struct crypto_acomp_ctx __percpu *acomp_ctx;
  struct percpu_ref ref;
  u8 idx;
  u8 compr_batch_size;
  struct hlist_node node;
  struct rcu_work release_rwork;
  char tfm_name[CRYPTO_MAX_ALG_NAME];
};

struct zswap_pool {
        struct zs_pool *           zs_pool;              /*     0     8 */
        struct crypto_acomp_ctx *  acomp_ctx;            /*     8     8 */
        struct percpu_ref          ref;                  /*    16    16 */
        u8                         idx;                  /*    32     1 */
        u8                         compr_batch_size;     /*    33     1 */

        /* XXX 6 bytes hole, try to pack */

        struct hlist_node          node;                 /*    40    16 */
        struct rcu_work            release_rwork
__attribute__((__aligned__(8))); /*    56    56 */
        /* --- cacheline 1 boundary (64 bytes) was 48 bytes ago --- */
        char                       tfm_name[128];        /*   112   128 */

        /* size: 240, cachelines: 4, members: 8 */
        /* sum members: 234, holes: 1, sum holes: 6 */
        /* forced alignments: 1 */
        /* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));

>
> >       char tfm_name[CRYPTO_MAX_ALG_NAME];
> >  };
> >
> > @@ -202,10 +202,10 @@ static struct shrinker *zswap_shrinker;
> >   *              logic if referenced is unset. See comments in the shrinker
> >   *              section for context.
> >   * nid - NUMA node id of the page for which this is the zswap entry.
> > - * pool_idx - id 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.
> > + * pool_idx - id of the zswap_pool that the entry's data is in.
> >   */
> >  struct zswap_entry {
> >       swp_entry_t swpentry;
> > @@ -214,10 +214,10 @@ struct zswap_entry {
> >               bool referenced:1;
> >       };
> >       int nid;
> > -     u8 pool_idx;
> >       unsigned long handle;
> >       struct obj_cgroup *objcg;
> >       struct list_head lru;
> > +     u8 pool_idx;
>
> As mentioned previously, this needs to be packed with the length.

Done.

>
> >  };
> >
> >  /*
> > --
> > 2.39.5
> >