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

[Jianyue Wu]

On Tue, Jul 28, 2026 at 1:07 AM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:

Thanks Yosry and Nhat for the review.

> Technically we are introducing a new limit on the number of pools, so
> let's mention that.

That's good, can simplify a lot, will do in v2.

> Did you try to benchmark it? I doubt the indirection would actually
> show up as most use cases should have a single pool.

I ran zswap e2e benchmark comparing baseline vs this RFC and did not
observe a measurable performance regression.
BTW, it seems zswap crypto-sg zstd and lzo are slower than zcomp.
Throughput in MB/s:
 alg      | acomp store | zcomp store | acomp load | zcomp load
----------+-------------+-------------+------------+-----------
 lzo      |        1140 |        1710 |       1070 |       1070
 lzo-rle  |        1490 |        1320 |       2030 |       2070
 lz4      |        1520 |        1600 |       1710 |       1870
 lz4hc    |         640 |         680 |       1770 |       1850
 zstd     |         740 |        1190 |        980 |       1720
 deflate  |         180 |         180 |        665 |        660
But that's another topic.

> No need to document the function name here, most probably will be
> stale at some point.

Ack.

> This seems like the only user of zswap_pool_by_index(), just open-code
> the xa_load() here.
>
> Let's add a NULL check here to avoid blowing up if something goes wrong.
>
> We need a NULL check here as well.

Ack on all of these.

> Maybe just add a fixed array and call it a day? :) Seems like we can
> probably get away with, say a fixed array 16 elements.
>
> If we want to be really conservative we can use an array of 256
> elements indexed by a u8. That's 1KB of memory, probably mostly wasted.

I prefer dropping the xarray and go with 16 entries indexed by a u8,
keeping runtime compressor switching, which can increase when needed.

> 2. Note that the zswap pools are also organized into a list. If you're
> adding a new data structure, maybe remove that.
>
> Also a good point, I think we may be able to drop the list, and I
> think that's ultimately more valuable than the memory savings.

Agreed, the list looks like unneeded complexity, will have a try to drop it.
Looks obj_cgroup pointer can also be dropped on !MEMCG, but zswap usually use
in server with cgroup, maybe not worth pursuing.

Best regards,
Jianyue