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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2073A599-E7CA-476A-9B4B-7BC76B454B9A@fb.com>
Date:   Thu, 17 Sep 2020 10:28:15 -0400
From:   "Chris Mason" <clm@...com>
To:     Christoph Hellwig <hch@...radead.org>
CC:     Rik van Riel <riel@...riel.com>, Nick Terrell <terrelln@...com>,
        Nick Terrell <nickrterrell@...il.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        <linux-crypto@...r.kernel.org>,
        Btrfs BTRFS <linux-btrfs@...r.kernel.org>,
        <squashfs-devel@...ts.sourceforge.net>,
        <linux-f2fs-devel@...ts.sourceforge.net>,
        <linux-kernel@...r.kernel.org>, Kernel Team <Kernel-team@...com>,
        Petr Malat <oss@...at.biz>, Johannes Weiner <jweiner@...com>,
        Niket Agarwal <niketa@...com>, Yann Collet <cyan@...com>
Subject: Re: [PATCH 5/9] btrfs: zstd: Switch to the zstd-1.4.6 API

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