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.
