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

Cabiddu, Giovanni
2025-05-07T13:12:26Z

On Wed, May 07, 2025 at 02:43:21PM +0200, David Sterba wrote:
> 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.

Thanks David. I like the incremental approach.

I'll be sending a new version that is rebased on top of 6.15-rc. This
update will include the addition of compression levels to zlib-deflate
and will utilize folios instead of pages.
We can proceed from there.

Regards,

--
Giovanni