Re: [PATCH 5/9] btrfs: zstd: Switch to the zstd-1.4.6 API

Chris Mason
2020-09-17T15:42:38Z

On 17 Sep 2020, at 6:04, Christoph Hellwig wrote:

> On Wed, Sep 16, 2020 at 09:35:51PM -0400, Rik van Riel wrote:
>>> One possibility is to have a kernel wrapper on top of the zstd API
>>> to
>>> make it
>>> more ergonomic. I personally don???t really see the value in it,
>>> since
>>> it adds
>>> another layer of indirection between zstd and the caller, but it
>>> could be done.
>>
>> Zstd would not be the first part of the kernel to
>> come from somewhere else, and have wrappers when
>> it gets integrated into the kernel. There certainly
>> is precedence there.
>>
>> It would be interesting to know what Christoph's
>> preference is.
>
> Yes, I think kernel wrappers would be a pretty sensible step forward.
> That also avoid the need to do strange upgrades to a new version,
> and instead we can just change APIs on a as-needed basis.

When we add wrappers, we end up creating a kernel specific API that
doesn’t match the upstream zstd docs, and it doesn’t leverage as
much of the zstd fuzzing and testing.

So we’re actually making kernel zstd slightly less usable in hopes
that our kernel specific part of the API is familiar enough to us that
it makes zstd more usable. There’s no way to compare the two until
the wrappers are done, but given the code today I’d prefer that we
focus on making it really easy to track upstream. I really understand
Christoph’s side here, but I’d rather ride a camel with the group
than go it alone.

I’d also much rather spend time on any problems where the structure of
the zstd APIs don’t fit the kernel’s needs. The btrfs streaming
compression/decompression looks pretty clean to me, but I think Johannes
mentioned some possibilities to improve things for zswap (optimizations
for page-at-atime). If there are places where the zstd memory
management or error handling don’t fit naturally into the kernel, that
would also be higher on my list.

Fixing those are probably going to be much easier if we’re close to
the zstd upstream, again so that we can leverage testing and long term
code maintenance done there.

-chris