From: David Sterba <dsterba@suse.cz>
To: Chris Mason <clm@fb.com>
Cc: Christoph Hellwig <hch@infradead.org>,
Nick Terrell <nickrterrell@gmail.com>,
Herbert Xu <herbert@gondor.apana.org.au>,
linux-crypto@vger.kernel.org, linux-btrfs@vger.kernel.org,
squashfs-devel@lists.sourceforge.net,
linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, Kernel Team <Kernel-team@fb.com>,
Nick Terrell <terrelln@fb.com>, Petr Malat <oss@malat.biz>,
Johannes Weiner <jweiner@fb.com>, Niket Agarwal <niketa@fb.com>,
Yann Collet <cyan@fb.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v5 1/9] lib: zstd: Add zstd compatibility wrapper
Date: Tue, 10 Nov 2020 16:25:41 +0100 [thread overview]
Message-ID: <20201110152541.GK6756@twin.jikos.cz> (raw)
In-Reply-To: <D9338FE4-1518-4C7B-8C23-DBDC542DAC35@fb.com>
On Mon, Nov 09, 2020 at 02:01:41PM -0500, Chris Mason wrote:
> On 6 Nov 2020, at 13:38, Christoph Hellwig wrote:
> > You just keep resedning this crap, don't you? Haven't you been told
> > multiple times to provide a proper kernel API by now?
>
> You do consistently ask for a shim layer, but you haven’t explained
> what we gain by diverging from the documented and tested API of the
> upstream zstd project. It’s an important discussion given that we
> hope to regularly update the kernel side as they make improvements in
> zstd.
>
> The only benefit described so far seems to be camelcase related, but if
> there are problems in the API beyond that, I haven’t seen you describe
> them. I don’t think the camelcase alone justifies the added costs of
> the shim.
The API change in this patchset is adding churn that wouldn't be
necessary if there were an upstream<->kernel API from the beginning.
The patch 5/9 is almost entirely renaming just some internal identifiers
- ZSTD_CStreamWorkspaceBound(params.cParams),
- ZSTD_DStreamWorkspaceBound(ZSTD_BTRFS_MAX_INPUT));
+ ZSTD_estimateCStreamSize_usingCParams(params.cParams),
+ ZSTD_estimateDStreamSize(ZSTD_BTRFS_MAX_INPUT));
plus updating the names in the error strings. The compression API that
filesystems need is simple:
- set up workspace and parameters
- compress buffer
- decompress buffer
We really should not care if upstream has 3 functions for initializing
stream (ZSTD_initCStream/ZSTD_initStaticCStream/ZSTD_initCStream_advanced),
or if the name changes again in the future.
This should not require explicit explanation, this should be a natural
requirement especially for separate projects that don't share the same
coding style but have to be integrated in some way.
next prev parent reply other threads:[~2020-11-10 15:27 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-03 6:05 [GIT PULL][PATCH v5 0/9] Update to zstd-1.4.6 Nick Terrell
2020-11-03 6:05 ` [PATCH v5 1/9] lib: zstd: Add zstd compatibility wrapper Nick Terrell
2020-11-06 18:38 ` Christoph Hellwig
2020-11-09 19:01 ` Chris Mason
2020-11-10 15:25 ` David Sterba [this message]
2020-11-10 19:33 ` Nick Terrell
2020-11-10 18:39 ` Christoph Hellwig
2020-11-10 19:24 ` Chris Mason
2020-11-16 16:52 ` Christoph Hellwig
2020-11-10 20:38 ` Nick Terrell
2020-11-03 6:05 ` [PATCH v5 2/9] lib: zstd: Add decompress_sources.h for decompress_unzstd Nick Terrell
2020-11-03 6:05 ` [PATCH v5 4/9] crypto: zstd: Switch to zstd-1.4.6 API Nick Terrell
2020-11-03 6:05 ` [PATCH v5 5/9] btrfs: zstd: Switch to the " Nick Terrell
2020-11-06 17:10 ` Josef Bacik
2020-11-06 18:36 ` Nick Terrell
2020-11-03 6:05 ` [PATCH v5 6/9] f2fs: " Nick Terrell
2020-11-03 6:05 ` [PATCH v5 7/9] squashfs: " Nick Terrell
2020-11-03 6:05 ` [PATCH v5 8/9] lib: unzstd: " Nick Terrell
2020-11-03 6:05 ` [PATCH v5 9/9] lib: zstd: Remove zstd compatibility wrapper Nick Terrell
2020-11-06 17:15 ` [GIT PULL][PATCH v5 0/9] Update to zstd-1.4.6 Josef Bacik
2020-11-06 18:51 ` Nick Terrell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201110152541.GK6756@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=Kernel-team@fb.com \
--cc=clm@fb.com \
--cc=cyan@fb.com \
--cc=gregkh@linuxfoundation.org \
--cc=hch@infradead.org \
--cc=herbert@gondor.apana.org.au \
--cc=jweiner@fb.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=nickrterrell@gmail.com \
--cc=niketa@fb.com \
--cc=oss@malat.biz \
--cc=squashfs-devel@lists.sourceforge.net \
--cc=terrelln@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).