Re: [PATCH 3/3] raid6: simplify CPU feature checks

[Eric Biggers <ebiggers@kernel.org>]

On Mon, Oct 26, 2026 at 02:57:04PM +0000, David Laight wrote:
> On Sun, 25 Oct 2026 12:12:07 -0700
> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > Remove unnecessary checks for features that are prerequisites for other
> > features, such as AVX which is a prerequisite for AVX2.
> > 
> > (Note: in most cases arch/x86/kernel/cpu/cpuid-deps.c warns about
> > missing prerequisites rather than enforcing them.  This is intentional,
> > and it does not mean the prerequisites should be checked elsewhere.)
> > 
> > Then, move the raid6_recov_algo_add() lines into the appropriate places
> > just above so that the same CPU features aren't checked multiple times.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> >  lib/raid/raid6/x86/pq_arch.h | 51 ++++++++----------------------------
> >  1 file changed, 11 insertions(+), 40 deletions(-)
> > 
> > diff --git a/lib/raid/raid6/x86/pq_arch.h b/lib/raid/raid6/x86/pq_arch.h
> > index 4a863b253b909..418e4d4a2930f 100644
> > --- a/lib/raid/raid6/x86/pq_arch.h
> > +++ b/lib/raid/raid6/x86/pq_arch.h
> > @@ -16,63 +16,34 @@ extern const struct raid6_recov_calls raid6_recov_ssse3;
> >  extern const struct raid6_recov_calls raid6_recov_avx2;
> >  extern const struct raid6_recov_calls raid6_recov_avx512;
> >  
> > ...
> >  static __always_inline void __init arch_raid6_init(void)
> >  {
> > -	if (raid6_has_avx2()) {
> > +	if (boot_cpu_has(X86_FEATURE_AVX2)) {
> >  		raid6_algo_add(&raid6_avx2x1);
> >  		raid6_algo_add(&raid6_avx2x2);
> >  		if (IS_ENABLED(CONFIG_X86_64))
> >  			raid6_algo_add(&raid6_avx2x4);
> > -		if (raid6_has_avx512()) {
> > +		if (boot_cpu_has(X86_FEATURE_AVX512BW) &&
> > +		    boot_cpu_has(X86_FEATURE_AVX512VL) &&
> > +		    boot_cpu_has(X86_FEATURE_AVX512DQ)) {
> >  			raid6_algo_add(&raid6_avx512x1);
> >  			raid6_algo_add(&raid6_avx512x2);
> >  			if (IS_ENABLED(CONFIG_X86_64))
> >  				raid6_algo_add(&raid6_avx512x4);
> > +			raid6_recov_algo_add(&raid6_recov_avx512);
> > +		} else {
> > +			raid6_recov_algo_add(&raid6_recov_avx2);
> 
> Is that right (you haven't changed it)?
> You get both the avx2 and axv512 normal algorithms, but only one
> of the recov ones.
> 
> I presume these are added in addition to the generic C code and
> some startup test decides which is fastest and so used?

Yes, it's unchanged from before.

The raid6 library implements runtime benchmark-based selection for
struct raid6_algo, but not for struct raid6_recov_algo where only one
can be registered.  As always, you're welcome to check the code rather
than speculate; see lib/raid/raid6/algos.c.

Yes, it's inconsistent.  However, changing that is outside the scope of
this series.  (FWIW, none of the crypto or CRC code uses runtime
benchmark-based selection.  I think it's not worth it.)

- Eric