Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbounded preemption-disabled region in `xor_benchmark` causes softlockup panics and RCU stall warnings on slower architectures.
--
--- Patch [4]: [PATCH 4/8] xor/kunit: add a benchmark ---
commit f403ad2f80a6eb7a690f9b948152ad30281caf15
Author: Christoph Hellwig <[email protected]>
xor/kunit: add a benchmark
This commit adds a benchmark to the XOR KUnit test suite to evaluate
performance across varying block sizes and numbers of disks. The benchmark
reports runtime speeds in GB/s.
> diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
> --- a/lib/raid/xor/tests/xor_kunit.c
> +++ b/lib/raid/xor/tests/xor_kunit.c
[ ... ]
> +static void xor_benchmark(struct kunit *test)
> +{
[ ... ]
> + for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
> + unsigned int len = len_to_test[j];
> + const unsigned long num_iters = 1000;
> +
> + KUNIT_ASSERT_GT(test, len, 0);
> + KUNIT_ASSERT_LE(test, len, XOR_KUNIT_MAX_BYTES);
> +
> + preempt_disable();
> + t = ktime_get_ns();
> + for (l = 0; l < num_iters; l++)
> + xor_gen(test_dest, test_buffers, nr, len);
> + t = max(ktime_get_ns() - t, 1);
> + preempt_enable();
[Severity: High]
Will this preemption-disabled region cause softlockup panics or RCU stalls
on slower architectures?
In xor_benchmark(), preempt_disable() is called before processing 1,000
iterations of up to 32 blocks of 16KB each. On a slow emulated architecture
without hardware acceleration, this loop will synchronously process over
500 MB of memory.
If the memory bandwidth is low enough, could this tight loop exceed the
default 20-second softlockup and 21-second RCU stall timeouts, resulting
in a kernel panic during CI testing?
Would it be safer to place the preemption-disabled block inside the
iteration loop and accumulate the time, or dynamically scale the number
of iterations based on a time threshold so the scheduler has a chance to run?
> +
> + speed[j] = div64_u64((u64)len * num_iters * nr, t);
> + }
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.