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.
