Re: [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context

[Linus Torvalds]

[ 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