* Subject: RFC: Read repair for md RAID1 after mirror read failures
From: G.W. Kant - Hunenet B.V. @ 2026-07-15 2:52 UTC (permalink / raw)
To: linux-raid@vger.kernel.org
Hello list,
Recently I encountered an interesting failure mode while migrating data
from an aging backup system.
The source Btrfs filesystem spanned several LVM logical volumes, each
backed by an md RAID1 array. One of these logical volumes resided on a
degraded RAID1 array, so I added a new logical volume on a new RAID1
array and started migrating the data using:
btrfs device remove
During the migration, many read requests encountered unrecoverable read
errors (UNC) on the remaining member of the degraded RAID1 array. The
migration continued, which is exactly what I had hoped for, since only a
small fraction of the media appeared to be affected.
This made me wonder whether md RAID1 has, or has ever considered, a read
repair mechanism.
Consider the following situation:
mirror A:
read -> UNC
mirror B:
read -> OK
During a RAID1 read request, if md can satisfy the read from the
alternate mirror, no data is lost. However, this also represents an
opportunity to repair the degraded copy. Once a read has failed on one
mirror, the array has effectively lost its ability to tolerate a second
read failure for that logical block. This window persists until the
affected block is rewritten by normal filesystem activity, which may
never happen on cold archival data. Repairing the degraded copy
immediately may restore full redundancy while a valid copy is still
available. Writing the recovered block back to the the corresponding
block on the failed mirror would give the drive an opportunity to
recover that location, for example by successfully rewriting the sector
or remapping it if the write cannot be verified.
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.
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.
One advantage of such an approach is that it does not require md to
decide between two conflicting copies. One mirror has already reported
an unrecoverable read error, while the other has successfully
reconstructed the requested block. The proposal only applies to this
specific case.
Has this idea been discussed before, or is there a reason why md
deliberately avoids this type of read repair?
Regards,
Dion Kant
^ permalink raw reply
* Re: [PATCH] md/raid5: fix lockless max_nr_stripes reads
From: yu kuai @ 2026-07-15 4:09 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yu kuai; +Cc: linux-kernel
In-Reply-To: <20260624024042.2561803-1-chencheng@fnnas.com>
在 2026/6/24 10:40, Chen Cheng 写道:
> From: Chen Cheng<chencheng@fnnas.com>
>
> max_nr_stripes is updated under cache_size_mutex in the stripe cache
> grow/shrink paths, while is_inactive_blocked() and
> raid5_end_read_request() read it without that lock.
>
> Use READ_ONCE() for those reads in lockless path to match the WRITE_ONCE()
> updates and avoid KCSAN data race reports.
>
> A similar issue was previously fixed in commit-id:
> dfd2bf436709b2bccb78c2dda550dde93700efa7.
>
> Fixes: 0009fad033370 ("raid5 improve too many read errors msg by adding limits")
> Fixes: 3514da58be9c4 ("md/raid5: Make is_inactive_blocked() helper")
>
> KCSAN report:
> =================
>
> BUG: KCSAN: data-race in grow_one_stripe / is_inactive_blocked
>
> write (marked) to 0xffff8f01f0b5a268 of 4 bytes by task 12616 on cpu 9:
> grow_one_stripe+0x2d8/0x320
> raid5d+0xb57/0xba0
> md_thread+0x15a/0x2d0
> [..........]
>
> read to 0xffff8f01f0b5a268 of 4 bytes by task 12670 on cpu 11:
> is_inactive_blocked+0x97/0xc0
> raid5_get_active_stripe+0x2fd/0xa70
> raid5_make_request+0x4aa/0x2940
> [..........]
>
> value changed: 0x000003b9 -> 0x000003ba
>
> Signed-off-by: Chen Cheng<chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 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 5:27 UTC (permalink / raw)
To: G.W. Kant - Hunenet B.V., linux-raid@vger.kernel.org
In-Reply-To: <8dcbfa43-c687-49bb-81b5-e6b8e8848c77@hunenet.nl>
Hi Dion,
On Wed, Jul 15, 2026 at 02:52 +0000, G. W. Kant wrote:
> Hello list,
>
> Recently I encountered an interesting failure mode while migrating data
> from an aging backup system.
>
> The source Btrfs filesystem spanned several LVM logical volumes, each
> backed by an md RAID1 array. One of these logical volumes resided on a
> degraded RAID1 array, so I added a new logical volume on a new RAID1
> array and started migrating the data using:
>
> btrfs device remove
>
> During the migration, many read requests encountered unrecoverable read
> errors (UNC) on the remaining member of the degraded RAID1 array. The
> migration continued, which is exactly what I had hoped for, since only a
> small fraction of the media appeared to be affected.
>
> This made me wonder whether md RAID1 has, or has ever considered, a read
> repair mechanism.
>
> Consider the following situation:
>
> mirror A:
> read -> UNC
>
> mirror B:
> read -> OK
>
> During a RAID1 read request, if md can satisfy the read from the
> alternate mirror, no data is lost. However, this also represents an
> opportunity to repair the degraded copy. Once a read has failed on one
> mirror, the array has effectively lost its ability to tolerate a second
> read failure for that logical block. This window persists until the
> affected block is rewritten by normal filesystem activity, which may
> never happen on cold archival data. Repairing the degraded copy
> immediately may restore full redundancy while a valid copy is still
> available. Writing the recovered block back to the the corresponding
> block on the failed mirror would give the drive an opportunity to
> recover that location, for example by successfully rewriting the sector
> or remapping it if the write cannot be verified.
>
> 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.
> One advantage of such an approach is that it does not require md to
> decide between two conflicting copies. One mirror has already reported
> an unrecoverable read error, while the other has successfully
> reconstructed the requested block. The proposal only applies to this
> specific case.
>
> Has this idea been discussed before, or is there a reason why md
> deliberately avoids this type of read repair?
>
> Regards,
>
> Dion Kant
--
Best Regards,
Abd-Alrhman
^ 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 7:10 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, linux-raid@vger.kernel.org
In-Reply-To: <m2y0fcesbe.fsf@gmail.com>
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?
Best regards,
Dion Kant
^ permalink raw reply
* Re: Subject: RFC: Read repair for md RAID1 after mirror read failures
From: Yu Kuai @ 2026-07-15 7:27 UTC (permalink / raw)
To: G.W. Kant - Hunenet B.V., Abd-Alrhman Masalkhi,
linux-raid@vger.kernel.org, yu kuai
In-Reply-To: <368d1223-1d41-4227-af33-bd6bd00b5c3d@hunenet.nl>
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.
>
> Best regards,
> Dion Kant
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v4 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
From: yu kuai @ 2026-07-15 7:35 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, magiclinan, xiao, axboe, vverma,
john.g.garry, martin.petersen, linux-kernel, yu kuai
Cc: linux-raid, sashiko-bot
In-Reply-To: <20260710101521.1714-2-abd.masalkhi@gmail.com>
在 2026/7/10 18:15, Abd-Alrhman Masalkhi 写道:
> When raid10_write_request() fails because REQ_NOWAIT is set, the
> allocated r10_bio is not freed before returning, resulting in a memory
> leak. Free r10_bio before returning from the REQ_NOWAIT error paths.
>
> Fixes: c9aa889b035f ("md: raid10 add nowait support")
> Reported-by: sashiko-bot<sashiko-bot@kernel.org>
> Closes:https://lore.kernel.org/linux-raid/20260613184042.BCEC01F000E9@smtp.kernel.org/
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v4 2/7] md/raid1: restrict atomic write limits and handle runtime constraints
From: yu kuai @ 2026-07-15 9:09 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-3-abd.masalkhi@gmail.com>
在 2026/7/10 18:15, Abd-Alrhman Masalkhi 写道:
> Restrict the RAID1 atomic write limits by setting chunk_sectors to
> BARRIER_UNIT_SECTOR_SIZE so that atomic writes never straddle a barrier
> unit.
>
> A bio that passes block-layer validation may still become unserviceable
> within RAID1 due to bad blocks or write-behind constraints. In the former
> case, complete the bio with EIO. In the latter case, disable
> write-behind rather than failing the bio with EIO.
>
> Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
> Fixes: a4c55c902670 ("md/raid1: simplify raid1_write_request() error handling")
> Reviewed-by: John Garry<john.g.garry@oracle.com>
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
> ---
> Changes in v4:
> - Improve the commit message.
> - Add Reviewed-by tag from John Garry.
> - Link to v3:https://lore.kernel.org/linux-raid/20260708101341.473750-3-abd.masalkhi@gmail.com/
>
> Changes in v3:
> - Set chunk_sectors to BARRIER_UNIT_SECTOR_SIZE instead of setting
> atomic_write_hw_unit_max.
> - Avoid enabling write-behind when the atomic write exceeds the
> write-behind limit.
> - Link to v2:https://lore.kernel.org/linux-raid/20260628142420.1051027-3-abd.masalkhi@gmail.com/
>
> Changes in v2:
> - Drop the early atomic write split check from raid1_write_request().
> - Advertise the atomic write size limit via queue limits.
> - Disable write-behind instead of failing atomic writes when the
> BIO_MAX_VECS limit is encountered.
> - Link to v1:https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/
> ---
> drivers/md/raid1.c | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v4 3/7] md/raid10: consistently fail atomic writes that require splitting
From: yu kuai @ 2026-07-15 9:10 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-4-abd.masalkhi@gmail.com>
在 2026/7/10 18:15, Abd-Alrhman Masalkhi 写道:
> 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(),
> 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>
> ---
> Changes in v4:
> - No changes.
> - Link to v3:https://lore.kernel.org/linux-raid/20260708101341.473750-4-abd.masalkhi@gmail.com/
>
> Changes in v3:
> - No changes.
> - Link to v2:https://lore.kernel.org/linux-raid/20260628142420.1051027-4-abd.masalkhi@gmail.com/
>
> Changes in v2:
> - Drop the early atomic write split check from raid10_write_request()
> and rely on queue limits instead.
> - Link to v1:https://lore.kernel.org/linux-raid/20260623072456.333437-4-abd.masalkhi@gmail.com/
> ---
> drivers/md/raid10.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ 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
* 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 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 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: 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: 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
* 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
* [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
* [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 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 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 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 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 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 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
* 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
* 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
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