* Re: [PATCH v4 4/7] md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
From: yu kuai @ 2026-07-16 9:24 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, magiclinan, xiao, axboe, vverma,
john.g.garry, martin.petersen, linux-kernel, yu kuai
Cc: linux-raid
In-Reply-To: <20260710101521.1714-5-abd.masalkhi@gmail.com>
在 2026/7/10 18:15, Abd-Alrhman Masalkhi 写道:
> raid10_write_request() drops the barrier before calling
> bio_submit_split_bioset() and reacquires it afterwards. This is no
> longer necessary because the split bio cannot re-enter
> raid10_write_request() while the barrier is held.
>
> The allow_barrier()/wait_barrier() pair was introduced by commit
> e820d55cb99d ("md: fix raid10 hang issue caused by barrier") when
> submit_flushes() called md_handle_request() directly, allowing re-entry
> into raid10_write_request(). Since v5.2, submit_flushes() has instead
> gone through submit_bio(), eliminating that recursion. submit_flushes()
> was later removed entirely by commit b75197e86e6d ("md: Remove flush
> handling").
>
> Currently, raid10_write_request() is only entered from the bio
> submission path, so the split bio submitted by bio_submit_split_bioset()
> cannot recurse back into wait_barrier().
>
> Remove the redundant allow_barrier()/wait_barrier() pair around
> bio_submit_split_bioset().
>
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
> ---
> Changes in v4:
> - No changes.
> - Link to v3:https://lore.kernel.org/linux-raid/20260708101341.473750-5-abd.masalkhi@gmail.com/
>
> Changes in v3:
> - No changes.
> - Link to v2:https://lore.kernel.org/linux-raid/20260628142420.1051027-5-abd.masalkhi@gmail.com/
>
> Changes in v2:
> - Expand the commit message to explain why the
> allow_barrier()/wait_barrier() pair is no longer needed.
> - Link to v1:https://lore.kernel.org/linux-raid/20260623072456.333437-5-abd.masalkhi@gmail.com/
> ---
> drivers/md/raid10.c | 2 --
> 1 file changed, 2 deletions(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: Subject: RFC: Read repair for md RAID1 after mirror read failures
From: Abd-Alrhman Masalkhi @ 2026-07-15 18:04 UTC (permalink / raw)
To: G.W. Kant - Hunenet B.V., linux-raid@vger.kernel.org,
yukuai@fygo.io
In-Reply-To: <m2jyqwduba.fsf@f8410a22-cd7b-4baf-acf5-82353315d5e5.fritz.box>
On Wed, Jul 15, 2026 at 19:41 +0200, Abd-Alrhman Masalkhi wrote:
> Hi Dion,
>
> On Wed, Jul 15, 2026 at 14:13 +0000, G. W. Kant wrote:
>> Hi Abd,
>>
>> On 7/15/26 7:27 AM, Abd-Alrhman Masalkhi wrote:
>>>> With today's 18–24 TB HDDs and backup/archive workloads, where data may
>>>> remain unchanged for years, latent media degradation seems increasingly
>>>> relevant. A successful read from the alternate mirror may be one of the
>>>> last opportunities to refresh such a sector before it becomes
>>>> permanently unreadable.
>>>>
>>> And Check/Repair is the right defense for cold archival data on large
>>> drives.
>>
>>
>> I have been looking into your suggestion that periodic check/repair is
>> the appropriate defense for cold archival data on large drives.
>>
>> Before I start reading the resync code, I would like to make sure I
>> understand what repair actually does.
>>
>> Suppose all mirrors read successfully and contain identical data. In
>> other words, there are no read errors and no mismatches.
>>
>> Does repair simply verify the data and move on, or does it also rewrite
>> each successfully read block to the corresponding LBA on every mirror?
>>
>> The distinction is important for what I have in mind.
>>
>> My concern is not recovery from an existing read error—that is already
>> handled by fix_read_error()—but rather the long-term aging of cold
>> magnetic recordings.
>>
>> If a sector is still readable today but is approaching the point where
>> the drive's ECC will eventually no longer be able to reconstruct it,
>> then a successful rewrite would effectively establish a new "write time"
>> for that sector. If the drive can no longer reliably store the data at
>> that location, the rewrite would also give the firmware an opportunity
>> to remap the sector.
>>
>> So my question is essentially:
>>
>> Does repair perform such a rewrite of successfully read blocks, or does
>> it only rewrite blocks when a read error or mismatch has already been
>> detected?
>>
> Okay, I see what you're getting at now. No, check/repair won't rewrite
> sectors that are read successfully and match across all mirrors. It only
> rewrites data when it encounters a mismatch or needs to recover from a
> read error.
>
> I think a full-array media refresh is a reasonable idea. but to my
> knowlage, modern drives already perform background media scans that
> refresh or remap marginal sectors, which covers part of this...
>
I'm not entirely sure about the media refresh part on modern drives...,
I'll have to look into that first.
>> Regards,
>>
>> Dion
>
> --
> Best Regards,
> Abd-Alrhman
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: Subject: RFC: Read repair for md RAID1 after mirror read failures
From: Abd-Alrhman Masalkhi @ 2026-07-15 17:53 UTC (permalink / raw)
To: Yu Kuai, G.W. Kant - Hunenet B.V., linux-raid@vger.kernel.org,
yu kuai
In-Reply-To: <2718fd27-9453-4dea-9d1d-13cfecef4e6c@fnnas.com>
Hi Kuai,
On Wed, Jul 15, 2026 at 15:27 +0800, Yu Kuai wrote:
> Hi,
>
> 在 2026/7/15 15:10, G.W. Kant - Hunenet B.V. 写道:
>> On 7/15/26 7:27 AM, Abd-Alrhman Masalkhi wrote:
>>>> In other words, the first successfully recovered read request could
>>>> automatically become a repair opportunity. The repair could even be
>>>> scheduled asynchronously, so the successful read is returned immediately
>>>> while the rewrite is performed in the background. Unlike a periodic
>>>> resync, this repair would be driven by an actual read failure, making it
>>>> targeted rather than rewriting the entire mirror.
>>>>
>>> Yes, md has had this for a long time. Look at fix_read_error() in
>>> raid1.c. It is called from handle_read_error() on any failed read. It
>>> reads from a healthy mirror and rewrites the bad region on the failing
>>> device, giving the drive a chance to rewrite or remap the sector. If the
>>> rewrite fails, it records a bad block. md does this synchronously under
>>> a frozen array, so it is not a missing feature.
>>>
>>> The likely reason you didn't see it is that your array was already
>>> degraded, so there was no healthy in-array copy for fix_read_error() to
>>> recover from. In your case, you were likely able to retrieve the data
>>> due to btrfs level redundancy, and md can't repair across arrays.
>>>
>>>> With today's 18–24 TB HDDs and backup/archive workloads, where data may
>>>> remain unchanged for years, latent media degradation seems increasingly
>>>> relevant. A successful read from the alternate mirror may be one of the
>>>> last opportunities to refresh such a sector before it becomes
>>>> permanently unreadable.
>>>>
>>> And Check/Repair is the right defense for cold archival data on large
>>> drives.
>> Thank you. fix_read_error() was exactly the piece I was looking for. I
>> had missed that md already performs read repair when another healthy
>> mirror is available.
>>
>> My situation was indeed different because the array had already lost one
>> member, so the successful read originated from Btrfs redundancy across
>> another md RAID1 rather than from the surviving md mirror.
>>
>> One question remains, though. check/repair periodically reads all
>> sectors, but as far as I understand it, successfully read sectors are
>> not rewritten. From a media aging perspective this is a different
>> problem than recovering from a read error. Have there ever been
>> discussions about a true media refresh pass that rewrites successfully
>> read sectors to refresh long-lived magnetic recordings?
>
> Perhaps this is what you looking for:
>
> [RFC v2 0/5] md/raid1: introduce a new sync action to repair badblocks -
> Zheng Qixing
> <https://lore.kernel.org/all/20260203061259.609206-1-zhengqixing@huaweicloud.com/>
>
> However, AFAIK, Qixing no longer work in this area and there will not be
> new version.
>
It seems interesting. Do you think it's worth carrying it forward?
>>
>> Best regards,
>> Dion Kant
>
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: Subject: RFC: Read repair for md RAID1 after mirror read failures
From: Abd-Alrhman Masalkhi @ 2026-07-15 17:41 UTC (permalink / raw)
To: G.W. Kant - Hunenet B.V., linux-raid@vger.kernel.org,
yukuai@fygo.io
In-Reply-To: <cff97a09-ee38-4785-9a45-1fd562fbaf5e@hunenet.nl>
Hi Dion,
On Wed, Jul 15, 2026 at 14:13 +0000, G. W. Kant wrote:
> Hi Abd,
>
> On 7/15/26 7:27 AM, Abd-Alrhman Masalkhi wrote:
>>> With today's 18–24 TB HDDs and backup/archive workloads, where data may
>>> remain unchanged for years, latent media degradation seems increasingly
>>> relevant. A successful read from the alternate mirror may be one of the
>>> last opportunities to refresh such a sector before it becomes
>>> permanently unreadable.
>>>
>> And Check/Repair is the right defense for cold archival data on large
>> drives.
>
>
> I have been looking into your suggestion that periodic check/repair is
> the appropriate defense for cold archival data on large drives.
>
> Before I start reading the resync code, I would like to make sure I
> understand what repair actually does.
>
> Suppose all mirrors read successfully and contain identical data. In
> other words, there are no read errors and no mismatches.
>
> Does repair simply verify the data and move on, or does it also rewrite
> each successfully read block to the corresponding LBA on every mirror?
>
> The distinction is important for what I have in mind.
>
> My concern is not recovery from an existing read error—that is already
> handled by fix_read_error()—but rather the long-term aging of cold
> magnetic recordings.
>
> If a sector is still readable today but is approaching the point where
> the drive's ECC will eventually no longer be able to reconstruct it,
> then a successful rewrite would effectively establish a new "write time"
> for that sector. If the drive can no longer reliably store the data at
> that location, the rewrite would also give the firmware an opportunity
> to remap the sector.
>
> So my question is essentially:
>
> Does repair perform such a rewrite of successfully read blocks, or does
> it only rewrite blocks when a read error or mismatch has already been
> detected?
>
Okay, I see what you're getting at now. No, check/repair won't rewrite
sectors that are read successfully and match across all mirrors. It only
rewrites data when it encounters a mismatch or needs to recover from a
read error.
I think a full-array media refresh is a reasonable idea. but to my
knowlage, modern drives already perform background media scans that
refresh or remap marginal sectors, which covers part of this...
> Regards,
>
> Dion
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: Subject: RFC: Read repair for md RAID1 after mirror read failures
From: Dragan Milivojević @ 2026-07-15 17:01 UTC (permalink / raw)
To: John Stoffel
Cc: G.W. Kant - Hunenet B.V., Abd-Alrhman Masalkhi,
linux-raid@vger.kernel.org, yukuai@fygo.io
In-Reply-To: <27223.46342.543928.404779@quad.stoffel.home>
> It's not a simple problem. For long term archiving, I would try to
> use something with RAID6 or more, and ideally have a filesystem which
> also checksums data. This is what ZFS does, and which bcachefs does
> as well, but since they're both out of kernel, I don't use either with
> my current home linux setup.
>
> But for long term storage, that could be the way to go. but then you
> need to periodically scrub your data, which means reading it all,
> checking the checksums and re-writing and fixing corruption.
>
> > My concern is not recovery from an existing read error—that is already
> > handled by fix_read_error()—but rather the long-term aging of cold
> > magnetic recordings.
>
> You really need three or more disks with multiple parity and checksums
> for long term storage.
dm-integrity ...
^ permalink raw reply
* Re: Subject: RFC: Read repair for md RAID1 after mirror read failures
From: John Stoffel @ 2026-07-15 16:27 UTC (permalink / raw)
To: G.W. Kant - Hunenet B.V.
Cc: Abd-Alrhman Masalkhi, linux-raid@vger.kernel.org, yukuai@fygo.io
In-Reply-To: <cff97a09-ee38-4785-9a45-1fd562fbaf5e@hunenet.nl>
>>>>> "G" == "G W Kant <- Hunenet B.V." <g.w.kant@hunenet.nl>> writes:
> Hi Abd,
> On 7/15/26 7:27 AM, Abd-Alrhman Masalkhi wrote:
>>> With today's 18–24 TB HDDs and backup/archive workloads, where data may
>>> remain unchanged for years, latent media degradation seems increasingly
>>> relevant. A successful read from the alternate mirror may be one of the
>>> last opportunities to refresh such a sector before it becomes
>>> permanently unreadable.
>>>
>> And Check/Repair is the right defense for cold archival data on large
>> drives.
> I have been looking into your suggestion that periodic check/repair is
> the appropriate defense for cold archival data on large drives.
> Before I start reading the resync code, I would like to make sure I
> understand what repair actually does.
> Suppose all mirrors read successfully and contain identical data. In
> other words, there are no read errors and no mismatches.
> Does repair simply verify the data and move on, or does it also rewrite
> each successfully read block to the corresponding LBA on every mirror?
> The distinction is important for what I have in mind.
So you have to also think of other situations. You have a RAID1
array, and one block gets corrupted, but is still readable. How do
you tell which block is the proper one? You need checksums or parity
or more copies to check against for consistency.
It's not a simple problem. For long term archiving, I would try to
use something with RAID6 or more, and ideally have a filesystem which
also checksums data. This is what ZFS does, and which bcachefs does
as well, but since they're both out of kernel, I don't use either with
my current home linux setup.
But for long term storage, that could be the way to go. but then you
need to periodically scrub your data, which means reading it all,
checking the checksums and re-writing and fixing corruption.
> My concern is not recovery from an existing read error—that is already
> handled by fix_read_error()—but rather the long-term aging of cold
> magnetic recordings.
You really need three or more disks with multiple parity and checksums
for long term storage.
> If a sector is still readable today but is approaching the point where
> the drive's ECC will eventually no longer be able to reconstruct it,
> then a successful rewrite would effectively establish a new "write time"
> for that sector. If the drive can no longer reliably store the data at
> that location, the rewrite would also give the firmware an opportunity
> to remap the sector.
> So my question is essentially:
> Does repair perform such a rewrite of successfully read blocks, or does
> it only rewrite blocks when a read error or mismatch has already been
> detected?
> Regards,
> Dion
^ permalink raw reply
* Re: [PATCH 4/8] xor/kunit: add a benchmark
From: sashiko-bot @ 2026-07-15 15:05 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: yukuai, linux-raid
In-Reply-To: <20260715144825.95432-5-hch@lst.de>
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 <hch@lst.de>
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/20260715144825.95432-1-hch@lst.de?part=4
^ permalink raw reply
* Re: [PATCH 2/8] xor: improve the runtime selection benchmark
From: sashiko-bot @ 2026-07-15 15:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, yukuai
In-Reply-To: <20260715144825.95432-3-hch@lst.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing clock-tick synchronization with a fixed number of loop iterations causes zero-duration time deltas on coarse clocks, leading to artificial throughput inflation, integer overflow, and random algorithm selection.
--
--- Patch [2]: [PATCH 2/8] xor: improve the runtime selection benchmark ---
commit 2205984bf337f5c013e14971e819bb6346e19853
Author: Christoph Hellwig <hch@lst.de>
xor: improve the runtime selection benchmark
This commit modifies the XOR block benchmark to use ktime_get_ns for timing
and switches to 4 + 1 disks for a more realistic load. It also reports the
throughput on the data disks instead of the parity disk.
> diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
> index 50931fbf0324e..f9d61b096b117 100644
> --- a/lib/raid/xor/xor-core.c
> +++ b/lib/raid/xor/xor-core.c
[ ... ]
> -#define BENCH_SIZE 4096
> +#define BENCH_SIZE SZ_4K
> +#define NR_SRCS 4
> #define REPS 800U
>
> -static void __init
> -do_xor_speed(struct xor_block_template *tmpl, void *b1, void *b2)
> +static void __init do_xor_speed(struct xor_block_template *tmpl, void *dest,
> + void *srcs[NR_SRCS])
> {
> - int speed;
> - unsigned long reps;
> - ktime_t min, start, t0;
> - void *srcs[1] = { b2 };
> + u64 t;
> + int i;
>
> preempt_disable();
> -
> - reps = 0;
> - t0 = ktime_get();
> - /* delay start until time has advanced */
> - while ((start = ktime_get()) == t0)
> - cpu_relax();
> - do {
> + t = ktime_get_ns();
> + for (i = 0; i < REPS; i++) {
> mb(); /* prevent loop optimization */
> - tmpl->xor_gen(b1, srcs, 1, BENCH_SIZE);
> + tmpl->xor_gen(dest, srcs, NR_SRCS, BENCH_SIZE);
> mb();
> - } while (reps++ < REPS || (t0 = ktime_get()) == start);
> - min = ktime_sub(t0, start);
> -
> + }
> + t = max(ktime_get_ns() - t, 1);
[Severity: High]
Does removing the clock-tick synchronization create an issue with low
resolution clocks?
If a high-resolution clocksource is not yet available during early boot,
the 800 loop iterations might complete within a single clock tick. If that
happens, the elapsed time t evaluates to 0 and gets clamped to 1.
> preempt_enable();
>
> - // bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
> - speed = (1000 * reps * BENCH_SIZE) / (unsigned int)ktime_to_ns(min);
> - tmpl->speed = speed;
> + /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
> + tmpl->speed = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
[Severity: High]
Can this division silently overflow the 32-bit signed integer tmpl->speed?
If t is clamped to 1 ns, it yields a quotient of approximately 13.1 billion.
This overflows the 32-bit integer and could result in negative or
astronomically high throughput values.
This might break the calibration logic and cause the kernel to select a
suboptimal XOR algorithm.
>
> - pr_info(" %-16s: %5d MB/sec\n", tmpl->name, speed);
> + pr_info(" %-16s: %5d MB/sec\n", tmpl->name, tmpl->speed);
> }
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715144825.95432-1-hch@lst.de?part=2
^ permalink raw reply
* Re: [PATCH 8/8] raid6/kunit: add a benchmark
From: sashiko-bot @ 2026-07-15 15:01 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, yukuai
In-Reply-To: <20260715144825.95432-9-hch@lst.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The benchmark loop disables preemption for an extended period, risking soft lockups and RCU stalls on slower architectures or emulators.
--
--- Patch [8]: [PATCH 8/8] raid6/kunit: add a benchmark ---
commit c8db04a9f21fef60a9c9e7ad0c98848e405330de
Author: Christoph Hellwig <hch@lst.de>
raid6/kunit: add a benchmark
This commit adds a benchmark to test the RAID 6 P/Q functions for more
representative block sizes and numbers of disks. The maximum allocation
size is split to allow benchmarking larger sizes, and results are reported
in GB/s.
> diff --git a/lib/raid/raid6/tests/raid6_kunit.c b/lib/raid/raid6/tests/raid6_kunit.c
> index 9f3e671a12241..7d0a1ec980013 100644
> --- a/lib/raid/raid6/tests/raid6_kunit.c
> +++ b/lib/raid/raid6/tests/raid6_kunit.c
[ ... ]
> @@ -229,6 +230,68 @@ static void raid6_test(struct kunit *test)
[ ... ]
> +static void raid6_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, RAID6_KUNIT_ALLOC_BYTES);
> +
> + preempt_disable();
> + t = ktime_get_ns();
> + for (l = 0; l < num_iters; l++)
> + raid6_gen_syndrome(nr_to_test[i],
> + len_to_test[j], test_buffers);
> + t = max(ktime_get_ns() - t, 1);
> + preempt_enable();
[Severity: Medium]
Is it safe to disable preemption across this entire 1000-iteration loop?
For the maximum test configuration of 32 disks and 16KB buffers, this loop
processes roughly 512MB of data in a single uninterruptible block. On slower
environments like emulators or User-Mode Linux (which are commonly used for
KUnit tests), could this exceed standard watchdog thresholds and trigger soft
lockups or RCU stalls?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715144825.95432-1-hch@lst.de?part=8
^ permalink raw reply
* Re: [PATCH 7/8] raid6: improve the runtime selection benchmark
From: sashiko-bot @ 2026-07-15 14:59 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: yukuai, linux-raid
In-Reply-To: <20260715144825.95432-8-hch@lst.de>
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 <hch@lst.de>
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/20260715144825.95432-1-hch@lst.de?part=7
^ permalink raw reply
* [PATCH 8/8] raid6/kunit: add a benchmark
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260715144825.95432-1-hch@lst.de>
Add a benchmark to test the RAID 6 P/Q functions for more representative
block sizes and numbers of disks. This splits the maximum alloc size
used for the benchmark from the max size for the kunit test because
recovery is currently limited to a single page at a time. Hopefully this
will be fixed soon.
The runtime numbers are reported in GB/s as the numbers of modern
implementations are basically unreadable as MB/s. This means
retro-architectures could report 0, but that is an easy tradeoff.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/Kconfig | 6 +++
lib/raid/raid6/tests/raid6_kunit.c | 66 +++++++++++++++++++++++++++++-
2 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index c4eeb7c716c8..01f007b2522c 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -60,6 +60,12 @@ config RAID6_PQ_KUNIT_TEST
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
+config RAID6_PQ_KUNIT_BENCHMARK
+ bool "Benchmark for RAID6 PQ"
+ depends on RAID6_PQ_KUNIT_TEST
+ help
+ Include benchmarks in the KUnit test suite for raid P/Q generation.
+
config RAID6_PQ_BENCHMARK
bool "Automatically choose fastest RAID6 PQ functions"
depends on RAID6_PQ
diff --git a/lib/raid/raid6/tests/raid6_kunit.c b/lib/raid/raid6/tests/raid6_kunit.c
index 9f3e671a1224..7d0a1ec98001 100644
--- a/lib/raid/raid6/tests/raid6_kunit.c
+++ b/lib/raid/raid6/tests/raid6_kunit.c
@@ -18,6 +18,7 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
#define RAID6_KUNIT_MAX_BUFFERS 64 /* Including P and Q */
#define RAID6_KUNIT_MAX_FAILURES 2
#define RAID6_KUNIT_MAX_BYTES PAGE_SIZE
+#define RAID6_KUNIT_ALLOC_BYTES SZ_16K
static struct rnd_state rng;
static void *test_buffers[RAID6_KUNIT_MAX_BUFFERS];
@@ -229,6 +230,68 @@ static void raid6_test(struct kunit *test)
raid6_test_one(test);
}
+static void raid6_benchmark(struct kunit *test)
+{
+ static const unsigned int nr_to_test[] = {
+ 4, 5, 6, 7, 8, 10, 12, 15, 16, 32,
+ };
+ static const unsigned int len_to_test[] = {
+ SZ_4K, SZ_16K,
+ };
+ unsigned int i, j, l;
+ u64 t;
+
+ if (!IS_ENABLED(CONFIG_RAID6_PQ_KUNIT_BENCHMARK))
+ kunit_skip(test, "not enabled");
+
+ /* warm-up */
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ for (l = 0; l < 10; l++) {
+ raid6_gen_syndrome(nr_to_test[i],
+ len_to_test[j], test_buffers);
+ }
+ }
+ }
+
+ /*
+ * Preferably this would be a loop over len_to_test, but the kunit
+ * logging always adds a newline to each logged format string.
+ */
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, " \t%5u bytes\t%5u bytes\n",
+ len_to_test[0], len_to_test[1]);
+
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ unsigned int nr = nr_to_test[i];
+ u64 speed[ARRAY_SIZE(len_to_test)];
+
+ KUNIT_ASSERT_LE(test, nr, RAID6_KUNIT_MAX_BUFFERS);
+
+ 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, RAID6_KUNIT_ALLOC_BYTES);
+
+ preempt_disable();
+ t = ktime_get_ns();
+ for (l = 0; l < num_iters; l++)
+ raid6_gen_syndrome(nr_to_test[i],
+ len_to_test[j], test_buffers);
+ t = max(ktime_get_ns() - t, 1);
+ preempt_enable();
+
+ speed[j] = div64_u64((u64)len * num_iters * nr, t);
+ }
+
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
+ nr, speed[0], speed[1]);
+ }
+}
+
static const void *raid6_gen_params(struct kunit *test, const void *prev,
char *desc)
{
@@ -256,6 +319,7 @@ static const void *raid6_gen_params(struct kunit *test, const void *prev,
static struct kunit_case raid6_test_cases[] = {
KUNIT_CASE_PARAM(raid6_test, raid6_gen_params),
+ KUNIT_CASE(raid6_benchmark),
{},
};
@@ -270,7 +334,7 @@ static int raid6_suite_init(struct kunit_suite *suite)
* so that it is immediately followed by a guard page. This allows
* buffer overreads to be detected, even in assembly code.
*/
- test_buflen = round_up(RAID6_KUNIT_MAX_BYTES, PAGE_SIZE);
+ test_buflen = round_up(RAID6_KUNIT_ALLOC_BYTES, PAGE_SIZE);
for (i = 0; i < RAID6_KUNIT_MAX_FAILURES; i++) {
test_recov_buffers[i] = vmalloc(test_buflen);
if (!test_recov_buffers[i])
--
2.53.0
^ permalink raw reply related
* [PATCH 7/8] raid6: improve the runtime selection benchmark
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260715144825.95432-1-hch@lst.de>
Use plain ktime_get_ns for the timing, use 8 + 2 disks for a realistic
load, and report the throughput on the data disks as that is what
storage systems are measured on.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/algos.c | 77 +++++++++++++++++++-----------------------
1 file changed, 35 insertions(+), 42 deletions(-)
diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index 702e167a543a..011aa9d0eb3c 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -152,40 +152,37 @@ void raid6_recov_datap(int disks, size_t bytes, int faila, void **ptrs)
}
EXPORT_SYMBOL_GPL(raid6_recov_datap);
-#define RAID6_TIME_JIFFIES_LG2 4
-#define RAID6_TEST_DISKS 8
+#define BENCH_SIZE SZ_4K
+#define NR_SRCS 8
+#define NR_DISKS (NR_SRCS + 2)
+#define REPS 800U
-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;
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();
+ /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
+ perf = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
if (perf > bestgenperf) {
bestgenperf = perf;
best = algo;
}
- pr_info("raid6: %-8s gen() %5ld MB/s\n", algo->name,
- (perf * HZ * (disks-2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2));
+ pr_info("raid6: %-8s gen() %5lu MB/s\n", algo->name, perf);
}
if (!best) {
@@ -197,28 +194,24 @@ static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
static_call_update(raid6_xor_syndrome_impl, best->xor_syndrome);
pr_info("raid6: using algorithm %s gen() %ld MB/s\n",
- best->name,
- (bestgenperf * HZ * (disks - 2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2));
+ best->name, bestgenperf);
if (best->xor_syndrome) {
- unsigned long perf = 0, j0, j1;
+ /* work on the second half of the disks */
+ int start = (disks / 2) - 1, stop = disks - 3;
+ u64 t;
preempt_disable();
- j0 = jiffies;
- while ((j1 = jiffies) == j0)
- cpu_relax();
- while (time_before(jiffies,
- j1 + (1 << RAID6_TIME_JIFFIES_LG2))) {
- best->xor_syndrome(disks, start, stop,
- PAGE_SIZE, *dptrs);
- perf++;
- }
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++)
+ best->xor_syndrome(disks, start, stop, BENCH_SIZE,
+ dptrs);
+ t = max(ktime_get_ns() - t, 1);
preempt_enable();
- pr_info("raid6: .... xor() %ld MB/s, rmw enabled\n",
- (perf * HZ * (disks - 2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2 + 1));
+ pr_info("raid6: .... xor() %llu MB/s, rmw enabled\n",
+ div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS / 2 * 1000,
+ t));
}
return 0;
@@ -230,9 +223,9 @@ static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
static int __init raid6_select_algo(void)
{
- const int disks = RAID6_TEST_DISKS;
+ const int disks = NR_DISKS;
+ void *dptrs[NR_DISKS];
char *disk_ptr, *p;
- void *dptrs[RAID6_TEST_DISKS];
int i, cycle;
int error;
@@ -243,7 +236,7 @@ static int __init raid6_select_algo(void)
}
/* prepare the buffer and fill it circularly with gfmul table */
- disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
+ disk_ptr = kmalloc_array(NR_DISKS, BENCH_SIZE, GFP_KERNEL);
if (!disk_ptr) {
pr_err("raid6: Yikes! No memory available.\n");
return -ENOMEM;
@@ -251,19 +244,19 @@ static int __init raid6_select_algo(void)
p = disk_ptr;
for (i = 0; i < disks; i++)
- dptrs[i] = p + PAGE_SIZE * i;
+ dptrs[i] = p + BENCH_SIZE * i;
- cycle = ((disks - 2) * PAGE_SIZE) / 65536;
+ cycle = ((disks - 2) * BENCH_SIZE) / 65536;
for (i = 0; i < cycle; i++) {
memcpy(p, raid6_gfmul, 65536);
p += 65536;
}
- if ((disks - 2) * PAGE_SIZE % 65536)
- memcpy(p, raid6_gfmul, (disks - 2) * PAGE_SIZE % 65536);
+ if ((disks - 2) * BENCH_SIZE % 65536)
+ memcpy(p, raid6_gfmul, (disks - 2) * BENCH_SIZE % 65536);
/* select raid gen_syndrome function */
- error = raid6_choose_gen(&dptrs, disks);
+ error = raid6_choose_gen(dptrs, disks);
kfree(disk_ptr);
--
2.53.0
^ permalink raw reply related
* [PATCH 6/8] raid6: defer implementation selection when built-in
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260715144825.95432-1-hch@lst.de>
Don't hold up early boot and defer the selection just like we've been
doing for the RAID5 XOR code since commit 524ccdbdfb52 ("crypto: xor -
defer load time benchmark to a later time").
This will also allow full use of the timing subsystem for benchmarking.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/algos.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index 6f5c89ab2b17..702e167a543a 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -239,10 +239,6 @@ static int __init raid6_select_algo(void)
if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
pr_info("raid6: skipped pq benchmark and selected %s\n",
raid6_algos[raid6_nr_algos - 1]->name);
- static_call_update(raid6_gen_syndrome_impl,
- raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
- static_call_update(raid6_xor_syndrome_impl,
- raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
return 0;
}
@@ -329,13 +325,34 @@ static int __init raid6_init(void)
static_call_update(raid6_recov_datap_impl, raid6_recov_algo->datap);
pr_info("raid6: using %s recovery algorithm\n", raid6_recov_algo->name);
+ /*
+ * Pick the last registered implementation as the temporary default until
+ * calibration happens.
+ */
+ static_call_update(raid6_gen_syndrome_impl,
+ raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
+ static_call_update(raid6_xor_syndrome_impl,
+ raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
+
+#ifdef MODULE
return raid6_select_algo();
+#else
+ return 0;
+#endif
}
static void __exit raid6_exit(void)
{
}
+/*
+ * When built-in we must register the default implementation before md
+ * initializes, but we don't want calibration to run that early as that
+ * would delay the boot process.
+ */
+#ifndef MODULE
+device_initcall(raid6_select_algo);
+#endif
subsys_initcall(raid6_init);
module_exit(raid6_exit);
MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related
* [PATCH 5/8] raid6: enable lock context analysis
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260715144825.95432-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/raid/raid6/Makefile b/lib/raid/raid6/Makefile
index 038d6c74d1ba..5cdb7223de2a 100644
--- a/lib/raid/raid6/Makefile
+++ b/lib/raid/raid6/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
ifeq ($(CONFIG_RAID6_PQ_ARCH),y)
--
2.53.0
^ permalink raw reply related
* [PATCH 4/8] xor/kunit: add a benchmark
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260715144825.95432-1-hch@lst.de>
Add a benchmark to test the XOR functions for more representative block
sizes and numbers of disks. Including 64k would be useful here, but
increasing the test buffer size increases the runtime of the functional
kunit test too much unfortunately.
The runtime numbers are reported in GB/s as the numbers of modern
implementations are basically unreadable as MB/s. This means
retro-architectures could report 0, but that is an easy tradeoff.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/Kconfig | 6 ++++
lib/raid/xor/tests/xor_kunit.c | 62 ++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index 978cd6ba08ac..c4eeb7c716c8 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -29,6 +29,12 @@ config XOR_KUNIT_TEST
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
+config XOR_BENCHMARK
+ bool "Benchmark for xor_gen"
+ depends on XOR_KUNIT_TEST
+ help
+ Include benchmarks in the KUnit test suite for xor_gen.
+
config RAID6_PQ
tristate
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 659ae3edbc25..1fb30b2c7c47 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -125,8 +125,70 @@ static void xor_test(struct kunit *test)
}
}
+static void xor_benchmark(struct kunit *test)
+{
+ static const unsigned int nr_to_test[] = {
+ 4, 5, 6, 7, 8, 10, 12, 15, 16, 32,
+ };
+ static const unsigned int len_to_test[] = {
+ SZ_4K, SZ_16K,
+ };
+ unsigned int i, j, l;
+ u64 t;
+
+ if (!IS_ENABLED(CONFIG_XOR_BENCHMARK))
+ kunit_skip(test, "not enabled");
+
+ /* warm-up */
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ for (l = 0; l < 10; l++) {
+ xor_gen(test_dest, test_buffers, nr_to_test[i],
+ len_to_test[j]);
+ }
+ }
+ }
+
+ /*
+ * Preferably this would be a loop over len_to_test, but the kunit
+ * logging always adds a newline to each logged format string.
+ */
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, " \t%5u bytes\t%5u bytes\n",
+ len_to_test[0], len_to_test[1]);
+
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ unsigned int nr = nr_to_test[i];
+ u64 speed[ARRAY_SIZE(len_to_test)];
+
+ KUNIT_ASSERT_LE(test, nr, XOR_KUNIT_MAX_BUFFERS);
+
+ 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();
+
+ speed[j] = div64_u64((u64)len * num_iters * nr, t);
+ }
+
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
+ nr, speed[0], speed[1]);
+ }
+}
+
static struct kunit_case xor_test_cases[] = {
KUNIT_CASE(xor_test),
+ KUNIT_CASE(xor_benchmark),
{},
};
--
2.53.0
^ permalink raw reply related
* [PATCH 3/8] xor/kunit: fix a spelling error
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260715144825.95432-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/tests/xor_kunit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 0c2a3a420bf9..659ae3edbc25 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -85,7 +85,7 @@ static void xor_test(struct kunit *test)
xor_generate_random_data();
/*
- * If we're not using the entire buffer size, inject randomize
+ * If we're not using the entire buffer size, inject randomized
* alignment into the buffer.
*/
max_alignment = XOR_KUNIT_MAX_BYTES - len;
--
2.53.0
^ permalink raw reply related
* [PATCH 2/8] xor: improve the runtime selection benchmark
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260715144825.95432-1-hch@lst.de>
Use plain ktime_get_ns for the timing, use 4 + 1 disks for a realistic
load, and report the throughput on the data disks instead of the that
on the parity disk, which isn't all that useful.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/xor-core.c | 55 +++++++++++++++++++----------------------
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
index 50931fbf0324..f9d61b096b11 100644
--- a/lib/raid/xor/xor-core.c
+++ b/lib/raid/xor/xor-core.c
@@ -10,7 +10,6 @@
#include <linux/gfp.h>
#include <linux/slab.h>
#include <linux/raid/xor.h>
-#include <linux/jiffies.h>
#include <linux/preempt.h>
#include <linux/static_call.h>
#include "xor_impl.h"
@@ -73,59 +72,56 @@ void __init xor_force(struct xor_block_template *tmpl)
forced_template = tmpl;
}
-#define BENCH_SIZE 4096
+#define BENCH_SIZE SZ_4K
+#define NR_SRCS 4
#define REPS 800U
-static void __init
-do_xor_speed(struct xor_block_template *tmpl, void *b1, void *b2)
+static void __init do_xor_speed(struct xor_block_template *tmpl, void *dest,
+ void *srcs[NR_SRCS])
{
- int speed;
- unsigned long reps;
- ktime_t min, start, t0;
- void *srcs[1] = { b2 };
+ u64 t;
+ int i;
preempt_disable();
-
- reps = 0;
- t0 = ktime_get();
- /* delay start until time has advanced */
- while ((start = ktime_get()) == t0)
- cpu_relax();
- do {
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++) {
mb(); /* prevent loop optimization */
- tmpl->xor_gen(b1, srcs, 1, BENCH_SIZE);
+ tmpl->xor_gen(dest, srcs, NR_SRCS, BENCH_SIZE);
mb();
- } while (reps++ < REPS || (t0 = ktime_get()) == start);
- min = ktime_sub(t0, start);
-
+ }
+ t = max(ktime_get_ns() - t, 1);
preempt_enable();
- // bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
- speed = (1000 * reps * BENCH_SIZE) / (unsigned int)ktime_to_ns(min);
- tmpl->speed = speed;
+ /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
+ tmpl->speed = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
- pr_info(" %-16s: %5d MB/sec\n", tmpl->name, speed);
+ pr_info(" %-16s: %5d MB/sec\n", tmpl->name, tmpl->speed);
}
static int __init calibrate_xor_blocks(void)
{
- void *b1, *b2;
struct xor_block_template *f, *fastest;
+ void *srcs[NR_SRCS];
+ void *buf, *dest;
+ int i;
if (forced_template)
return 0;
- b1 = kmalloc(PAGE_SIZE * 4, GFP_KERNEL);
- if (!b1) {
+ buf = kmalloc(BENCH_SIZE * (NR_SRCS + 1), GFP_KERNEL);
+ if (!buf) {
pr_warn("xor: Yikes! No memory available.\n");
return -ENOMEM;
}
- b2 = b1 + 2*PAGE_SIZE + BENCH_SIZE;
+ get_random_bytes(buf, BENCH_SIZE * (NR_SRCS + 1));
+ dest = buf;
+ for (i = 0; i < NR_SRCS; i++)
+ srcs[i] = buf + (i + 1) * BENCH_SIZE;
pr_info("xor: measuring software checksum speed\n");
fastest = template_list;
for (f = template_list; f; f = f->next) {
- do_xor_speed(f, b1, b2);
+ do_xor_speed(f, dest, srcs);
if (f->speed > fastest->speed)
fastest = f;
}
@@ -133,9 +129,10 @@ static int __init calibrate_xor_blocks(void)
pr_info("xor: using function: %s (%d MB/sec)\n",
fastest->name, fastest->speed);
- kfree(b1);
+ kfree(buf);
return 0;
}
+#undef NR_SRCS
#ifdef CONFIG_XOR_BLOCKS_ARCH
#include "xor_arch.h" /* $SRCARCH/xor_arch.h */
--
2.53.0
^ permalink raw reply related
* [PATCH 1/8] xor: enable lock context analysis
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260715144825.95432-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/Makefile | 2 ++
lib/raid/xor/tests/Makefile | 2 ++
2 files changed, 4 insertions(+)
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index e8ecec3c09f9..9b0fad459cdb 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
obj-$(CONFIG_XOR_BLOCKS) += xor.o
diff --git a/lib/raid/xor/tests/Makefile b/lib/raid/xor/tests/Makefile
index 661e8f6ffd1f..1cce833cd7fd 100644
--- a/lib/raid/xor/tests/Makefile
+++ b/lib/raid/xor/tests/Makefile
@@ -1,3 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_XOR_KUNIT_TEST) += xor_kunit.o
--
2.53.0
^ permalink raw reply related
* misc lib/raid/ improvements v2
From: Christoph Hellwig @ 2026-07-15 14:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
Hi all,
this series improves the benchmark-based algorithm selection and
adds kunit benchmarks for both the XOR and raid6 libraries, and tidies
up a few very minor other bits.
Note that Sashiko still complains about long preempt_disable regions.
While that is right in general, it is very much intentional for these
kunit boot / module load time benchmarks.
Changes since v1:
- fix the deferred implementation backing
- remove pointless variable shadowing
- fix up a commit message
- don't add a duplicate RAID6_PQ symbol
- fix the calculated RAID6 "XOR" bandwidth
- floor all timing delta to 1 to avoid entire hypothetical
divide by zero errors
diffstat:
Kconfig | 12 +++++
raid6/Makefile | 2
raid6/algos.c | 102 +++++++++++++++++++++++++---------------------
raid6/tests/raid6_kunit.c | 66 +++++++++++++++++++++++++++++
xor/Makefile | 2
xor/tests/Makefile | 2
xor/tests/xor_kunit.c | 64 ++++++++++++++++++++++++++++
xor/xor-core.c | 55 +++++++++++-------------
8 files changed, 228 insertions(+), 77 deletions(-)
^ permalink raw reply
* Re: Subject: RFC: Read repair for md RAID1 after mirror read failures
From: G.W. Kant - Hunenet B.V. @ 2026-07-15 14:13 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, linux-raid@vger.kernel.org, yukuai@fygo.io
In-Reply-To: <m2y0fcesbe.fsf@gmail.com>
Hi Abd,
On 7/15/26 7:27 AM, Abd-Alrhman Masalkhi wrote:
>> With today's 18–24 TB HDDs and backup/archive workloads, where data may
>> remain unchanged for years, latent media degradation seems increasingly
>> relevant. A successful read from the alternate mirror may be one of the
>> last opportunities to refresh such a sector before it becomes
>> permanently unreadable.
>>
> And Check/Repair is the right defense for cold archival data on large
> drives.
I have been looking into your suggestion that periodic check/repair is
the appropriate defense for cold archival data on large drives.
Before I start reading the resync code, I would like to make sure I
understand what repair actually does.
Suppose all mirrors read successfully and contain identical data. In
other words, there are no read errors and no mismatches.
Does repair simply verify the data and move on, or does it also rewrite
each successfully read block to the corresponding LBA on every mirror?
The distinction is important for what I have in mind.
My concern is not recovery from an existing read error—that is already
handled by fix_read_error()—but rather the long-term aging of cold
magnetic recordings.
If a sector is still readable today but is approaching the point where
the drive's ECC will eventually no longer be able to reconstruct it,
then a successful rewrite would effectively establish a new "write time"
for that sector. If the drive can no longer reliably store the data at
that location, the rewrite would also give the firmware an opportunity
to remap the sector.
So my question is essentially:
Does repair perform such a rewrite of successfully read blocks, or does
it only rewrite blocks when a read error or mismatch has already been
detected?
Regards,
Dion
^ permalink raw reply
* Re: Subject: RFC: Read repair for md RAID1 after mirror read failures
From: G.W. Kant - Hunenet B.V. @ 2026-07-15 11:24 UTC (permalink / raw)
To: yukuai@fygo.io, Abd-Alrhman Masalkhi, linux-raid@vger.kernel.org
In-Reply-To: <2718fd27-9453-4dea-9d1d-13cfecef4e6c@fnnas.com>
On 7/15/26 9:27 AM, Yu Kuai wrote:
>> Thank you. fix_read_error() was exactly the piece I was looking for. I
>> had missed that md already performs read repair when another healthy
>> mirror is available.
>>
>> My situation was indeed different because the array had already lost one
>> member, so the successful read originated from Btrfs redundancy across
>> another md RAID1 rather than from the surviving md mirror.
>>
>> One question remains, though. check/repair periodically reads all
>> sectors, but as far as I understand it, successfully read sectors are
>> not rewritten. From a media aging perspective this is a different
>> problem than recovering from a read error. Have there ever been
>> discussions about a true media refresh pass that rewrites successfully
>> read sectors to refresh long-lived magnetic recordings?
> Perhaps this is what you looking for:
>
> [RFC v2 0/5] md/raid1: introduce a new sync action to repair badblocks -
> Zheng Qixing
> <https://lore.kernel.org/all/20260203061259.609206-1-
> zhengqixing@huaweicloud.com/>
>
> However, AFAIK, Qixing no longer work in this area and there will not be
> new version.
>
>> Best regards,
>> Dion Kant
> -- Thanks, Kuai
Hi Kuai,
Thank you, this is certainly closely related.
As I understand the proposed 'rectify' action, it operates on ranges
that are already present in an md member's bad-block table. It reads the
data from a healthy mirror, writes it to the corresponding LBA on the
affected member, and clears the bad-block entry when the write succeeds.
That would be very useful, but I think it addresses a slightly different
problem from the media-refresh operation I had in mind.
The latter would proactively read and rewrite all allocated—or perhaps
all addressable—blocks, including blocks that are still read
successfully and have never been entered in the bad-block table. The
purpose would be to refresh cold magnetic recordings before they reach
the point at which the drive reports the first UNC.
So I see three separate mechanisms:
1. read repair after a failed normal read (`fix_read_error()`);
2. repair of already recorded bad blocks (`rectify`);
3. proactive read-and-rewrite of still-readable cold data.
The proposed patch appears to implement the second mechanism, whereas my
remaining question concerns the third.
Nevertheless, `rectify` is highly relevant, and it is unfortunate that
the series may no longer be developed. I will study the patch set and
its discussion.
Regards,
Dion Kant
^ permalink raw reply
* Re: [PATCH v4 3/7] md/raid10: consistently fail atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-07-15 11:16 UTC (permalink / raw)
To: John Garry, song, yukuai, magiclinan, xiao, axboe,
martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <f8554f5b-5a3f-4972-8bea-36fe983ded5e@oracle.com>
Hi John,
On Wed, Jul 15, 2026 at 11:49 +0100, John Garry wrote:
> On 15/07/2026 11:05, Abd-Alrhman Masalkhi wrote:
>> On Wed, Jul 15, 2026 at 10:25 +0100, John Garry wrote:
>>> On 10/07/2026 11:15, Abd-Alrhman Masalkhi wrote:
>>>> RAID10 currently handles one badblock path explicitly by failing atomic
>>>> writes with EIO. However, another badblock path can also reduce the
>>>> writable range and force the bio through bio_submit_split_bioset(),
>>>
>>> What is this other badblock path which you are referring to?
>>>
>> The other path is when the requested write overlaps a bad-block region
>> from the beginning, but the intended write extends beyond the bad-block
>> region. In that case, the current code silently reduces max_sectors.
>> As a result, we end up calling bio_submit_split_bioset(), which
>> completes the bio with -EINVAL rather than -EIO. This is the same issue
>> that existed in raid1 and was fixed by patch 2.
>
> OK, it would be nice to add a concise version of that info to the commit
> log.
>
Thanks. I'll update the commit log accordingly and send v5.
> Reviewed-by: John Garry <john.g.garry@oracle.com>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v4 3/7] md/raid10: consistently fail atomic writes that require splitting
From: John Garry @ 2026-07-15 10:49 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
vverma, martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <m2tsq0eff4.fsf@gmail.com>
On 15/07/2026 11:05, Abd-Alrhman Masalkhi wrote:
> On Wed, Jul 15, 2026 at 10:25 +0100, John Garry wrote:
>> On 10/07/2026 11:15, Abd-Alrhman Masalkhi wrote:
>>> RAID10 currently handles one badblock path explicitly by failing atomic
>>> writes with EIO. However, another badblock path can also reduce the
>>> writable range and force the bio through bio_submit_split_bioset(),
>>
>> What is this other badblock path which you are referring to?
>>
> The other path is when the requested write overlaps a bad-block region
> from the beginning, but the intended write extends beyond the bad-block
> region. In that case, the current code silently reduces max_sectors.
> As a result, we end up calling bio_submit_split_bioset(), which
> completes the bio with -EINVAL rather than -EIO. This is the same issue
> that existed in raid1 and was fixed by patch 2.
OK, it would be nice to add a concise version of that info to the commit
log.
Reviewed-by: John Garry <john.g.garry@oracle.com>
^ permalink raw reply
* Re: [PATCH v4 3/7] md/raid10: consistently fail atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-07-15 10:05 UTC (permalink / raw)
To: John Garry, song, yukuai, magiclinan, xiao, axboe, vverma,
martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <33aca226-f744-45e0-a137-1f85ab146e02@oracle.com>
On Wed, Jul 15, 2026 at 10:25 +0100, John Garry wrote:
> On 10/07/2026 11:15, Abd-Alrhman Masalkhi wrote:
>> RAID10 currently handles one badblock path explicitly by failing atomic
>> writes with EIO. However, another badblock path can also reduce the
>> writable range and force the bio through bio_submit_split_bioset(),
>
> What is this other badblock path which you are referring to?
>
The other path is when the requested write overlaps a bad-block region
from the beginning, but the intended write extends beyond the bad-block
region. In that case, the current code silently reduces max_sectors.
As a result, we end up calling bio_submit_split_bioset(), which
completes the bio with -EINVAL rather than -EIO. This is the same issue
that existed in raid1 and was fixed by patch 2.
>> which implicitly completes the bio with EINVAL.
>>
>> Fix this by handling atomic writes in the common split check. If RAID10
>> determines that an atomic write would require splitting, complete the
>> bio with EIO.
>>
>> Fixes: a1d9b4fd42d9 ("md/raid10: Atomic write support")
>> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
>> ---
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v4 3/7] md/raid10: consistently fail atomic writes that require splitting
From: John Garry @ 2026-07-15 9:25 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
vverma, martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <20260710101521.1714-4-abd.masalkhi@gmail.com>
On 10/07/2026 11:15, Abd-Alrhman Masalkhi wrote:
> RAID10 currently handles one badblock path explicitly by failing atomic
> writes with EIO. However, another badblock path can also reduce the
> writable range and force the bio through bio_submit_split_bioset(),
What is this other badblock path which you are referring to?
> which implicitly completes the bio with EINVAL.
>
> Fix this by handling atomic writes in the common split check. If RAID10
> determines that an atomic write would require splitting, complete the
> bio with EIO.
>
> Fixes: a1d9b4fd42d9 ("md/raid10: Atomic write support")
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
> ---
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox