Re: [RFC PATCH 6/6] btrfs: zlib: add support for zlib-deflate through acomp

David Sterba
2025-05-07T12:43:24Z

On Tue, May 06, 2025 at 04:38:11PM +0100, Cabiddu, Giovanni wrote:
> Hi David,
>
> I've resumed work on this, now that I have all necessary dependencies to
> offload ZSTD to QAT.
>
> On Mon, Apr 29, 2024 at 04:41:29PM +0100, David Sterba wrote:
> ...
> > I'd skip the style and implementation details for now. The absence of
> > compression level support seems like the biggest problem, also in
> > combination with uncondtional use of the acomp interface.
> Regarding compression levels, Herbert suggested a new interface that
> would effectively address this concern [1].

It seems to be sufficient for passing the level from filesystem to
crypto layer.

One thing I'm not sure is considered is that zstd has different
requirements for workspace size depending on the level. In btrfs this is
done in zstd_calc_ws_mem_sizes().

> > We'd have to enhance the compression format specifier to make it
> > configurable in the sense: if accelerator is available use it, otherwise
> > do CPU and synchronous compression.
> For usability, wouldn't it be better to have a transparent solution? If
> an accelerator is present, use it, rather than having a configuration
> knob.
>
> In any case, I would like to understand the best way forward here. I
> would like to offload both zlib_deflate and ZSTD. However, I wouldn't
> like to replicate the logic that calls the acomp APIs in both
> fs/btrfs/zlib.c and fs/btrfs/zstd.c.

This looks doable, acomp_comp_pages() from your patch could work for
both, the only difference is the parameter to crypto_alloc_acomp() and
eventually the level.

Otherwise, how to go forward with that. I think we'd need to get a few
iterations staring from what you have, with added support for the levels
and then we'll remove/replace the problematic parts like the numerous
allocations.

As the first step, please send an update with the acomp levels added to
zlib callbacks, so it works in normal conditons with all the
allocations.

We'll need to move repeatedly used structures to the workspaces so that
will be the second step. Once we settle on someting reasonable we can
extend it and add zstd support.