Re: [PATCH v14 26/26] mm: zswap: Batched zswap_compress() for compress batching of large folios.

[Yosry Ahmed <yosry.ahmed@linux.dev>]

On Tue, Feb 03, 2026 at 04:30:48PM -0800, Nhat Pham wrote:
[..]
 > @@ -900,84 +923,177 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
> >         return ret;
> >  }
> >
> > -static bool zswap_compress(struct page *page, struct zswap_entry *entry,
> > -                          struct zswap_pool *pool, bool wb_enabled)
> > +/*
> > + * zswap_compress() batching implementation for sequential and batching
> > + * compressors.
> > + *
> > + * Description:
> > + * ============
> > + *
> > + * Compress multiple @nr_pages in @folio starting from the @folio_start index in
> > + * batches of @nr_batch_pages.
> > + *
> > + * It is assumed that @nr_pages <= ZSWAP_MAX_BATCH_SIZE. zswap_store() makes
> > + * sure of this by design and zswap_store_pages() warns if this is not true.
> > + *
> > + * @nr_pages can be in (1, ZSWAP_MAX_BATCH_SIZE] even if the compressor does not
> > + * support batching.
> > + *
> > + * If @nr_batch_pages is 1, each page is processed sequentially.
> > + *
> > + * If @nr_batch_pages is > 1, compression batching is invoked within
> > + * the algorithm's driver, except if @nr_pages is 1: if so, the driver can
> > + * choose to call it's sequential/non-batching compress routine.
> 
> Hmm, I'm a bit confused by this documentation.
> 
> Why is there extra explanation about nr_batch_pages > 1 and nr_pages
> == 1? That cannot happen, no?
> 
> nr_batch_pages is already determined by the time we enter
> zswap_compress() (the computation is done at its callsite, and already
> takes into account nr_pages, since it is the min of nr_pages, and the
> compressor batch size).
> 
> I find this batching (for store), then sub-batching (for compression),
> confusing, even if I understand it's to maintain/improve performance
> for the software compressors... It makes indices in zswap_compress()
> very convoluted.
> 
> Yosry and Johannes - any thoughts on this?

Yeah, not a big fan either. I am really wondering if the perf hit is
real enough to justify this. I would much rather we use the same batch
size for both.

IIUC the problem is that we cannot use the crypto batching interface for
SW compressors as it requires compressor support, and we cannot avoid
batching altogether for SW compressors because they regress.

I wonder if we can add support in the crypto layer to handle batching
for SW compressors without compressor support (just loop on the batch
and compress one-by-one)?

Alternatively, I did suggest we at least introduce an intermediate
function to do the sub-batching to simplify zswap_compress() (e.g.
zswap_compress() and __zswap_compress()).  I think this also caused
regressions but I wonder if we can force inline it or sth.

The current design is really confusing.