6906598f | 29-Jan-2023 |
Jonathan Neuschäfer <j.neuschaefer@gmx.net> |
zstd: Fix definition of assert()
assert(x) should emit a warning if x is false. WARN_ON(x) emits a warning if x is true. Thus, assert(x) should be defined as WARN_ON(!x) rather than WARN_ON(x).
Sig
zstd: Fix definition of assert()
assert(x) should emit a warning if x is false. WARN_ON(x) emits a warning if x is true. Thus, assert(x) should be defined as WARN_ON(!x) rather than WARN_ON(x).
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Signed-off-by: Nick Terrell <terrelln@fb.com>
show more ...
|
038505c4 | 15-Feb-2023 |
Nick Terrell <terrelln@fb.com> |
lib: zstd: Backport fix for in-place decompression
Backport the relevant part of upstream commit 5b266196 [0].
This fixes in-place decompression for x86-64 kernel decompression. It uses a bound of
lib: zstd: Backport fix for in-place decompression
Backport the relevant part of upstream commit 5b266196 [0].
This fixes in-place decompression for x86-64 kernel decompression. It uses a bound of 131072 + (uncompressed_size >> 8), which can be violated after upstream commit 6a7ede3d [1], as zstd can use part of the output buffer as temporary storage, and without this patch needs a bound of ~262144.
The fix is for zstd to detect that the input and output buffers overlap, so that zstd knows it can't use the overlapping portion of the output buffer as tempoary storage. If the margin is not large enough, this will ensure that zstd will fail the decompression, rather than overwriting part of the input data, and causing corruption.
This fix has been landed upstream and is in release v1.5.4. That commit also adds unit and fuzz tests to verify that the margin we use is respected, and correct. That means that the fix is well tested upstream.
I have not been able to reproduce the potential bug in x86-64 kernel decompression locally, nor have I recieved reports of failures to decompress the kernel. It is possible that compression saves enough space to make it very hard for the issue to appear.
I've boot tested the zstd compressed kernel on x86-64 and i386 with this patch, which uses in-place decompression, and sanity tested zstd compression in btrfs / squashfs to make sure that we don't see any issues, but other uses of zstd shouldn't be affected, because they don't use in-place decompression.
Thanks to Vasily Gorbik <gor@linux.ibm.com> for debugging a related issue on s390, which was triggered by the same commit, but was a bug in how __decompress() was called [2]. And to Sasha Levin <sashal@kernel.org> for the CC alerting me of the issue.
[0] https://github.com/facebook/zstd/commit/5b266196a41e6a15e21bd4f0eeab43b938db1d90 [1] https://github.com/facebook/zstd/commit/6a7ede3dfccbf3e0a5928b4224a039c260dcff72 [2] https://lore.kernel.org/r/patch-1.thread-41c676.git-41c676c2d153.your-ad-here.call-01675030179-ext-9637@work.hours
CC: Vasily Gorbik <gor@linux.ibm.com> CC: Heiko Carstens <hca@linux.ibm.com> CC: Sasha Levin <sashal@kernel.org> CC: Yann Collet <cyan@fb.com> Signed-off-by: Nick Terrell <terrelln@fb.com>
show more ...
|
2aa14b1a | 17-Oct-2022 |
Nick Terrell <terrelln@fb.com> |
zstd: import usptream v1.5.2
Updates the kernel's zstd library to v1.5.2, the latest zstd release. The upstream tag it is updated to is `v1.5.2-kernel`, which contains several cherry-picked commits
zstd: import usptream v1.5.2
Updates the kernel's zstd library to v1.5.2, the latest zstd release. The upstream tag it is updated to is `v1.5.2-kernel`, which contains several cherry-picked commits on top of the v1.5.2 release which are required for the kernel update. I will create this tag once the PR is ready to merge, until then reference the temporary upstream branch `v1.5.2-kernel-cherrypicks`.
I plan to submit this patch as part of the v6.2 merge window.
I've done basic build testing & testing on x86-64, i386, and aarch64. I'm merging these patches into my `zstd-next` branch, which is pulled into `linux-next` for further testing.
I've benchmarked BtrFS with zstd compression on a x86-64 machine, and saw these results. Decompression speed is a small win across the board. The lower compression levels 1-4 see both compression speed and compression ratio wins. The higher compression levels see a small compression speed loss and about neutral ratio. I expect the lower compression levels to be used much more heavily than the high compression levels, so this should be a net win.
Level CTime DTime Ratio 1 -2.95% -1.1% -0.7% 3 -3.5% -1.2% -0.5% 5 +3.7% -1.0% +0.0% 7 +3.2% -0.9% +0.0% 9 -4.3% -0.8% +0.1%
Signed-off-by: Nick Terrell <terrelln@fb.com>
show more ...
|
4782c725 | 14-Oct-2022 |
Nick Terrell <terrelln@fb.com> |
zstd: Move zstd-common module exports to zstd_common_module.c
The zstd codebase is imported from the upstream zstd repo, and is over-written on every update. Upstream keeps the kernel specific code
zstd: Move zstd-common module exports to zstd_common_module.c
The zstd codebase is imported from the upstream zstd repo, and is over-written on every update. Upstream keeps the kernel specific code separate from the main library. So the module definition is moved into the zstd_common_module.c file. This matches the pattern followed by the zstd-compress and zstd-decompress files.
I've done build and boot testing on x86-64, i386, and aarch64. I've verified that zstd built both as modules and built-in build and boot.
Signed-off-by: Nick Terrell <terrelln@fb.com>
show more ...
|
7416cdc9 | 16-Nov-2021 |
Nick Terrell <terrelln@fb.com> |
lib: zstd: Don't add -O3 to cflags
After the update to zstd-1.4.10 passing -O3 is no longer necessary to get good performance from zstd. Using the default optimization level -O2 is sufficient to get
lib: zstd: Don't add -O3 to cflags
After the update to zstd-1.4.10 passing -O3 is no longer necessary to get good performance from zstd. Using the default optimization level -O2 is sufficient to get good performance.
I've measured no significant change to compression speed, and a ~1% decompression speed loss, which is acceptable.
This fixes the reported parisc -Wframe-larger-than=1536 errors [0]. The gcc-8-hppa-linux-gnu compiler performed very poorly with -O3, generating stacks that are ~3KB. With -O2 these same functions generate stacks in the < 100B, completely fixing the problem. Function size deltas are listed below:
ZSTD_compressBlock_fast_extDict_generic: 3800 -> 68 ZSTD_compressBlock_fast: 2216 -> 40 ZSTD_compressBlock_fast_dictMatchState: 1848 -> 64 ZSTD_compressBlock_doubleFast_extDict_generic: 3744 -> 76 ZSTD_fillDoubleHashTable: 3252 -> 0 ZSTD_compressBlock_doubleFast: 5856 -> 36 ZSTD_compressBlock_doubleFast_dictMatchState: 5380 -> 84 ZSTD_copmressBlock_lazy2: 2420 -> 72
Additionally, this improves the reported code bloat [1]. With gcc-11 bloat-o-meter shows an 80KB code size improvement:
``` > ../scripts/bloat-o-meter vmlinux.old vmlinux add/remove: 31/8 grow/shrink: 24/155 up/down: 25734/-107924 (-82190) Total: Before=6418562, After=6336372, chg -1.28% ```
Compared to before the zstd-1.4.10 update we see a total code size regression of 105KB, down from 374KB at v5.16-rc1:
``` > ../scripts/bloat-o-meter vmlinux.old vmlinux add/remove: 292/62 grow/shrink: 56/88 up/down: 235009/-127487 (107522) Total: Before=6228850, After=6336372, chg +1.73% ```
[0] https://lkml.org/lkml/2021/11/15/710 [1] https://lkml.org/lkml/2021/11/14/189
Link: https://lore.kernel.org/r/20211117014949.1169186-4-nickrterrell@gmail.com/ Link: https://lore.kernel.org/r/20211117201459.1194876-4-nickrterrell@gmail.com/
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Nick Terrell <terrelln@fb.com>
show more ...
|
1974990c | 15-Nov-2021 |
Nick Terrell <terrelln@fb.com> |
lib: zstd: Don't inline functions in zstd_opt.c
`zstd_opt.c` contains the match finder for the highest compression levels. These levels are already very slow, and are unlikely to be used in the kern
lib: zstd: Don't inline functions in zstd_opt.c
`zstd_opt.c` contains the match finder for the highest compression levels. These levels are already very slow, and are unlikely to be used in the kernel. If they are used, they shouldn't be used in latency sensitive workloads, so slowing them down shouldn't be a big deal.
This saves 188 KB of the 288 KB regression reported by Geert Uytterhoeven [0]. I've also opened an issue upstream [1] so that we can properly tackle the code size issue in `zstd_opt.c` for all users, and can hopefully remove this hack in the next zstd version we import.
Bloat-o-meter output on x86-64:
``` > ../scripts/bloat-o-meter vmlinux.old vmlinux add/remove: 6/5 grow/shrink: 1/9 up/down: 16673/-209939 (-193266) Function old new delta ZSTD_compressBlock_opt_generic.constprop - 7559 +7559 ZSTD_insertBtAndGetAllMatches - 6304 +6304 ZSTD_insertBt1 - 1731 +1731 ZSTD_storeSeq - 693 +693 ZSTD_BtGetAllMatches - 255 +255 ZSTD_updateRep - 128 +128 ZSTD_updateTree 96 99 +3 ZSTD_insertAndFindFirstIndexHash3 81 - -81 ZSTD_setBasePrices.constprop 98 - -98 ZSTD_litLengthPrice.constprop 138 - -138 ZSTD_count 362 181 -181 ZSTD_count_2segments 1407 938 -469 ZSTD_insertBt1.constprop 2689 - -2689 ZSTD_compressBlock_btultra2 19990 423 -19567 ZSTD_compressBlock_btultra 19633 15 -19618 ZSTD_initStats_ultra 19825 - -19825 ZSTD_compressBlock_btopt 20374 12 -20362 ZSTD_compressBlock_btopt_extDict 29984 12 -29972 ZSTD_compressBlock_btultra_extDict 30718 15 -30703 ZSTD_compressBlock_btopt_dictMatchState 32689 12 -32677 ZSTD_compressBlock_btultra_dictMatchState 33574 15 -33559 Total: Before=6611828, After=6418562, chg -2.92% ```
[0] https://lkml.org/lkml/2021/11/14/189 [1] https://github.com/facebook/zstd/issues/2862
Link: https://lore.kernel.org/r/20211117014949.1169186-3-nickrterrell@gmail.com/ Link: https://lore.kernel.org/r/20211117201459.1194876-3-nickrterrell@gmail.com/
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Nick Terrell <terrelln@fb.com>
show more ...
|
0a8ea235 | 21-Oct-2021 |
Nathan Chancellor <nathan@kernel.org> |
lib: zstd: Add cast to silence clang's -Wbitwise-instead-of-logical
A new warning in clang warns that there is an instance where boolean expressions are being used with bitwise operators instead of
lib: zstd: Add cast to silence clang's -Wbitwise-instead-of-logical
A new warning in clang warns that there is an instance where boolean expressions are being used with bitwise operators instead of logical ones:
lib/zstd/decompress/huf_decompress.c:890:25: warning: use of bitwise '&' with boolean operands [-Wbitwise-instead-of-logical] (BIT_reloadDStreamFast(&bitD1) == BIT_DStream_unfinished) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
zstd does this frequently to help with performance, as logical operators have branches whereas bitwise ones do not.
To fix this warning in other cases, the expressions were placed on separate lines with the '&=' operator; however, this particular instance was moved away from that so that it could be surrounded by LIKELY, which is a macro for __builtin_expect(), to help with a performance regression, according to upstream zstd pull #1973.
Aside from switching to logical operators, which is likely undesirable in this instance, or disabling the warning outright, the solution is casting one of the expressions to an integer type to make it clear to clang that the author knows what they are doing. Add a cast to U32 to silence the warning. The first U32 cast is to silence an instance of -Wshorten-64-to-32 because __builtin_expect() returns long so it cannot be moved.
Link: https://github.com/ClangBuiltLinux/linux/issues/1486 Link: https://github.com/facebook/zstd/pull/1973 Reported-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Nick Terrell <terrelln@fb.com>
show more ...
|
e0c1b49f | 11-Sep-2020 |
Nick Terrell <terrelln@fb.com> |
lib: zstd: Upgrade to latest upstream zstd version 1.4.10
Upgrade to the latest upstream zstd version 1.4.10.
This patch is 100% generated from upstream zstd commit 20821a46f412 [0].
This patch is
lib: zstd: Upgrade to latest upstream zstd version 1.4.10
Upgrade to the latest upstream zstd version 1.4.10.
This patch is 100% generated from upstream zstd commit 20821a46f412 [0].
This patch is very large because it is transitioning from the custom kernel zstd to using upstream directly. The new zstd follows upstreams file structure which is different. Future update patches will be much smaller because they will only contain the changes from one upstream zstd release.
As an aid for review I've created a commit [1] that shows the diff between upstream zstd as-is (which doesn't compile), and the zstd code imported in this patch. The verion of zstd in this patch is generated from upstream with changes applied by automation to replace upstreams libc dependencies, remove unnecessary portability macros, replace `/**` comments with `/*` comments, and use the kernel's xxhash instead of bundling it.
The benefits of this patch are as follows: 1. Using upstream directly with automated script to generate kernel code. This allows us to update the kernel every upstream release, so the kernel gets the latest bug fixes and performance improvements, and doesn't get 3 years out of date again. The automation and the translated code are tested every upstream commit to ensure it continues to work. 2. Upgrades from a custom zstd based on 1.3.1 to 1.4.10, getting 3 years of performance improvements and bug fixes. On x86_64 I've measured 15% faster BtrFS and SquashFS decompression+read speeds, 35% faster kernel decompression, and 30% faster ZRAM decompression+read speeds. 3. Zstd-1.4.10 supports negative compression levels, which allow zstd to match or subsume lzo's performance. 4. Maintains the same kernel-specific wrapper API, so no callers have to be modified with zstd version updates.
One concern that was brought up was stack usage. Upstream zstd had already removed most of its heavy stack usage functions, but I just removed the last functions that allocate arrays on the stack. I've measured the high water mark for both compression and decompression before and after this patch. Decompression is approximately neutral, using about 1.2KB of stack space. Compression levels up to 3 regressed from 1.4KB -> 1.6KB, and higher compression levels regressed from 1.5KB -> 2KB. We've added unit tests upstream to prevent further regression. I believe that this is a reasonable increase, and if it does end up causing problems, this commit can be cleanly reverted, because it only touches zstd.
I chose the bulk update instead of replaying upstream commits because there have been ~3500 upstream commits since the 1.3.1 release, zstd wasn't ready to be used in the kernel as-is before a month ago, and not all upstream zstd commits build. The bulk update preserves bisectablity because bugs can be bisected to the zstd version update. At that point the update can be reverted, and we can work with upstream to find and fix the bug.
Note that upstream zstd release 1.4.10 doesn't exist yet. I have cut a staging branch at 20821a46f412 [0] and will apply any changes requested to the staging branch. Once we're ready to merge this update I will cut a zstd release at the commit we merge, so we have a known zstd release in the kernel.
The implementation of the kernel API is contained in zstd_compress_module.c and zstd_decompress_module.c.
[0] https://github.com/facebook/zstd/commit/20821a46f4122f9abd7c7b245d28162dde8129c9 [1] https://github.com/terrelln/linux/commit/e0fa481d0e3df26918da0a13749740a1f6777574
Signed-off-by: Nick Terrell <terrelln@fb.com> Tested By: Paul Jones <paul@pauljones.id.au> Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v13.0.0 on x86-64 Tested-by: Jean-Denis Girard <jd.girard@sysnux.pf>
show more ...
|
2479b523 | 14-Sep-2020 |
Nick Terrell <terrelln@fb.com> |
lib: zstd: Add decompress_sources.h for decompress_unzstd
Adds decompress_sources.h which includes every .c file necessary for zstd decompression. This is used in decompress_unzstd.c so the internal
lib: zstd: Add decompress_sources.h for decompress_unzstd
Adds decompress_sources.h which includes every .c file necessary for zstd decompression. This is used in decompress_unzstd.c so the internal structure of the library isn't exposed.
This allows us to upgrade the zstd library version without modifying any callers. Instead we just need to update decompress_sources.h.
Signed-off-by: Nick Terrell <terrelln@fb.com> Tested By: Paul Jones <paul@pauljones.id.au> Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v13.0.0 on x86-64 Tested-by: Jean-Denis Girard <jd.girard@sysnux.pf>
show more ...
|
36f9ff9e | 19-Nov-2020 |
Gustavo A. R. Silva <gustavoars@kernel.org> |
lib: Fix fall-through warnings for Clang
In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple warnings by explicitly adding multiple break statements instead of letting the code f
lib: Fix fall-through warnings for Clang
In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple warnings by explicitly adding multiple break statements instead of letting the code fall through to the next case, and by replacing a number of /* fall through */ comments with the new pseudo-keyword macro fallthrough.
Notice that Clang doesn't recognize /* Fall through */ comments as implicit fall-through markings.
Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
show more ...
|