Re: [PATCH 7/8] raid6: improve the runtime selection benchmark

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The loop variable `i` is shadowed inside `raid6_choose_gen()`.
--

--- Patch [7]: [PATCH 7/8] raid6: improve the runtime selection benchmark ---
commit eff19093e5be2add0169cc2d5063db139e6f9d0c
Author: Christoph Hellwig <[email protected]>

raid6: improve the runtime selection benchmark

This commit modifies the RAID6 runtime selection benchmark to use plain
ktime_get_ns for timing. It also updates the benchmark to use 8 + 2 disks
for a more realistic load and reports throughput specifically on the data
disks.

> diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
> index 702e167a543af..011aa9d0eb3c5 100644
> --- a/lib/raid/raid6/algos.c
> +++ b/lib/raid/raid6/algos.c
[ ... ]
> -static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
> -		const int disks)
> +static int raid6_choose_gen(void *dptrs[NR_DISKS], const int disks)
>  {
> -	/* work on the second half of the disks */
> -	int start = (disks >> 1) - 1, stop = disks - 3;
>  	const struct raid6_calls *best = NULL;
>  	unsigned long bestgenperf = 0;
>  	unsigned int i;
>  
>  	for (i = 0; i < raid6_nr_algos; i++) {
>  		const struct raid6_calls *algo = raid6_algos[i];
> -		unsigned long perf = 0, j0, j1;
> +		unsigned long perf = 0;
> +		u64 t;
> +		int i;

[Severity: Low]
This isn't a bug, but does this inner declaration of int i shadow the
outer loop variable unsigned int i declared at the beginning of the
function?

This can trigger -Wshadow compiler warnings and might cause confusion when
reading the outer loop over raid6_nr_algos.

>  
>  		preempt_disable();
> -		j0 = jiffies;
> -		while ((j1 = jiffies) == j0)
> -			cpu_relax();
> -		while (time_before(jiffies,
> -				    j1 + (1<<RAID6_TIME_JIFFIES_LG2))) {
> -			algo->gen_syndrome(disks, PAGE_SIZE, *dptrs);
> -			perf++;
> -		}
> +		t = ktime_get_ns();
> +		for (i = 0; i < REPS; i++)
> +			algo->gen_syndrome(disks, BENCH_SIZE, dptrs);
> +		t = max(ktime_get_ns() - t, 1);
>  		preempt_enable();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7