[linux] Expose ZSTD_compressSequences*() in the kernel #4260
Conversation
|
Looks good to me, |
You should be able to fix the tests by moving this file: https://github.com/facebook/zstd/blob/dev/contrib/linux-kernel/test/include/asm/unaligned.h Into this directory: https://github.com/facebook/zstd/tree/dev/contrib/linux-kernel/test/include/linux This include directory fakes the Linux headers that Zstd depends on. |
contrib/linux-kernel/linux_zstd.h
Outdated
| * | ||
| * Return: Zero or an error, which can be checked using zstd_is_error(). | ||
| */ | ||
| size_t zstd_cctx_set_param(zstd_cctx *cctx, ZSTD_cParameter param, int value); |
There was a problem hiding this comment.
minor: We could also add a typedef for zstd_cparameter to give it a kernel-style name.
There was a problem hiding this comment.
I added a typedef for ZSTD_cParameter but I didn't add the documentation as the elements of the enum follow the ZSTD coding standard. Shall I redefine all parameters, or at least the one I need, using the kernel style?
In my driver I will have something like:
zstd_cctx_set_param(ctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters);
There was a problem hiding this comment.
I could go either way. I don't think we absolutely need to, given that we don't have many users of this API.
gcabiddu
force-pushed
the
compress_sequences_kernel
branch
from
a34fd5a to
f7c11d9
Compare
contrib/linux-kernel/linux_zstd.h
Outdated
| * | ||
| * Return: Zero or an error, which can be checked using zstd_is_error(). | ||
| */ | ||
| size_t zstd_cctx_set_param(zstd_cctx *cctx, ZSTD_cParameter param, int value); |
There was a problem hiding this comment.
I added a typedef for ZSTD_cParameter but I didn't add the documentation as the elements of the enum follow the ZSTD coding standard. Shall I redefine all parameters, or at least the one I need, using the kernel style?
In my driver I will have something like:
zstd_cctx_set_param(ctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters);
contrib/linux-kernel/linux_zstd.h
Outdated
| * Return: The compressed size or an error, which can be checked using | ||
| * zstd_is_error(). | ||
| */ | ||
| size_t zstd_compress_sequences(zstd_cctx *cctx, void* dst, size_t dst_capacity, |
There was a problem hiding this comment.
Shall we keep this even if it might end up not being used in the kernel?
There was a problem hiding this comment.
If we don't need it, and don't expect to need it, we can remove it. If we need to add it later we can.
There was a problem hiding this comment.
Thanks @terrelln. I updated the PR.
Who usually marks the comments as resolved? The submitter or the maintainer? Thanks.
Thanks @terrelln - I sorted it by moving that include as you suggested. |
| const zstd_sequence *in_seqs, size_t in_seqs_size, | ||
| const void* src, size_t src_size) | ||
| { | ||
| return ZSTD_compressSequences(cctx, dst, dst_capacity, in_seqs, |
There was a problem hiding this comment.
spaces instead of tabs and everywhere else
There was a problem hiding this comment.
This code will be ported to the kernel automatically. The kernel coding stiles uses tabs instead of spaces:
https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation
There was a problem hiding this comment.
oh, sorry, didn't notice it was in contrib.
Make the function ZSTD_compressSequencesAndLiterals() available in kernel space. This will be used by Intel QAT driver. Additionally, (1) expose the function ZSTD_CCtx_setParameter(), which is required to set parameters before calling ZSTD_compressSequencesAndLiterals(), (2) update the build process to include `compress/zstd_preSplit.o` and (3) replace `asm/unaligned.h` with `linux/unaligned.h`. Signed-off-by: Giovanni Cabiddu <[email protected]>
gcabiddu
force-pushed
the
compress_sequences_kernel
branch
from
f7c11d9 to
92be4be
Compare
Make the functions
ZSTD_compressSequences(),ZSTD_compressSequencesAndLiterals()andZSTD_CCtx_setParameter()available in kernel space.These will be used in future by the Intel QAT driver.
Note that:
ZSTD_compressSequencesAndLiterals()is sufficient for QAT. We could just expose that function and notZSTD_compressSequences().#include <asm/unaligned.h>should be replaced with#include <linux/unaligned.h>. I haven't made that change yet as it was causing some regressions in the tests.