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

From: Chris Mason
Date: Thu Sep 17 2020 - 11:42:40 EST

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.