* [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context
@ 2026-08-26 12:25 Usama Arif
2026-08-26 12:25 ` [PATCH 1/3] zstd: use ZSTD_cpuSupportsBmi2() in ZSTD_initStaticCCtx() Usama Arif
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Usama Arif @ 2026-08-26 12:25 UTC (permalink / raw)
To: dsterba, linux-kernel, terrelln, linux-crypto, yosry
Cc: hannes, nphamcs, chengming.zhou, shakeel.butt, kernel-team, Usama Arif
zstd picks between BMI2 and generic code paths at runtime, and issues
CPUID to decide every time a compression or decompression context is set
up. The answer cannot change while the kernel is running.
It is not a cold path: squashfs calls zstd_init_dstream() for every block
it decompresses, and erofs, btrfs, f2fs and crypto/zstd all initialise a
context per operation. Each probe is two serializing CPUID instructions
on x86.
Patch 1 routes ZSTD_initStaticCCtx() through ZSTD_cpuSupportsBmi2()
instead of open-coding the probe, which also fixes it testing for BMI2
without BMI1 - the bodies it selects are tagged
TARGET_ATTRIBUTE("lzcnt,bmi,bmi2"), so both are needed. No CPU in the
field implements BMI2 without BMI1, so this is latent. Patch 2 skips the
probe when DYNAMIC_BMI2 is 0, where every consumer ignores the flag
anyway. Patch 3 caches the result.
A 4 KiB crypto_acomp benchmark [1] in a one-vCPU KVM guest, twelve boots
of nine 30,000-operation rounds, median per-round mean over 108 rounds:
compress decompress
unpatched 16,756 ns 3,455 ns
patched 13,646 ns 1,002 ns
-3,110 ns -2,452 ns
(18.6%) (71.0%)
The main reason is because CPUID is an unconditional VM exit.
[1] https://gist.github.com/uarif1/5cf02f0e22c23f0d1b3d84348f12914c
Usama Arif (3):
zstd: use ZSTD_cpuSupportsBmi2() in ZSTD_initStaticCCtx()
zstd: skip the BMI2 probe when dynamic BMI2 dispatch is disabled
zstd: probe the CPU for BMI2 support only once
lib/zstd/common/zstd_internal.h | 22 ++++++++++++++++++++--
lib/zstd/compress/zstd_compress.c | 2 +-
2 files changed, 21 insertions(+), 3 deletions(-)
base-commit: 4b18edbd8e70f7e6860d56370f13244896d0f95c
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] zstd: use ZSTD_cpuSupportsBmi2() in ZSTD_initStaticCCtx()
2026-08-26 12:25 [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Usama Arif
@ 2026-08-26 12:25 ` Usama Arif
2026-08-26 12:25 ` [PATCH 2/3] zstd: skip the BMI2 probe when dynamic BMI2 dispatch is disabled Usama Arif
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Usama Arif @ 2026-08-26 12:25 UTC (permalink / raw)
To: dsterba, linux-kernel, terrelln, linux-crypto, yosry
Cc: hannes, nphamcs, chengming.zhou, shakeel.butt, kernel-team, Usama Arif
ZSTD_initStaticCCtx() open-codes its BMI2 probe as
ZSTD_cpuid_bmi2(ZSTD_cpuid()). Every other context setup -
ZSTD_initCCtx() and ZSTD_initDCtx_internal() - goes through
ZSTD_cpuSupportsBmi2(), which requires BMI1 as well as BMI2.
Requiring both is the correct test. The alternate function bodies the
flag selects are tagged BMI2_TARGET_ATTRIBUTE, which is
TARGET_ATTRIBUTE("lzcnt,bmi,bmi2"), so the compiler may emit BMI1 and
LZCNT instructions in them; BMI2 alone does not make that safe. No
shipping x86 CPU implements BMI2 without BMI1, so this is a latent
inconsistency rather than an observed failure.
Route the static path through the same helper. That also leaves a
single place where the probe is issued, which the following patches
build on.
No functional change on any CPU in the field.
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
lib/zstd/compress/zstd_compress.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/zstd/compress/zstd_compress.c b/lib/zstd/compress/zstd_compress.c
index c41a747413e01..4ab67d679bb63 100644
--- a/lib/zstd/compress/zstd_compress.c
+++ b/lib/zstd/compress/zstd_compress.c
@@ -142,7 +142,7 @@ ZSTD_CCtx* ZSTD_initStaticCCtx(void* workspace, size_t workspaceSize)
cctx->blockState.nextCBlock = (ZSTD_compressedBlockState_t*)ZSTD_cwksp_reserve_object(&cctx->workspace, sizeof(ZSTD_compressedBlockState_t));
cctx->tmpWorkspace = ZSTD_cwksp_reserve_object(&cctx->workspace, TMP_WORKSPACE_SIZE);
cctx->tmpWkspSize = TMP_WORKSPACE_SIZE;
- cctx->bmi2 = ZSTD_cpuid_bmi2(ZSTD_cpuid());
+ cctx->bmi2 = ZSTD_cpuSupportsBmi2();
return cctx;
}
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] zstd: skip the BMI2 probe when dynamic BMI2 dispatch is disabled
2026-08-26 12:25 [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Usama Arif
2026-08-26 12:25 ` [PATCH 1/3] zstd: use ZSTD_cpuSupportsBmi2() in ZSTD_initStaticCCtx() Usama Arif
@ 2026-08-26 12:25 ` Usama Arif
2026-08-26 12:25 ` [PATCH 3/3] zstd: probe the CPU for BMI2 support only once Usama Arif
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Usama Arif @ 2026-08-26 12:25 UTC (permalink / raw)
To: dsterba, linux-kernel, terrelln, linux-crypto, yosry
Cc: hannes, nphamcs, chengming.zhou, shakeel.butt, kernel-team, Usama Arif
When DYNAMIC_BMI2 is 0 - GCC older than 11, a non-x86 target, or a build
that already has BMI2 on globally - nothing reads the flag.
HUF_compress1X_usingCTable_internal(), FSE_decompress_wksp_bmi2(),
HUF_readStats_body() and the rest all resolve to the default body
without consulting it. ZSTD_cpuSupportsBmi2() nevertheless issues
CPUID, which on x86 is two serializing instructions, and throws the
answer away.
ZSTD_initDCtx_internal() already wraps its assignment in
#if DYNAMIC_BMI2 - it has to, because the dctx->bmi2 field is itself
declared under that #if. The two compress-side callers have no such
guard, and ZSTD_CCtx_s::bmi2 is unconditional, so they probe
unconditionally.
Put the test inside ZSTD_cpuSupportsBmi2() so every caller gets it
without having to remember.
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
lib/zstd/common/zstd_internal.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/zstd/common/zstd_internal.h b/lib/zstd/common/zstd_internal.h
index 52a79435caf66..41f190b533209 100644
--- a/lib/zstd/common/zstd_internal.h
+++ b/lib/zstd/common/zstd_internal.h
@@ -311,8 +311,13 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr,
*/
MEM_STATIC int ZSTD_cpuSupportsBmi2(void)
{
+#if DYNAMIC_BMI2
ZSTD_cpuid_t cpuid = ZSTD_cpuid();
return ZSTD_cpuid_bmi1(cpuid) && ZSTD_cpuid_bmi2(cpuid);
+#else
+ /* Nothing looks at the flag in this configuration. */
+ return 0;
+#endif
}
#endif /* ZSTD_CCOMMON_H_MODULE */
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] zstd: probe the CPU for BMI2 support only once
2026-08-26 12:25 [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Usama Arif
2026-08-26 12:25 ` [PATCH 1/3] zstd: use ZSTD_cpuSupportsBmi2() in ZSTD_initStaticCCtx() Usama Arif
2026-08-26 12:25 ` [PATCH 2/3] zstd: skip the BMI2 probe when dynamic BMI2 dispatch is disabled Usama Arif
@ 2026-08-26 12:25 ` Usama Arif
2026-08-26 17:10 ` [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Nhat Pham
2026-08-27 2:39 ` Eric Biggers
4 siblings, 0 replies; 8+ messages in thread
From: Usama Arif @ 2026-08-26 12:25 UTC (permalink / raw)
To: dsterba, linux-kernel, terrelln, linux-crypto, yosry
Cc: hannes, nphamcs, chengming.zhou, shakeel.butt, kernel-team, Usama Arif
ZSTD_cpuSupportsBmi2() issues CPUID on every context setup for an answer
that cannot change while the kernel is running. On x86 that is two
serializing CPUID instructions, and the callers are not rare:
squashfs, erofs, btrfs, f2fs and crypto/zstd all initialise a context
per operation, so a busy squashfs or zswap workload pays for it per
block or per page. Under KVM it is worse, because CPUID is an
unconditional VM exit.
Cache the result. Keeping the cache as a single int with a negative
sentinel, rather than a copy of ZSTD_cpuid_t, keeps it to one word: a
racing pair of probes computes the same value from the same CPUID leaf,
so the unsynchronized access is benign, and READ_ONCE()/WRITE_ONCE()
keep the compiler and KCSAN in agreement about that.
ZSTD_cpuSupportsBmi2() is MEM_STATIC, so each translation unit that
inlines it gets its own cache - three in a modular build, plus one in
each preboot decompressor. That is a handful of ints in bss and one
extra probe apiece, not worth avoiding.
The cached answer is the one the probing CPU reported. zstd could
already be migrated between the probe and the use of the flag, so this
does not introduce a heterogeneity question that was not there before.
Suggested-by: Yosry Ahmed <yosry@kernel.org>
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
lib/zstd/common/zstd_internal.h | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/lib/zstd/common/zstd_internal.h b/lib/zstd/common/zstd_internal.h
index 41f190b533209..b179f44753598 100644
--- a/lib/zstd/common/zstd_internal.h
+++ b/lib/zstd/common/zstd_internal.h
@@ -312,8 +312,21 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr,
MEM_STATIC int ZSTD_cpuSupportsBmi2(void)
{
#if DYNAMIC_BMI2
- ZSTD_cpuid_t cpuid = ZSTD_cpuid();
- return ZSTD_cpuid_bmi1(cpuid) && ZSTD_cpuid_bmi2(cpuid);
+ /*
+ * The answer cannot change over the life of the kernel, so probe
+ * once. Racing probes compute the same value, so the unsynchronized
+ * access is benign; the annotations are there to keep it that way.
+ */
+ static int supported = -1;
+ int s = READ_ONCE(supported);
+
+ if (s < 0) {
+ ZSTD_cpuid_t const cpuid = ZSTD_cpuid();
+
+ s = ZSTD_cpuid_bmi1(cpuid) && ZSTD_cpuid_bmi2(cpuid);
+ WRITE_ONCE(supported, s);
+ }
+ return s;
#else
/* Nothing looks at the flag in this configuration. */
return 0;
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context
2026-08-26 12:25 [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Usama Arif
` (2 preceding siblings ...)
2026-08-26 12:25 ` [PATCH 3/3] zstd: probe the CPU for BMI2 support only once Usama Arif
@ 2026-08-26 17:10 ` Nhat Pham
2026-08-27 2:39 ` Eric Biggers
4 siblings, 0 replies; 8+ messages in thread
From: Nhat Pham @ 2026-08-26 17:10 UTC (permalink / raw)
To: Usama Arif
Cc: dsterba, linux-kernel, terrelln, linux-crypto, yosry, hannes,
chengming.zhou, shakeel.butt, kernel-team
On Wed, Aug 26, 2026 at 5:26 AM Usama Arif <usama.arif@linux.dev> wrote:
>
> zstd picks between BMI2 and generic code paths at runtime, and issues
> CPUID to decide every time a compression or decompression context is set
> up. The answer cannot change while the kernel is running.
>
> It is not a cold path: squashfs calls zstd_init_dstream() for every block
> it decompresses, and erofs, btrfs, f2fs and crypto/zstd all initialise a
> context per operation. Each probe is two serializing CPUID instructions
> on x86.
>
> Patch 1 routes ZSTD_initStaticCCtx() through ZSTD_cpuSupportsBmi2()
> instead of open-coding the probe, which also fixes it testing for BMI2
> without BMI1 - the bodies it selects are tagged
> TARGET_ATTRIBUTE("lzcnt,bmi,bmi2"), so both are needed. No CPU in the
> field implements BMI2 without BMI1, so this is latent. Patch 2 skips the
> probe when DYNAMIC_BMI2 is 0, where every consumer ignores the flag
> anyway. Patch 3 caches the result.
>
> A 4 KiB crypto_acomp benchmark [1] in a one-vCPU KVM guest, twelve boots
> of nine 30,000-operation rounds, median per-round mean over 108 rounds:
>
> compress decompress
> unpatched 16,756 ns 3,455 ns
> patched 13,646 ns 1,002 ns
> -3,110 ns -2,452 ns
> (18.6%) (71.0%)
>
> The main reason is because CPUID is an unconditional VM exit.
>
> [1] https://gist.github.com/uarif1/5cf02f0e22c23f0d1b3d84348f12914c
Awesome! I'm no zstd expert, but this looks very nice. Thanks for
fixing it. I'll defer mostly to Nick (is the fb.com email still
working?) and Herbert for correctness checking.
BTW, I think this has been reported in the past:
https://lore.kernel.org/all/CAJxJ_jhvyMukPLThpgcdCMHwbp3b3bFvc4Va1cK79_3z6ubhwQ@mail.gmail.com/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context
2026-08-26 12:25 [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Usama Arif
` (3 preceding siblings ...)
2026-08-26 17:10 ` [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Nhat Pham
@ 2026-08-27 2:39 ` Eric Biggers
2026-08-27 14:21 ` Usama Arif
4 siblings, 1 reply; 8+ messages in thread
From: Eric Biggers @ 2026-08-27 2:39 UTC (permalink / raw)
To: Usama Arif
Cc: dsterba, linux-kernel, terrelln, linux-crypto, yosry, hannes,
nphamcs, chengming.zhou, shakeel.butt, kernel-team
On Wed, Aug 26, 2026 at 05:25:35AM -0700, Usama Arif wrote:
> zstd picks between BMI2 and generic code paths at runtime, and issues
> CPUID to decide every time a compression or decompression context is set
> up. The answer cannot change while the kernel is running.
>
> It is not a cold path: squashfs calls zstd_init_dstream() for every block
> it decompresses, and erofs, btrfs, f2fs and crypto/zstd all initialise a
> context per operation. Each probe is two serializing CPUID instructions
> on x86.
>
> Patch 1 routes ZSTD_initStaticCCtx() through ZSTD_cpuSupportsBmi2()
> instead of open-coding the probe, which also fixes it testing for BMI2
> without BMI1 - the bodies it selects are tagged
> TARGET_ATTRIBUTE("lzcnt,bmi,bmi2"), so both are needed. No CPU in the
> field implements BMI2 without BMI1, so this is latent. Patch 2 skips the
> probe when DYNAMIC_BMI2 is 0, where every consumer ignores the flag
> anyway. Patch 3 caches the result.
>
> A 4 KiB crypto_acomp benchmark [1] in a one-vCPU KVM guest, twelve boots
> of nine 30,000-operation rounds, median per-round mean over 108 rounds:
>
> compress decompress
> unpatched 16,756 ns 3,455 ns
> patched 13,646 ns 1,002 ns
> -3,110 ns -2,452 ns
> (18.6%) (71.0%)
>
> The main reason is because CPUID is an unconditional VM exit.
>
> [1] https://gist.github.com/uarif1/5cf02f0e22c23f0d1b3d84348f12914c
>
> Usama Arif (3):
> zstd: use ZSTD_cpuSupportsBmi2() in ZSTD_initStaticCCtx()
> zstd: skip the BMI2 probe when dynamic BMI2 dispatch is disabled
> zstd: probe the CPU for BMI2 support only once
>
> lib/zstd/common/zstd_internal.h | 22 ++++++++++++++++++++--
> lib/zstd/compress/zstd_compress.c | 2 +-
> 2 files changed, 21 insertions(+), 3 deletions(-)
Why not just use cpu_feature_enabled(X86_FEATURE_BMI2), which compiles
down to a static branch? All these issues are caused by lib/zstd/ using
its own custom CPU feature detection code, instead of the normal CPU
feature detection code that the rest of the kernel uses.
- Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context
2026-08-27 2:39 ` Eric Biggers
@ 2026-08-27 14:21 ` Usama Arif
0 siblings, 0 replies; 8+ messages in thread
From: Usama Arif @ 2026-08-27 14:21 UTC (permalink / raw)
To: Eric Biggers
Cc: dsterba, linux-kernel, terrelln, linux-crypto, yosry, hannes,
nphamcs, chengming.zhou, shakeel.butt, kernel-team
On 27/08/2026 03:39, Eric Biggers wrote:
> On Wed, Aug 26, 2026 at 05:25:35AM -0700, Usama Arif wrote:
>> zstd picks between BMI2 and generic code paths at runtime, and issues
>> CPUID to decide every time a compression or decompression context is set
>> up. The answer cannot change while the kernel is running.
>>
>> It is not a cold path: squashfs calls zstd_init_dstream() for every block
>> it decompresses, and erofs, btrfs, f2fs and crypto/zstd all initialise a
>> context per operation. Each probe is two serializing CPUID instructions
>> on x86.
>>
>> Patch 1 routes ZSTD_initStaticCCtx() through ZSTD_cpuSupportsBmi2()
>> instead of open-coding the probe, which also fixes it testing for BMI2
>> without BMI1 - the bodies it selects are tagged
>> TARGET_ATTRIBUTE("lzcnt,bmi,bmi2"), so both are needed. No CPU in the
>> field implements BMI2 without BMI1, so this is latent. Patch 2 skips the
>> probe when DYNAMIC_BMI2 is 0, where every consumer ignores the flag
>> anyway. Patch 3 caches the result.
>>
>> A 4 KiB crypto_acomp benchmark [1] in a one-vCPU KVM guest, twelve boots
>> of nine 30,000-operation rounds, median per-round mean over 108 rounds:
>>
>> compress decompress
>> unpatched 16,756 ns 3,455 ns
>> patched 13,646 ns 1,002 ns
>> -3,110 ns -2,452 ns
>> (18.6%) (71.0%)
>>
>> The main reason is because CPUID is an unconditional VM exit.
>>
>> [1] https://gist.github.com/uarif1/5cf02f0e22c23f0d1b3d84348f12914c
>>
>> Usama Arif (3):
>> zstd: use ZSTD_cpuSupportsBmi2() in ZSTD_initStaticCCtx()
>> zstd: skip the BMI2 probe when dynamic BMI2 dispatch is disabled
>> zstd: probe the CPU for BMI2 support only once
>>
>> lib/zstd/common/zstd_internal.h | 22 ++++++++++++++++++++--
>> lib/zstd/compress/zstd_compress.c | 2 +-
>> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> Why not just use cpu_feature_enabled(X86_FEATURE_BMI2), which compiles
> down to a static branch? All these issues are caused by lib/zstd/ using
> its own custom CPU feature detection code, instead of the normal CPU
> feature detection code that the rest of the kernel uses.
>
> - Eric
The only issue I saw with that was that zstd is a standalone library that
is imported, so I am not sure how using cpu_feature_enabled() would work
for the maintainers. If the maintainers are happy with it, I think the
below patch is much better.
From 0b21d945f579cbb5e41b92b4c9306e9d5bac84f1 Mon Sep 17 00:00:00 2001
From: Usama Arif <usama.arif@linux.dev>
Date: Thu, 27 Aug 2026 05:34:41 -0700
Subject: [PATCH] zstd: use kernel CPU feature detection on x86
Zstd currently probes CPUID while initializing each compression or
decompression context. This bypasses the x86 feature policy used by the
rest of the kernel and repeats a serializing instruction sequence for
every context.
Use cpu_feature_enabled() in the normal x86 compressor and decompressor
objects. Check ABM, BMI1, and BMI2 because the dispatched functions are
compiled with lzcnt, bmi, and bmi2. The checks are alternatives-patched
at boot.
The effect is especially visible under virtualization, where CPUID
normally causes a VM exit. A 4 KiB crypto_acomp benchmark was run in
one-vCPU KVM guests. Comparing the unpatched baseline with this
three-patch series, the median reported ns/op values were:
before after
compression 16,777 13,635 ns/op (-18.7%)
decompression 3,454 1,006 ns/op (-70.9%)
Keep the existing raw CPUID fallback for preboot and other builds which
cannot use the normal x86 feature infrastructure. Also retain the early
return when dynamic dispatch is disabled.
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
lib/zstd/Makefile | 5 +++++
lib/zstd/common/zstd_internal.h | 18 ++++++++++++++----
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/lib/zstd/Makefile b/lib/zstd/Makefile
index be218b5e0ed59..db1d1439f5447 100644
--- a/lib/zstd/Makefile
+++ b/lib/zstd/Makefile
@@ -42,3 +42,8 @@ zstd_common-y := \
common/error_private.o \
common/fse_decompress.o \
common/zstd_common.o \
+
+ifeq ($(CONFIG_X86),y)
+CFLAGS_compress/zstd_compress.o += -DZSTD_USE_KERNEL_CPU_FEATURES
+CFLAGS_decompress/zstd_decompress.o += -DZSTD_USE_KERNEL_CPU_FEATURES
+endif
diff --git a/lib/zstd/common/zstd_internal.h b/lib/zstd/common/zstd_internal.h
index 41f190b533209..f7d3bca650747 100644
--- a/lib/zstd/common/zstd_internal.h
+++ b/lib/zstd/common/zstd_internal.h
@@ -31,6 +31,11 @@
#include "fse.h"
#include "huf.h"
#include <linux/xxhash.h> /* XXH_reset, update, digest */
+
+/* Use the kernel's CPU feature policy in normal x86 kernel builds. */
+#if defined(ZSTD_USE_KERNEL_CPU_FEATURES)
+#include <asm/cpufeature.h>
+#endif
#define ZSTD_TRACE 0
/* ---- static assert (debug) --- */
@@ -311,12 +316,17 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr,
*/
MEM_STATIC int ZSTD_cpuSupportsBmi2(void)
{
-#if DYNAMIC_BMI2
- ZSTD_cpuid_t cpuid = ZSTD_cpuid();
- return ZSTD_cpuid_bmi1(cpuid) && ZSTD_cpuid_bmi2(cpuid);
-#else
+#if !DYNAMIC_BMI2
/* Nothing looks at the flag in this configuration. */
return 0;
+#elif defined(ZSTD_USE_KERNEL_CPU_FEATURES)
+ return cpu_feature_enabled(X86_FEATURE_ABM) &&
+ cpu_feature_enabled(X86_FEATURE_BMI1) &&
+ cpu_feature_enabled(X86_FEATURE_BMI2);
+#else
+ ZSTD_cpuid_t cpuid = ZSTD_cpuid();
+ return ZSTD_cpuid_bmi1(cpuid) &&
+ ZSTD_cpuid_bmi2(cpuid);
#endif
}
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context
@ 2026-08-28 20:19 Linus Torvalds
0 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2026-08-28 20:19 UTC (permalink / raw)
To: Usama Arif
Cc: Eric Biggers, David Sterba, Linux Kernel Mailing List,
Nick Terrell, Linux Crypto Mailing List, yosry, Johannes Weiner,
Nhat Pham, Chengming Zhou, Shakeel Butt, Kernel Team
[ Sorry for breaking threading - I have turned off IMAP access to my
mailbox, and so I have issues replying to lore messages sanely ]
On Thu, 27 Aug 2026, Usama Arif wrote:
>
> On 27/08/2026 03:39, Eric Biggers wrote:
> >
> > Why not just use cpu_feature_enabled(X86_FEATURE_BMI2), which compiles
> > down to a static branch? All these issues are caused by lib/zstd/ using
> > its own custom CPU feature detection code, instead of the normal CPU
> > feature detection code that the rest of the kernel uses.
Yes, please. The zlib code is just broken in how it makes up its own
random inine asm that is actively worse than what the kernel already
exposes.
> The only issue I saw with that was that zstd is a standalone library that
> is imported
Let's ignore that part, and just make it work right. The Zstd people
should think about this problem on their side.
> Keep the existing raw CPUID fallback for preboot and other builds which
> cannot use the normal x86 feature infrastructure. Also retain the early
> return when dynamic dispatch is disabled.
No, this is not great, that whole
cctx->bmi2 = ZSTD_cpuSupportsBmi2();
...
if (bmi2) ...
model in zstd needs to just die.
For the kernel, the whole dynamic "test a variable" model is simply wrong.
It should expand to
if (cpu_feature_enabled(X86_FEATURE_BMI2))
because for the kernel, that becomes a simple static branch.
When zstd goes through that variable, it loses that entirely and
instead turns it in a static assignment and then a dynamic test (well,
not "entirely" - with inlining it could still recover the right code).
So zstd really should be fixed to get rid of that bad
#if DYNAMIC_BMI2
if (bmi2) {
....
pattern entirely, and be taught to have a *helper* macro that just turns
into 0 for when DYNAMIC_BMI2 is not set, and turns into using that stupid
flag in user mode, and for the kernel it should just turn into that
"cpu_feature_enabled(X86_FEATURE_BMI2)"
Why does it check for both BMI1 and BMI2 anyway? And Arif added an
extra check for ABM. That all looks bogus. You can't have BMI2 without
having BMI1, so all this code looks completely bogus to begin with.
Nick? David?
Linus
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-28 20:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-26 12:25 [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Usama Arif
2026-08-26 12:25 ` [PATCH 1/3] zstd: use ZSTD_cpuSupportsBmi2() in ZSTD_initStaticCCtx() Usama Arif
2026-08-26 12:25 ` [PATCH 2/3] zstd: skip the BMI2 probe when dynamic BMI2 dispatch is disabled Usama Arif
2026-08-26 12:25 ` [PATCH 3/3] zstd: probe the CPU for BMI2 support only once Usama Arif
2026-08-26 17:10 ` [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context Nhat Pham
2026-08-27 2:39 ` Eric Biggers
2026-08-27 14:21 ` Usama Arif
2026-08-28 20:19 Linus Torvalds
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®