Re: [PATCH] Use strict priority ranking for pq gen() benchmarking

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Dirk,


Am 29.12.21 um 23:36 schrieb Dirk Müller:
> On x86_64, currently 3 variants of AVX512, 3 variants of AVX2
> and 3 variants of SSE2 are benchmarked on initialization, taking
> between 144-153 jiffies. Over a hardware pool of various generations
> of intel cpus I could not find a single case where SSE2 won over
> AVX2 or AVX512. There are cases where AVX2 wins over AVX512.

Can the AVX2 wins over AVX512 be explained, or does it point to some 
implementation problem? By the way, Borislav did not give much credit to 
the benchmarks results [1].

> By giving AVXx variants higher priority over SSE, we can generally
> skip 3 benchmarks which speeds this up by 33% - 50%, depending on
> whether AVX512 is available.

Please give concrete timing numbers for one system you tested this on.

> Signed-off-by: Dirk Müller <dmueller@suse.de>
> ---
>   include/linux/raid/pq.h | 2 +-
>   lib/raid6/algos.c       | 2 +-
>   lib/raid6/avx2.c        | 6 +++---
>   lib/raid6/avx512.c      | 6 +++---
>   4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/raid/pq.h b/include/linux/raid/pq.h
> index 154e954b711d..d6e5a1feb947 100644
> --- a/include/linux/raid/pq.h
> +++ b/include/linux/raid/pq.h
> @@ -81,7 +81,7 @@ struct raid6_calls {
>   	void (*xor_syndrome)(int, int, int, size_t, void **);
>   	int  (*valid)(void);	/* Returns 1 if this routine set is usable */
>   	const char *name;	/* Name of this routine set */
> -	int prefer;		/* Has special performance attribute */
> +	int priority;		/* Relative priority ranking if non-zero */
>   };
>   
>   /* Selected algorithm */
> diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c
> index 889033b7fc0d..d1e8ff837a32 100644
> --- a/lib/raid6/algos.c
> +++ b/lib/raid6/algos.c
> @@ -151,7 +151,7 @@ static inline const struct raid6_calls *raid6_choose_gen(
>   	const struct raid6_calls *best;
>   
>   	for (bestgenperf = 0, best = NULL, algo = raid6_algos; *algo; algo++) {
> -		if (!best || (*algo)->prefer >= best->prefer) {
> +		if (!best || (*algo)->priority >= best->priority) {
>   			if ((*algo)->valid && !(*algo)->valid())
>   				continue;
>   
> diff --git a/lib/raid6/avx2.c b/lib/raid6/avx2.c
> index f299476e1d76..31be496b8c81 100644
> --- a/lib/raid6/avx2.c
> +++ b/lib/raid6/avx2.c
> @@ -132,7 +132,7 @@ const struct raid6_calls raid6_avx2x1 = {
>   	raid6_avx21_xor_syndrome,
>   	raid6_have_avx2,
>   	"avx2x1",
> -	1			/* Has cache hints */
> +	.priority = 2
>   };
>   
>   /*
> @@ -262,7 +262,7 @@ const struct raid6_calls raid6_avx2x2 = {
>   	raid6_avx22_xor_syndrome,
>   	raid6_have_avx2,
>   	"avx2x2",
> -	1			/* Has cache hints */
> +	.priority = 2
>   };
>   
>   #ifdef CONFIG_X86_64
> @@ -465,6 +465,6 @@ const struct raid6_calls raid6_avx2x4 = {
>   	raid6_avx24_xor_syndrome,
>   	raid6_have_avx2,
>   	"avx2x4",
> -	1			/* Has cache hints */
> +	.priority = 2
>   };
>   #endif
> diff --git a/lib/raid6/avx512.c b/lib/raid6/avx512.c
> index bb684d144ee2..63ae197c3294 100644
> --- a/lib/raid6/avx512.c
> +++ b/lib/raid6/avx512.c
> @@ -162,7 +162,7 @@ const struct raid6_calls raid6_avx512x1 = {
>   	raid6_avx5121_xor_syndrome,
>   	raid6_have_avx512,
>   	"avx512x1",
> -	1                       /* Has cache hints */
> +	.priority = 2
>   };
>   
>   /*
> @@ -319,7 +319,7 @@ const struct raid6_calls raid6_avx512x2 = {
>   	raid6_avx5122_xor_syndrome,
>   	raid6_have_avx512,
>   	"avx512x2",
> -	1                       /* Has cache hints */
> +	.priority = 2
>   };
>   
>   #ifdef CONFIG_X86_64
> @@ -557,7 +557,7 @@ const struct raid6_calls raid6_avx512x4 = {
>   	raid6_avx5124_xor_syndrome,
>   	raid6_have_avx512,
>   	"avx512x4",
> -	1                       /* Has cache hints */
> +	.priority = 2
>   };
>   #endif
>   

Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul


[1]: https://lore.kernel.org/all/20210406124126.GM17806@zn.tnic/