On Fri, Nov 22, 2024 at 11:01 PM Kanchana P Sridhar <kanchana.p.sridhar@xxxxxxxxx> wrote: > > This patch does the following: > > 1) Modifies the definition of "struct crypto_acomp_ctx" to represent a > configurable number of acomp_reqs and buffers. Adds a "nr_reqs" to > "struct crypto_acomp_ctx" to contain the nr of resources that will be > allocated in the cpu onlining code. > > 2) The zswap_cpu_comp_prepare() cpu onlining code will detect if the > crypto_acomp created for the pool (in other words, the zswap compression > algorithm) has registered an implementation for batch_compress() and > batch_decompress(). If so, it will set "nr_reqs" to > SWAP_CRYPTO_BATCH_SIZE and allocate these many reqs/buffers, and set > the acomp_ctx->nr_reqs accordingly. If the crypto_acomp does not support > batching, "nr_reqs" defaults to 1. > > 3) Adds a "bool can_batch" to "struct zswap_pool" that step (2) will set to > true if the batching API are present for the crypto_acomp. Why do we need this "can_batch" field? IIUC, this can be determined from the compressor internal fields itself, no? acomp_has_async_batching(acomp); Is this just for convenience, or is this actually an expensive thing to compute? > > SWAP_CRYPTO_BATCH_SIZE is set to 8, which will be the IAA compress batching I like a sane default value as much as the next guy, but this seems a bit odd to me: 1. The placement of this constant/default value seems strange to me. This is a compressor-specific value no? Why are we enforcing this batching size at the zswap level, and uniformly at that? What if we introduce a new batch compression algorithm...? Or am I missing something, and this is a sane default for other compressors too? 2. Why is this value set to 8? Experimentation? Could you add some justification in documentation?
