* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: John Garry @ 2026-07-03 8:28 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
vverma, martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <m2se64zak9.fsf@gmail.com>
On 30/06/2026 09:39, Abd-Alrhman Masalkhi wrote:
>
> Hi John,
>
> On Mon, Jun 29, 2026 at 15:48 +0100, John Garry wrote:
>> On 28/06/2026 15:24, Abd-Alrhman Masalkhi wrote:
>>> Atomic writes in RAID1 must fit within a single barrier unit. Advertise
>>> this restriction through the queue limits by setting
>>> atomic_write_hw_unit_max to BARRIER_UNIT_SECTOR_SIZE so that bios which
>>> would cross a barrier-unit boundary are rejected by the block layer
>>> before reaching MD.
>>>
>>> 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")
>>> Signed-off-by: Abd-Alrhman Masalkhi <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://urldefense.com/v3/__https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/__;!!ACWV5N9M2RV99hQ!LbMSGSClRi0PNBqQti5ZNWGDVjDd34-7saYEAwNyBNjpNTjEA7veqM5RHG8KB1QiscarW4UaIefjm19ywSImtIgh$
>>> ---
>>> drivers/md/raid1.c | 36 +++++++++++++++++++-----------------
>>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>>> index afe2ca96ad8c..f322048ab3c2 100644
>>> --- a/drivers/md/raid1.c
>>> +++ b/drivers/md/raid1.c
>>> @@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>>> int first_clone;
>>> bool write_behind = false;
>>> bool nowait = bio->bi_opf & REQ_NOWAIT;
>>> + bool atomic = bio->bi_opf & REQ_ATOMIC;
>>> bool is_discard = op_is_discard(bio->bi_opf);
>>> sector_t sector = bio->bi_iter.bi_sector;
>>>
>>> @@ -1603,20 +1604,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>>> }
>>> if (is_bad) {
>>> int good_sectors;
>>> -
>>> - /*
>>> - * We cannot atomically write this, so just
>>> - * error in that case. It could be possible to
>>> - * atomically write other mirrors, but the
>>> - * complexity of supporting that is not worth
>>> - * the benefit.
>>> - */
>>> - if (bio->bi_opf & REQ_ATOMIC) {
>>> - bio->bi_status = BLK_STS_NOTSUPP;
>>> - bio_endio(bio);
>>> - goto err_dec_pending;
>>> - }
>>> -
>>> good_sectors = first_bad - sector;
>>> if (good_sectors < max_sectors)
>>> max_sectors = good_sectors;
>>> @@ -1633,10 +1620,24 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>>> * at a time and thus needs a new bio that can fit the whole payload
>>> * this bio in page sized chunks.
>>> */
>>> - if (write_behind && mddev->bitmap)
>>> - max_sectors = min_t(int, max_sectors,
>>> - BIO_MAX_VECS * (PAGE_SIZE >> 9));
>>> + if (write_behind && mddev->bitmap) {
>>> + if (atomic && max_sectors > BIO_MAX_VECS * (PAGE_SIZE >> 9))
>>
>> where does BIO_MAX_VECS * (PAGE_SIZE >> 9) even come from?
>>
>
> BIO_MAX_VECS * (PAGE_SIZE >> 9) defines the maximum size supported by
> write-behind. The write-behind copy (alloc_behind_master_bio) uses a
> single bio, which can hold at most BIO_MAX_VECS pages, making this the
> largest payload it can carry. With a 4 KiB PAGE_SIZE, that corresponds
> to 256 pages, or 1 MiB (2048 sectors).
> This patch changes the behavior for atomic writes that exceed this
> limit. Instead of failing the write with -EIO when the number of sectors
> must be reduced, it disables write-behind and proceeds with the atomic
> write.
>
>>> + /*
>>> + * Atomic writes cannot be split, so disable
>>> + * write-behind.
>>> + */
>>> + write_behind = false;
It's a bit poor to have write_behind initially = false, then allow it to
be set = true, and then later be set = false. Can this be improved?
>>> + else
>>> + max_sectors = min_t(int, max_sectors,
>>> + BIO_MAX_VECS * (PAGE_SIZE >> 9));
>>> + }
>>> +
>>> if (max_sectors < bio_sectors(bio)) {
>>> + if (atomic) {
>>> + bio_io_error(bio);
>>> + goto err_dec_pending;
>>> + }
>>> +
>>> bio = bio_submit_split_bioset(bio, max_sectors,
>>> &conf->bio_split);
>>> if (!bio)
>>> @@ -3229,6 +3230,7 @@ static int raid1_set_limits(struct mddev *mddev)
>>> lim.max_write_zeroes_sectors = 0;
>>> lim.max_hw_wzeroes_unmap_sectors = 0;
>>> lim.logical_block_size = mddev->logical_block_size;
>>> + lim.atomic_write_hw_unit_max = BARRIER_UNIT_SECTOR_SIZE;
>>
>> This BARRIER_UNIT_SECTOR_SIZE is a bit like chunk sectors, no? I am just
>> wondering if we just should set it to chunk sectors =
>> BARRIER_UNIT_SECTOR_SIZE
>>
>> I assume that it affects more than Reads and writes, e.g. discard also.
>>
>
> BARRIER_UNIT_SECTOR_SIZE is the resync barrier-bucket, not the layout
> chunk size. Unless I'm missing something, using
> atomic_write_hw_unit_max seems more appropriate than using the chunk
> size. That way, the limit only applies to atomic writes instead of
> affecting other operations such.
well it seems to be that everything in the driver is split over
BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
Note that atomic_write_hw_unit_max is going to be small always compared
to BARRIER_UNIT_SECTOR_SIZE.
However, can you check the blk stacking code to make sure that this does
as you want? As I remember, for stacking we take the atomic write limits
of the first bottom device and then stack the other bottom devices and I
don't think that setting atomic_write_hw_unit_max in this way has an
impact - see blk_stack_atomic_writes_limits()
>
>>> lim.features |= BLK_FEAT_ATOMIC_WRITES;
>>> lim.features |= BLK_FEAT_PCI_P2PDMA;
>>> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
>>
>
^ permalink raw reply
* Re: [PATCH 1/8] x86/fpu: Check for missing AVX and AVX-512 xstate bits
From: Borislav Petkov @ 2026-07-01 0:49 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Eric Biggers, linux-raid, yukuai, LKML
In-Reply-To: <20260626050038.2AD661F000E9@smtp.kernel.org>
On Fri, Jun 26, 2026 at 05:00:37AM +0000, sashiko-bot@kernel.org wrote:
> [Severity: Critical]
> This is a pre-existing issue, but Memory Protection Keys (PKU) isolation
> can be bypassed because we don't verify X86_FEATURE_PKU against the
> presence of XFEATURE_PKRU in xstate here.
>
> If a hypervisor reports X86_FEATURE_PKU in CPUID but masks out the
> XFEATURE_PKRU xstate component, the kernel's setup_pku() enables CR4.PKE
> and forces X86_FEATURE_OSPKE. Because X86_FEATURE_OSPKE is artificially
> forced, the feature is not cleared.
>
> CR4.PKE=1 permits userspace to use WRPKRU, but because XCR0.PKRU=0,
> xsaves/xrstors silently ignore the PKRU register during task context
> switches, leading to state leakage across tasks.
>
> Should we also validate PKU against missing xstate components in this
> function to prevent isolation bypass?
I *think* Sashiko means here our preexisting fpu__init_system_xstate() here
but I'm not sure...
> [Severity: High]
> Does this logic get bypassed if XSAVE or FPU support is missing?
>
> Earlier in fpu__init_system_xstate(), there are early returns if
> X86_FEATURE_FPU or X86_FEATURE_XSAVE are missing:
>
> if (!boot_cpu_has(X86_FEATURE_XSAVE)) {
> pr_info("x86/fpu: x87 FPU will use %s\n",
> boot_cpu_has(X86_FEATURE_FXSR) ? "FXSAVE" : "FSAVE");
> return;
> }
>
> If a hypervisor natively enumerates AVX=1 but XSAVE=0 (such as QEMU
> with -cpu host,-xsave), check_cpufeature_deps() only warns about unmet
Yah, we don't care. We won't support non-sensical guest configs.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: sashiko-bot @ 2026-06-30 10:18 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: linux-raid, yukuai
In-Reply-To: <20260630075640.1081634-1-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch prevents the recovery thread from starting or resuming for existing rebuilding spares, silently abandoning recovery and leaving the array permanently degraded.
Pre-existing issues:
- [High] The patch addresses a TOCTOU race for `state_store` but misses other vulnerable paths like `slot_store` and cluster updates, which still call `remove_and_add_spares()` without array suspension.
--
commit 16cf64f20d25bc671dc52422ec0a44672674e524
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md: avoid modifying spares while the array is not suspended
This commit ensures that remove_spares() and remove_and_add_spares() are
only called after the array has been suspended. This prevents a race
condition where normal I/O could trigger a NULL pointer dereference when
rdevs are removed or replaced concurrently.
Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..f555eb5aca5d8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -10151,7 +10154,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
> * also removed and re-added, to allow the personality to fail the
> * re-add.
> */
> - *spares = remove_and_add_spares(mddev, NULL);
> + if (array_suspended)
> + *spares = remove_and_add_spares(mddev, NULL);
> + else
> + *spares = 0;
> +
[Severity: High]
Does this else branch prevent the recovery thread from starting or resuming
for existing rebuilding spares?
In md_start_sync(), suspend is determined by checking
md_spares_need_change(mddev). For an existing recovering spare receiving
normal I/O (where nr_pending > 0), both rdev_removeable() and
rdev_addable() can evaluate to false. This causes md_spares_need_change()
to return false, leaving suspend as false.
When md_choose_sync_action() is called with array_suspended == false, this
new else branch forces *spares = 0. By completely bypassing the counting of
existing recovering spares, md_choose_sync_action() will return false and
skip starting the sync thread.
Since nr_pending dropping to zero on a non-faulty spare doesn't trigger a
new MD_RECOVERY_NEEDED event, could this silently abandon recovery and
leave the array permanently degraded?
[Severity: High]
This is a pre-existing issue, but does restricting remove_and_add_spares()
calls to suspended arrays in md_choose_sync_action() leave other
vulnerable paths exposed?
The commit addresses a TOCTOU race by ensuring remove_and_add_spares() is
only called when the array is suspended, preventing concurrent I/O from
encountering a cleared replacement pointer.
However, slot_store() can still call remove_and_add_spares() without an
array suspension when a user writes "none" (slot == -1):
drivers/md/md.c:slot_store() {
...
clear_bit(Blocked, &rdev->flags);
remove_and_add_spares(rdev->mddev, rdev);
if (rdev->raid_disk >= 0)
return -EBUSY;
...
}
Similarly, the MD cluster component updates the array configuration in
check_sb_changes() by calling remove_and_add_spares() while the array is
not suspended.
Could normal I/O still increment nr_pending on a replacement drive just
before remove_spares() removes the faulty drive and clears the replacement
pointer in these paths, reproducing the exact TOCTOU crash this patch
aims to fix?
> @@ -10196,11 +10203,12 @@ static void md_start_sync(struct work_struct *ws)
> * As we only add devices that are already in-sync, we can
> * activate the spares immediately.
> */
> - remove_and_add_spares(mddev, NULL);
> + if (suspend)
> + remove_and_add_spares(mddev, NULL);
> goto not_running;
> }
>
> - if (!md_choose_sync_action(mddev, &spares))
> + if (!md_choose_sync_action(mddev, &spares, suspend))
> goto not_running;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260630075640.1081634-1-abd.masalkhi@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-06-30 8:39 UTC (permalink / raw)
To: John Garry, song, yukuai, magiclinan, xiao, axboe, vverma,
martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <7cbaaca3-4eb5-434d-a13f-f9574c9f977b@oracle.com>
Hi John,
On Mon, Jun 29, 2026 at 15:48 +0100, John Garry wrote:
> On 28/06/2026 15:24, Abd-Alrhman Masalkhi wrote:
>> Atomic writes in RAID1 must fit within a single barrier unit. Advertise
>> this restriction through the queue limits by setting
>> atomic_write_hw_unit_max to BARRIER_UNIT_SECTOR_SIZE so that bios which
>> would cross a barrier-unit boundary are rejected by the block layer
>> before reaching MD.
>>
>> 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")
>> Signed-off-by: Abd-Alrhman Masalkhi <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://urldefense.com/v3/__https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/__;!!ACWV5N9M2RV99hQ!LbMSGSClRi0PNBqQti5ZNWGDVjDd34-7saYEAwNyBNjpNTjEA7veqM5RHG8KB1QiscarW4UaIefjm19ywSImtIgh$
>> ---
>> drivers/md/raid1.c | 36 +++++++++++++++++++-----------------
>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index afe2ca96ad8c..f322048ab3c2 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>> int first_clone;
>> bool write_behind = false;
>> bool nowait = bio->bi_opf & REQ_NOWAIT;
>> + bool atomic = bio->bi_opf & REQ_ATOMIC;
>> bool is_discard = op_is_discard(bio->bi_opf);
>> sector_t sector = bio->bi_iter.bi_sector;
>>
>> @@ -1603,20 +1604,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>> }
>> if (is_bad) {
>> int good_sectors;
>> -
>> - /*
>> - * We cannot atomically write this, so just
>> - * error in that case. It could be possible to
>> - * atomically write other mirrors, but the
>> - * complexity of supporting that is not worth
>> - * the benefit.
>> - */
>> - if (bio->bi_opf & REQ_ATOMIC) {
>> - bio->bi_status = BLK_STS_NOTSUPP;
>> - bio_endio(bio);
>> - goto err_dec_pending;
>> - }
>> -
>> good_sectors = first_bad - sector;
>> if (good_sectors < max_sectors)
>> max_sectors = good_sectors;
>> @@ -1633,10 +1620,24 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>> * at a time and thus needs a new bio that can fit the whole payload
>> * this bio in page sized chunks.
>> */
>> - if (write_behind && mddev->bitmap)
>> - max_sectors = min_t(int, max_sectors,
>> - BIO_MAX_VECS * (PAGE_SIZE >> 9));
>> + if (write_behind && mddev->bitmap) {
>> + if (atomic && max_sectors > BIO_MAX_VECS * (PAGE_SIZE >> 9))
>
> where does BIO_MAX_VECS * (PAGE_SIZE >> 9) even come from?
>
BIO_MAX_VECS * (PAGE_SIZE >> 9) defines the maximum size supported by
write-behind. The write-behind copy (alloc_behind_master_bio) uses a
single bio, which can hold at most BIO_MAX_VECS pages, making this the
largest payload it can carry. With a 4 KiB PAGE_SIZE, that corresponds
to 256 pages, or 1 MiB (2048 sectors).
This patch changes the behavior for atomic writes that exceed this
limit. Instead of failing the write with -EIO when the number of sectors
must be reduced, it disables write-behind and proceeds with the atomic
write.
>> + /*
>> + * Atomic writes cannot be split, so disable
>> + * write-behind.
>> + */
>> + write_behind = false;
>> + else
>> + max_sectors = min_t(int, max_sectors,
>> + BIO_MAX_VECS * (PAGE_SIZE >> 9));
>> + }
>> +
>> if (max_sectors < bio_sectors(bio)) {
>> + if (atomic) {
>> + bio_io_error(bio);
>> + goto err_dec_pending;
>> + }
>> +
>> bio = bio_submit_split_bioset(bio, max_sectors,
>> &conf->bio_split);
>> if (!bio)
>> @@ -3229,6 +3230,7 @@ static int raid1_set_limits(struct mddev *mddev)
>> lim.max_write_zeroes_sectors = 0;
>> lim.max_hw_wzeroes_unmap_sectors = 0;
>> lim.logical_block_size = mddev->logical_block_size;
>> + lim.atomic_write_hw_unit_max = BARRIER_UNIT_SECTOR_SIZE;
>
> This BARRIER_UNIT_SECTOR_SIZE is a bit like chunk sectors, no? I am just
> wondering if we just should set it to chunk sectors =
> BARRIER_UNIT_SECTOR_SIZE
>
> I assume that it affects more than Reads and writes, e.g. discard also.
>
BARRIER_UNIT_SECTOR_SIZE is the resync barrier-bucket, not the layout
chunk size. Unless I'm missing something, using
atomic_write_hw_unit_max seems more appropriate than using the chunk
size. That way, the limit only applies to atomic writes instead of
affecting other operations such.
>> lim.features |= BLK_FEAT_ATOMIC_WRITES;
>> lim.features |= BLK_FEAT_PCI_P2PDMA;
>> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* [PATCH] md: avoid modifying spares while the array is not suspended
From: Abd-Alrhman Masalkhi @ 2026-06-30 7:56 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi, sashiko-bot
remove_spares() and remove_and_add_spares() modify the array's rdev
configuration. These operations are only safe after the array has been
suspended.
Today, md_start_sync() can call md_choose_sync_action() even when the
array has not been suspended. As a result, md_choose_sync_action() may
remove or replace rdevs while normal I/O is still accessing them.
The race can occur as follows:
raid10d Worker Normal IO
____________ _______________________ ______________________
raid10_write_request()
wait_blocked_dev()
set Blocked
set Faulty
Skip Faulty rdev
rrdev->nr_pending++
.repl_bio = bio
removeable_rdev = false .
array not suspended .
lock mddev goto err_handle
lock mddev (wait)
.
update sb .
clear Blocked .
.
unlock mddev .
lock mddev (acquires)
remove_spares()
removeable_rdev = true
raid10_remove_disk()
rdev = replacement
replacement = NULL
rdev_dec_pending(NULL)
unlock mddev (NULL)->nr_pending--
In this case, rdev_dec_pending() is called with a NULL pointer,
resulting in a NULL pointer dereference when attempting to decrement
nr_pending.
Fix this by ensuring that remove_spares() and remove_and_add_spares()
are only called after the array has been suspended, preventing
concurrent rdev configuration changes while normal I/O is in progress.
Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/md/md.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 66a41d482e59..c85ebb59535b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev,
return spares;
}
-static bool md_choose_sync_action(struct mddev *mddev, int *spares)
+static bool md_choose_sync_action(struct mddev *mddev, int *spares,
+ bool array_suspended)
{
/* Check if reshape is in progress first. */
if (mddev->reshape_position != MaxSector) {
@@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
/* Check if resync is in progress. */
if (mddev->resync_offset < MaxSector) {
- remove_spares(mddev, NULL);
+ if (array_suspended)
+ remove_spares(mddev, NULL);
+
set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery);
@@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
* also removed and re-added, to allow the personality to fail the
* re-add.
*/
- *spares = remove_and_add_spares(mddev, NULL);
+ if (array_suspended)
+ *spares = remove_and_add_spares(mddev, NULL);
+ else
+ *spares = 0;
+
if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) {
clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
@@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws)
* As we only add devices that are already in-sync, we can
* activate the spares immediately.
*/
- remove_and_add_spares(mddev, NULL);
+ if (suspend)
+ remove_and_add_spares(mddev, NULL);
goto not_running;
}
- if (!md_choose_sync_action(mddev, &spares))
+ if (!md_choose_sync_action(mddev, &spares, suspend))
goto not_running;
if (!mddev->pers->sync_request)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: John Garry @ 2026-06-29 14:48 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
vverma, martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-3-abd.masalkhi@gmail.com>
On 28/06/2026 15:24, Abd-Alrhman Masalkhi wrote:
> Atomic writes in RAID1 must fit within a single barrier unit. Advertise
> this restriction through the queue limits by setting
> atomic_write_hw_unit_max to BARRIER_UNIT_SECTOR_SIZE so that bios which
> would cross a barrier-unit boundary are rejected by the block layer
> before reaching MD.
>
> 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")
> Signed-off-by: Abd-Alrhman Masalkhi <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://urldefense.com/v3/__https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/__;!!ACWV5N9M2RV99hQ!LbMSGSClRi0PNBqQti5ZNWGDVjDd34-7saYEAwNyBNjpNTjEA7veqM5RHG8KB1QiscarW4UaIefjm19ywSImtIgh$
> ---
> drivers/md/raid1.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index afe2ca96ad8c..f322048ab3c2 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
> int first_clone;
> bool write_behind = false;
> bool nowait = bio->bi_opf & REQ_NOWAIT;
> + bool atomic = bio->bi_opf & REQ_ATOMIC;
> bool is_discard = op_is_discard(bio->bi_opf);
> sector_t sector = bio->bi_iter.bi_sector;
>
> @@ -1603,20 +1604,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
> }
> if (is_bad) {
> int good_sectors;
> -
> - /*
> - * We cannot atomically write this, so just
> - * error in that case. It could be possible to
> - * atomically write other mirrors, but the
> - * complexity of supporting that is not worth
> - * the benefit.
> - */
> - if (bio->bi_opf & REQ_ATOMIC) {
> - bio->bi_status = BLK_STS_NOTSUPP;
> - bio_endio(bio);
> - goto err_dec_pending;
> - }
> -
> good_sectors = first_bad - sector;
> if (good_sectors < max_sectors)
> max_sectors = good_sectors;
> @@ -1633,10 +1620,24 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
> * at a time and thus needs a new bio that can fit the whole payload
> * this bio in page sized chunks.
> */
> - if (write_behind && mddev->bitmap)
> - max_sectors = min_t(int, max_sectors,
> - BIO_MAX_VECS * (PAGE_SIZE >> 9));
> + if (write_behind && mddev->bitmap) {
> + if (atomic && max_sectors > BIO_MAX_VECS * (PAGE_SIZE >> 9))
where does BIO_MAX_VECS * (PAGE_SIZE >> 9) even come from?
> + /*
> + * Atomic writes cannot be split, so disable
> + * write-behind.
> + */
> + write_behind = false;
> + else
> + max_sectors = min_t(int, max_sectors,
> + BIO_MAX_VECS * (PAGE_SIZE >> 9));
> + }
> +
> if (max_sectors < bio_sectors(bio)) {
> + if (atomic) {
> + bio_io_error(bio);
> + goto err_dec_pending;
> + }
> +
> bio = bio_submit_split_bioset(bio, max_sectors,
> &conf->bio_split);
> if (!bio)
> @@ -3229,6 +3230,7 @@ static int raid1_set_limits(struct mddev *mddev)
> lim.max_write_zeroes_sectors = 0;
> lim.max_hw_wzeroes_unmap_sectors = 0;
> lim.logical_block_size = mddev->logical_block_size;
> + lim.atomic_write_hw_unit_max = BARRIER_UNIT_SECTOR_SIZE;
This BARRIER_UNIT_SECTOR_SIZE is a bit like chunk sectors, no? I am just
wondering if we just should set it to chunk sectors =
BARRIER_UNIT_SECTOR_SIZE
I assume that it affects more than Reads and writes, e.g. discard also.
> lim.features |= BLK_FEAT_ATOMIC_WRITES;
> lim.features |= BLK_FEAT_PCI_P2PDMA;
> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
^ permalink raw reply
* Re: cleanup the RAID6 P/Q library v3
From: Christoph Hellwig @ 2026-06-29 12:48 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Christoph Hellwig, Andrew Morton, Catalin Marinas, Will Deacon,
Ard Biesheuvel, Huacai Chen, WANG Xuerui, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Herbert Xu,
Dan Williams, Chris Mason, David Sterba, Arnd Bergmann, Song Liu,
Yu Kuai, Li Nan, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-riscv, linux-s390, linux-crypto, linux-btrfs,
linux-arch, linux-raid
In-Reply-To: <8684FEC2-AD56-4BDA-874F-1D3D7170F880@zytor.com>
On Mon, Jun 29, 2026 at 05:34:41AM -0700, H. Peter Anvin wrote:
> The real issue though is that the benchmarking isn't really helpful, either.
>
> It has to do with power across the whole chip, so it perversely enough depends on what *else* is going on...
Yes. And preferences might matter if this is just a file server or
something running a real workload as well. But I don't really have a
good answer to that.
^ permalink raw reply
* Re: cleanup the RAID6 P/Q library v3
From: H. Peter Anvin @ 2026-06-29 12:34 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Andrew Morton, Catalin Marinas, Will Deacon, Ard Biesheuvel,
Huacai Chen, WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260629122152.GA21344@lst.de>
On June 29, 2026 5:21:52 AM PDT, Christoph Hellwig <hch@lst.de> wrote:
>On Fri, Jun 26, 2026 at 05:52:18PM -0700, H. Peter Anvin wrote:
>> On 2026-05-19 01:24, Christoph Hellwig wrote:
>> >
>> > raid6: rework registration of optimized algorithms
>> >
>> > - avx2 instead of avx512 is probably the right thing for no
>> > benchmarking, but if it was intentional (it wasn't), that should
>> > be document. So I'll just switch back to the previous version to
>> > keep the state of the art
>>
>> It is unlikely to be the right thing *going forward*, though.
>>
>> The very unfortunate performance inversion is likely model-specific. It is one
>> of those things where you largely would have to have a list of quirks :(
>
>Note that the default is still to benchmark the implementations.
>This is just about the case where the user explicitly disabled that
>benchmark-based selection.
>
The real issue though is that the benchmarking isn't really helpful, either.
It has to do with power across the whole chip, so it perversely enough depends on what *else* is going on...
^ permalink raw reply
* Re: cleanup the RAID6 P/Q library v3
From: Christoph Hellwig @ 2026-06-29 12:21 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Christoph Hellwig, Andrew Morton, Catalin Marinas, Will Deacon,
Ard Biesheuvel, Huacai Chen, WANG Xuerui, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Herbert Xu,
Dan Williams, Chris Mason, David Sterba, Arnd Bergmann, Song Liu,
Yu Kuai, Li Nan, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-riscv, linux-s390, linux-crypto, linux-btrfs,
linux-arch, linux-raid
In-Reply-To: <7d8efc7c-65e3-49f8-9381-910de7affef2@zytor.com>
On Fri, Jun 26, 2026 at 05:52:18PM -0700, H. Peter Anvin wrote:
> On 2026-05-19 01:24, Christoph Hellwig wrote:
> >
> > raid6: rework registration of optimized algorithms
> >
> > - avx2 instead of avx512 is probably the right thing for no
> > benchmarking, but if it was intentional (it wasn't), that should
> > be document. So I'll just switch back to the previous version to
> > keep the state of the art
>
> It is unlikely to be the right thing *going forward*, though.
>
> The very unfortunate performance inversion is likely model-specific. It is one
> of those things where you largely would have to have a list of quirks :(
Note that the default is still to benchmark the implementations.
This is just about the case where the user explicitly disabled that
benchmark-based selection.
^ permalink raw reply
* Re: [PATCH v2 3/7] md/raid10: consistently fail atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-06-28 21:35 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-raid, yukuai
In-Reply-To: <20260628143620.64A6C1F000E9@smtp.kernel.org>
Hi Kuai,
On Sun, Jun 28, 2026 at 14:36 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The `err_dec_pending` (formerly `err_handle`) cleanup path in `raid10_write_request` can trigger a NULL pointer dereference when a replacement device is promoted concurrently.
> --
>
> commit bc9d81fdb2c33f8c55baad27ab1eedf427ae25ed
> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>
> md/raid10: consistently fail atomic writes that require splitting
>
> RAID10 handles one badblock path explicitly by failing atomic writes with
> EIO, but another path can also reduce the writable range and force the bio
> through bio_submit_split_bioset(), completing it with EINVAL. This commit
> centralizes the check to fail all atomic writes requiring splitting with
> EIO.
>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index bd322eccdc3ff..3480fc7907f01 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
> [ ... ]
>> @@ -1493,6 +1484,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
>> r10_bio->sectors = max_sectors;
>>
>> if (r10_bio->sectors < bio_sectors(bio)) {
>> + if (atomic)
>> + goto err_handle;
>> +
>
> [Severity: High]
> This is a pre-existing issue, but can this jump to the error handler cause a
> null pointer dereference if a replacement device is concurrently promoted?
>
> In the error handler path, the code fetches the replacement device without
> checking for null or falling back to the primary slot:
>
> err_handle:
> for (k = 0; k < i; k++) {
> int d = r10_bio->devs[k].devnum;
> struct md_rdev *rrdev = conf->mirrors[d].replacement;
> ...
> if (r10_bio->devs[k].repl_bio) {
> rdev_dec_pending(rrdev, mddev);
> r10_bio->devs[k].repl_bio = NULL;
> }
> }
>
> If raid10_remove_disk() runs concurrently, it can promote the replacement to
> the primary slot and clear conf->mirrors[d].replacement:
>
> raid10_remove_disk() {
> ...
> WRITE_ONCE(*rdevp, NULL);
> if (p->replacement) {
> WRITE_ONCE(p->rdev, p->replacement);
> clear_bit(Replacement, &p->replacement->flags);
> WRITE_ONCE(p->replacement, NULL);
> }
> ...
> }
>
At first look this looks unreachable, because raid10_remove_disk()
should only run while the array is suspended, which would drain the
in-flight write before any promotion. But that assumption does not hold
on the md_start_sync() path. The suspend there is gated on a lock-free
check of md_spares_need_change() taken once at function entry; we only
suspend if it returns true. We then call remove_spares()
unconditionally, regardless of that earlier decision, and
remove_spares() re-evaluates rdev_removeable() independently under the
lock.
So the suspend decision and the actual removal are sampled at different
times with nothing held across them. A primary that was In_sync at the
entry check (suspend skipped) but fails afterward becomes removeable by
the time remove_spares() runs, and raid10_remove_disk() then promotes
the replacement and clears the slot with no suspend in effect. Meanwhile
an in-flight raid10_write_request() that referenced only the replacement
(because the primary was already Faulty when it ran) can be sitting in
err_handle, and it reads conf->mirrors[d].replacement as NULL.
It seems real. I'll submit a fix addressing this issue.
> If this concurrent promotion happens, will rrdev be null here? It looks like
> other paths such as raid10_end_write_request() safely handle this exact
> race by falling back to rdev when replacement is null, but this cleanup
> path does not.
>
>> allow_barrier(conf);
>> bio = bio_submit_split_bioset(bio, r10_bio->sectors,
>> &conf->bio_split);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v2 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
From: sashiko-bot @ 2026-06-28 14:39 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: yukuai, linux-raid
In-Reply-To: <20260628142420.1051027-2-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Execution paths in raid10_write_request and raid10_read_request unconditionally block on REQ_NOWAIT bios.
- [High] Unconditional use of GFP_NOIO for r10_bio allocation can block, violating REQ_NOWAIT semantics.
--
commit d4f532acb9d5fd7506dfbbb4a12a97248c3fb429
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid10: fix r10bio leak in raid10_write_request() error paths
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.
Link: https://lore.kernel.org/linux-raid/20260613184042.BCEC01F000E9@smtp.kernel.org/
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df8..bd322eccdc3ff 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1365,6 +1365,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> /* Bail out if REQ_NOWAIT is set for the bio */
> if (bio->bi_opf & REQ_NOWAIT) {
> bio_wouldblock_error(bio);
> + free_r10bio(r10_bio);
> return false;
> }
[ ... ]
> @@ -1398,6 +1399,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> if (bio->bi_opf & REQ_NOWAIT) {
> allow_barrier(conf);
> bio_wouldblock_error(bio);
> + free_r10bio(r10_bio);
> return false;
> }
[ ... ]
[Severity: High]
This is a pre-existing issue, but does wait_blocked_dev() still block
unconditionally later in raid10_write_request() even when REQ_NOWAIT is set?
raid10_write_request() {
...
r10_bio->read_slot = -1;
raid10_find_phys(conf, r10_bio);
wait_blocked_dev(mddev, r10_bio);
...
}
Inside wait_blocked_dev(), the code calls md_wait_for_blocked_rdev() and
wait_barrier(), both of which can block indefinitely. Can this cause
unintended latency spikes or thread starvation for userspace applications
issuing async I/O?
Similarly, in raid10_read_request(), when a bio is split across a chunk
boundary, it calls wait_barrier(conf, false) without checking the
parent bio's REQ_NOWAIT flag.
[Severity: High]
This is another pre-existing issue, but do the mempool_alloc() calls in
__make_request() and raid10_handle_discard() bypass REQ_NOWAIT semantics?
__make_request() {
...
r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
...
}
If the mempool is depleted and memory is low, this GFP_NOIO allocation can
block. Should this use GFP_NOWAIT for REQ_NOWAIT bios and return
BLK_STS_AGAIN when the pool is exhausted to preserve the non-blocking
contract?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: sashiko-bot @ 2026-06-28 14:38 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: linux-raid, yukuai
In-Reply-To: <20260628142420.1051027-3-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Atomic write limits are improperly configured and completely overwritten during limit stacking.
--
commit c2837eff05b0ac2b80653d924f40f1bef7bad6d3
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid1: advertise atomic write limits and handle runtime constraints
Atomic writes in RAID1 must fit within a single barrier unit. Advertise
this restriction through the queue limits by setting
atomic_write_hw_unit_max to BARRIER_UNIT_SECTOR_SIZE so that bios which
would cross a barrier-unit boundary are rejected by the block layer
before reaching MD.
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.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1633,10 +1620,24 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
[ ... ]
> if (max_sectors < bio_sectors(bio)) {
> + if (atomic) {
> + bio_io_error(bio);
> + goto err_dec_pending;
> + }
> +
[Severity: High]
Will this result in atomic writes dynamically failing with unexpected I/O
errors instead of being properly rejected early by the block layer?
Because the limits configured in raid1_set_limits() appear to get overwritten
during stacking, boundary-crossing bios will bypass block layer validation and
trigger an error dynamically here.
> bio = bio_submit_split_bioset(bio, max_sectors,
> &conf->bio_split);
[ ... ]
> @@ -3229,6 +3230,7 @@ static int raid1_set_limits(struct mddev *mddev)
> lim.max_write_zeroes_sectors = 0;
> lim.max_hw_wzeroes_unmap_sectors = 0;
> lim.logical_block_size = mddev->logical_block_size;
> + lim.atomic_write_hw_unit_max = BARRIER_UNIT_SECTOR_SIZE;
> + lim.features |= BLK_FEAT_ATOMIC_WRITES;
> + lim.features |= BLK_FEAT_PCI_P2PDMA;
> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
[Severity: High]
Is BARRIER_UNIT_SECTOR_SIZE being assigned in sectors while the
atomic_write_hw_unit_max limit expects bytes? If so, this might artificially
cap atomic writes to mismatched units.
Additionally, will these custom RAID1 atomic limits be completely overwritten
when stacking?
Because atomic_write_hw_max is left unmodified from its default of UINT_MAX,
when mddev_stack_rdev_limits() is called:
mddev_stack_rdev_limits() ->
queue_limits_stack_bdev() ->
blk_stack_atomic_writes_limits() ->
blk_stack_atomic_writes_head()
The stacking code will see UINT_MAX and unconditionally overwrite the newly
configured RAID1 limit with the first underlying device's limits.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v2 3/7] md/raid10: consistently fail atomic writes that require splitting
From: sashiko-bot @ 2026-06-28 14:36 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: linux-raid, yukuai
In-Reply-To: <20260628142420.1051027-4-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `err_dec_pending` (formerly `err_handle`) cleanup path in `raid10_write_request` can trigger a NULL pointer dereference when a replacement device is promoted concurrently.
--
commit bc9d81fdb2c33f8c55baad27ab1eedf427ae25ed
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid10: consistently fail atomic writes that require splitting
RAID10 handles one badblock path explicitly by failing atomic writes with
EIO, but another path can also reduce the writable range and force the bio
through bio_submit_split_bioset(), completing it with EINVAL. This commit
centralizes the check to fail all atomic writes requiring splitting with
EIO.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index bd322eccdc3ff..3480fc7907f01 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -1493,6 +1484,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> r10_bio->sectors = max_sectors;
>
> if (r10_bio->sectors < bio_sectors(bio)) {
> + if (atomic)
> + goto err_handle;
> +
[Severity: High]
This is a pre-existing issue, but can this jump to the error handler cause a
null pointer dereference if a replacement device is concurrently promoted?
In the error handler path, the code fetches the replacement device without
checking for null or falling back to the primary slot:
err_handle:
for (k = 0; k < i; k++) {
int d = r10_bio->devs[k].devnum;
struct md_rdev *rrdev = conf->mirrors[d].replacement;
...
if (r10_bio->devs[k].repl_bio) {
rdev_dec_pending(rrdev, mddev);
r10_bio->devs[k].repl_bio = NULL;
}
}
If raid10_remove_disk() runs concurrently, it can promote the replacement to
the primary slot and clear conf->mirrors[d].replacement:
raid10_remove_disk() {
...
WRITE_ONCE(*rdevp, NULL);
if (p->replacement) {
WRITE_ONCE(p->rdev, p->replacement);
clear_bit(Replacement, &p->replacement->flags);
WRITE_ONCE(p->replacement, NULL);
}
...
}
If this concurrent promotion happens, will rrdev be null here? It looks like
other paths such as raid10_end_write_request() safely handle this exact
race by falling back to rdev when replacement is null, but this cleanup
path does not.
> allow_barrier(conf);
> bio = bio_submit_split_bioset(bio, r10_bio->sectors,
> &conf->bio_split);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
^ permalink raw reply
* [PATCH] md: remove REQ_NOWAIT support
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:27 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, vverma, axboe, linux-raid,
linux-kernel
Cc: Abd-Alrhman Masalkhi
REQ_NOWAIT support in md is fundamentally incomplete. While reads can
avoid some blocking paths, write requests can still encounter cases
where one mirror succeeds while another returns -EAGAIN. At that point
md cannot distinguish queue pressure from a real device failure, so it
can neither record a bad block nor safely retry the write without
REQ_NOWAIT, leaving mirrors with divergent data.
Rather than continue advertising REQ_NOWAIT support that cannot be
implemented correctly, remove it from md and its raid personalities.
This simplifies the waiting paths and avoids exposing incorrect
non-blocking semantics.
Fixes: bf2c411bb1cf ("md: raid456 add nowait support")
Fixes: c9aa889b035f ("md: raid10 add nowait support")
Fixes: 5aa705039c4f ("md: raid1 add nowait support")
Fixes: f51d46d0e7cb ("md: add support for REQ_NOWAIT")
Suggested-by: Yu Kuai <yukuai@fygo.io>
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/md/md-bitmap.c | 9 +---
drivers/md/md-bitmap.h | 2 +-
drivers/md/md-llbitmap.c | 10 +----
drivers/md/md.c | 15 ++-----
drivers/md/raid1-10.c | 8 ++--
drivers/md/raid1.c | 91 +++++++++-------------------------------
drivers/md/raid10.c | 80 +++++++++--------------------------
drivers/md/raid5.c | 13 ------
8 files changed, 51 insertions(+), 177 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 0f02e2956398..7d778fe1c47c 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2064,23 +2064,18 @@ static void bitmap_end_behind_write(struct mddev *mddev)
bitmap->mddev->bitmap_info.max_write_behind);
}
-static bool bitmap_wait_behind_writes(struct mddev *mddev, bool nowait)
+static void bitmap_wait_behind_writes(struct mddev *mddev)
{
struct bitmap *bitmap = mddev->bitmap;
/* wait for behind writes to complete */
if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
- if (nowait)
- return false;
-
pr_debug("md:%s: behind writes in progress - waiting to stop.\n",
mdname(mddev));
/* need to kick something here to make sure I/O goes? */
wait_event(bitmap->behind_wait,
atomic_read(&bitmap->behind_writes) == 0);
}
-
- return true;
}
static void bitmap_destroy(struct mddev *mddev)
@@ -2090,7 +2085,7 @@ static void bitmap_destroy(struct mddev *mddev)
if (!bitmap) /* there was no bitmap */
return;
- bitmap_wait_behind_writes(mddev, false);
+ bitmap_wait_behind_writes(mddev);
if (!test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
mddev_destroy_serial_pool(mddev, NULL);
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index f46674bdfeb9..214f623c7e79 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -98,7 +98,7 @@ struct bitmap_operations {
void (*start_behind_write)(struct mddev *mddev);
void (*end_behind_write)(struct mddev *mddev);
- bool (*wait_behind_writes)(struct mddev *mddev, bool nowait);
+ void (*wait_behind_writes)(struct mddev *mddev);
md_bitmap_fn *start_write;
md_bitmap_fn *end_write;
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 5a4e2abaa757..2a2b38c663c3 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1574,19 +1574,13 @@ static void llbitmap_end_behind_write(struct mddev *mddev)
wake_up(&llbitmap->behind_wait);
}
-static bool llbitmap_wait_behind_writes(struct mddev *mddev, bool nowait)
+static void llbitmap_wait_behind_writes(struct mddev *mddev)
{
struct llbitmap *llbitmap = mddev->bitmap;
- if (llbitmap && atomic_read(&llbitmap->behind_writes) > 0) {
- if (nowait)
- return false;
-
+ if (llbitmap && atomic_read(&llbitmap->behind_writes) > 0)
wait_event(llbitmap->behind_wait,
atomic_read(&llbitmap->behind_writes) == 0);
- }
-
- return true;
}
static ssize_t bits_show(struct mddev *mddev, char *page)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..66a41d482e59 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -402,14 +402,9 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
*/
percpu_ref_get(&mddev->active_io);
} else {
- if (is_suspended(mddev, bio)) {
- /* Bail out if REQ_NOWAIT is set for the bio */
- if (bio->bi_opf & REQ_NOWAIT) {
- bio_wouldblock_error(bio);
- return true;
- }
+ if (is_suspended(mddev, bio))
wait_event(mddev->sb_wait, !is_suspended(mddev, bio));
- }
+
if (!percpu_ref_tryget_live(&mddev->active_io))
goto check_suspended;
}
@@ -6283,7 +6278,7 @@ void md_init_stacking_limits(struct queue_limits *lim)
{
blk_set_stacking_limits(lim);
lim->features = BLK_FEAT_WRITE_CACHE | BLK_FEAT_FUA |
- BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT;
+ BLK_FEAT_IO_STAT;
}
EXPORT_SYMBOL_GPL(md_init_stacking_limits);
@@ -6631,7 +6626,6 @@ int md_run(struct mddev *mddev)
int err;
struct md_rdev *rdev;
struct md_personality *pers;
- bool nowait = true;
if (list_empty(&mddev->disks))
/* cannot run an array with no devices.. */
@@ -6702,7 +6696,6 @@ int md_run(struct mddev *mddev)
}
}
sysfs_notify_dirent_safe(rdev->sysfs_state);
- nowait = nowait && bdev_nowait(rdev->bdev);
}
pers = get_pers(mddev->level, mddev->clevel);
@@ -7050,7 +7043,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes);
static void mddev_detach(struct mddev *mddev)
{
if (md_bitmap_enabled(mddev, false))
- mddev->bitmap_ops->wait_behind_writes(mddev, false);
+ mddev->bitmap_ops->wait_behind_writes(mddev);
if (mddev->pers && mddev->pers->quiesce && !is_md_suspended(mddev)) {
mddev->pers->quiesce(mddev, 1);
mddev->pers->quiesce(mddev, 0);
diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index 56a56a4da4f8..3b0e230692ba 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -290,9 +290,8 @@ static inline bool raid1_should_read_first(struct mddev *mddev,
}
/*
- * bio with REQ_RAHEAD or REQ_NOWAIT can fail at anytime, before such IO is
- * submitted to the underlying disks, hence don't record badblocks or retry
- * in this case.
+ * bio with REQ_RAHEAD can fail at anytime, before such IO is submitted to the
+ * underlying disks, hence don't record badblocks or retry in this case.
*
* BLK_STS_INVAL means the bio was not valid for the underlying device. This
* is a user error, not a device failure, so retrying or recording bad blocks
@@ -300,6 +299,5 @@ static inline bool raid1_should_read_first(struct mddev *mddev,
*/
static inline bool raid1_should_handle_error(struct bio *bio)
{
- return !(bio->bi_opf & (REQ_RAHEAD | REQ_NOWAIT)) &&
- bio->bi_status != BLK_STS_INVAL;
+ return !(bio->bi_opf & REQ_RAHEAD) && bio->bi_status != BLK_STS_INVAL;
}
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index f322048ab3c2..a217c9e2d504 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1051,10 +1051,8 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr)
wake_up(&conf->wait_barrier);
}
-static bool _wait_barrier(struct r1conf *conf, int idx, bool nowait)
+static void _wait_barrier(struct r1conf *conf, int idx)
{
- bool ret = true;
-
/*
* We need to increase conf->nr_pending[idx] very early here,
* then raise_barrier() can be blocked when it waits for
@@ -1085,7 +1083,7 @@ static bool _wait_barrier(struct r1conf *conf, int idx, bool nowait)
*/
if (!READ_ONCE(conf->array_frozen) &&
!atomic_read(&conf->barrier[idx]))
- return ret;
+ return;
/*
* After holding conf->resync_lock, conf->nr_pending[idx]
@@ -1104,26 +1102,18 @@ static bool _wait_barrier(struct r1conf *conf, int idx, bool nowait)
wake_up_barrier(conf);
/* Wait for the barrier in same barrier unit bucket to drop. */
- /* Return false when nowait flag is set */
- if (nowait) {
- ret = false;
- } else {
- wait_event_lock_irq(conf->wait_barrier,
- !conf->array_frozen &&
- !atomic_read(&conf->barrier[idx]),
- conf->resync_lock);
- atomic_inc(&conf->nr_pending[idx]);
- }
+ wait_event_lock_irq(conf->wait_barrier, !conf->array_frozen &&
+ !atomic_read(&conf->barrier[idx]),
+ conf->resync_lock);
+ atomic_inc(&conf->nr_pending[idx]);
atomic_dec(&conf->nr_waiting[idx]);
spin_unlock_irq(&conf->resync_lock);
- return ret;
}
-static bool wait_read_barrier(struct r1conf *conf, sector_t sector_nr, bool nowait)
+static void wait_read_barrier(struct r1conf *conf, sector_t sector_nr)
{
int idx = sector_to_idx(sector_nr);
- bool ret = true;
/*
* Very similar to _wait_barrier(). The difference is, for read
@@ -1135,7 +1125,7 @@ static bool wait_read_barrier(struct r1conf *conf, sector_t sector_nr, bool nowa
atomic_inc(&conf->nr_pending[idx]);
if (!READ_ONCE(conf->array_frozen))
- return ret;
+ return;
spin_lock_irq(&conf->resync_lock);
atomic_inc(&conf->nr_waiting[idx]);
@@ -1147,27 +1137,19 @@ static bool wait_read_barrier(struct r1conf *conf, sector_t sector_nr, bool nowa
wake_up_barrier(conf);
/* Wait for array to be unfrozen */
- /* Return false when nowait flag is set */
- if (nowait) {
- /* Return false when nowait flag is set */
- ret = false;
- } else {
- wait_event_lock_irq(conf->wait_barrier,
- !conf->array_frozen,
- conf->resync_lock);
- atomic_inc(&conf->nr_pending[idx]);
- }
+ wait_event_lock_irq(conf->wait_barrier, !conf->array_frozen,
+ conf->resync_lock);
+ atomic_inc(&conf->nr_pending[idx]);
atomic_dec(&conf->nr_waiting[idx]);
spin_unlock_irq(&conf->resync_lock);
- return ret;
}
-static bool wait_barrier(struct r1conf *conf, sector_t sector_nr, bool nowait)
+static void wait_barrier(struct r1conf *conf, sector_t sector_nr)
{
int idx = sector_to_idx(sector_nr);
- return _wait_barrier(conf, idx, nowait);
+ _wait_barrier(conf, idx);
}
static void _allow_barrier(struct r1conf *conf, int idx)
@@ -1342,7 +1324,6 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
int max_sectors;
int rdisk;
bool r1bio_existed = !!r1_bio;
- bool nowait = bio->bi_opf & REQ_NOWAIT;
/*
* An md cloned bio indicates we are in the error path.
@@ -1362,16 +1343,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
* Still need barrier for READ in case that whole
* array is frozen.
*/
- if (!wait_read_barrier(conf, bio->bi_iter.bi_sector, nowait)) {
- bio_wouldblock_error(bio);
-
- if (r1bio_existed) {
- set_bit(R1BIO_Returned, &r1_bio->state);
- raid_end_bio_io(r1_bio);
- }
-
- return;
- }
+ wait_read_barrier(conf, bio->bi_iter.bi_sector);
if (!r1_bio)
r1_bio = alloc_r1bio(mddev, bio);
@@ -1409,11 +1381,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
* over-take any writes that are 'behind'
*/
mddev_add_trace_msg(mddev, "raid1 wait behind writes");
- if (!mddev->bitmap_ops->wait_behind_writes(mddev, nowait)) {
- bio_wouldblock_error(bio);
- set_bit(R1BIO_Returned, &r1_bio->state);
- goto err_handle;
- }
+ mddev->bitmap_ops->wait_behind_writes(mddev);
}
if (max_sectors < bio_sectors(bio)) {
@@ -1435,7 +1403,6 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
}
read_bio = bio_alloc_clone(mirror->rdev->bdev, bio, gfp,
&mddev->bio_set);
- read_bio->bi_opf &= ~REQ_NOWAIT;
r1_bio->bios[rdisk] = read_bio;
read_bio->bi_iter.bi_sector = r1_bio->sector +
@@ -1454,7 +1421,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
raid_end_bio_io(r1_bio);
}
-static bool wait_blocked_rdev(struct mddev *mddev, struct bio *bio)
+static void wait_blocked_rdev(struct mddev *mddev, struct bio *bio)
{
struct r1conf *conf = mddev->private;
int disks = conf->raid_disks * 2;
@@ -1474,9 +1441,6 @@ static bool wait_blocked_rdev(struct mddev *mddev, struct bio *bio)
set_bit(BlockedBadBlocks, &rdev->flags);
if (rdev_blocked(rdev)) {
- if (bio->bi_opf & REQ_NOWAIT)
- return false;
-
mddev_add_trace_msg(rdev->mddev, "raid1 wait rdev %d blocked",
rdev->raid_disk);
atomic_inc(&rdev->nr_pending);
@@ -1484,8 +1448,6 @@ static bool wait_blocked_rdev(struct mddev *mddev, struct bio *bio)
goto retry;
}
}
-
- return true;
}
static void raid1_start_write_behind(struct mddev *mddev, struct r1bio *r1_bio,
@@ -1521,7 +1483,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
unsigned long flags;
int first_clone;
bool write_behind = false;
- bool nowait = bio->bi_opf & REQ_NOWAIT;
bool atomic = bio->bi_opf & REQ_ATOMIC;
bool is_discard = op_is_discard(bio->bi_opf);
sector_t sector = bio->bi_iter.bi_sector;
@@ -1529,11 +1490,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
if (mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE, sector,
bio_end_sector(bio))) {
-
- if (nowait) {
- bio_wouldblock_error(bio);
- return false;
- }
wait_event_idle(conf->wait_barrier,
!mddev->cluster_ops->area_resyncing(mddev, WRITE,
sector,
@@ -1545,15 +1501,9 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* thread has put up a bar for new requests.
* Continue immediately if no resync is active currently.
*/
- if (!wait_barrier(conf, sector, nowait)) {
- bio_wouldblock_error(bio);
- return false;
- }
+ wait_barrier(conf, sector);
- if (!wait_blocked_rdev(mddev, bio)) {
- bio_wouldblock_error(bio);
- goto err_allow_barrier;
- }
+ wait_blocked_rdev(mddev, bio);
r1_bio = alloc_r1bio(mddev, bio);
r1_bio->sectors = max_sectors;
@@ -1682,7 +1632,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
wait_for_serialization(rdev, r1_bio);
}
- mbio->bi_opf &= ~REQ_NOWAIT;
r1_bio->bios[i] = mbio;
mbio->bi_iter.bi_sector = sector + rdev->data_offset;
@@ -1721,8 +1670,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
}
free_r1bio(r1_bio);
-
-err_allow_barrier:
allow_barrier(conf, sector);
return false;
@@ -1851,7 +1798,7 @@ static void close_sync(struct r1conf *conf)
int idx;
for (idx = 0; idx < BARRIER_BUCKETS_NR; idx++) {
- _wait_barrier(conf, idx, false);
+ _wait_barrier(conf, idx);
_allow_barrier(conf, idx);
}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 01162c483644..c26507a79fbc 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1002,32 +1002,22 @@ static bool wait_barrier_nolock(struct r10conf *conf)
return false;
}
-static bool wait_barrier(struct r10conf *conf, bool nowait)
+static void wait_barrier(struct r10conf *conf)
{
- bool ret = true;
-
if (wait_barrier_nolock(conf))
- return true;
+ return;
write_seqlock_irq(&conf->resync_lock);
if (conf->barrier) {
- /* Return false when nowait flag is set */
- if (nowait) {
- ret = false;
- } else {
- conf->nr_waiting++;
- mddev_add_trace_msg(conf->mddev, "raid10 wait barrier");
- wait_event_barrier(conf, stop_waiting_barrier(conf));
- conf->nr_waiting--;
- }
+ conf->nr_waiting++;
+ mddev_add_trace_msg(conf->mddev, "raid10 wait barrier");
+ wait_event_barrier(conf, stop_waiting_barrier(conf));
+ conf->nr_waiting--;
if (!conf->nr_waiting)
wake_up(&conf->wait_barrier);
}
- /* Only increment nr_pending when we wait */
- if (ret)
- atomic_inc(&conf->nr_pending);
+ atomic_inc(&conf->nr_pending);
write_sequnlock_irq(&conf->resync_lock);
- return ret;
}
static void allow_barrier(struct r10conf *conf)
@@ -1119,28 +1109,22 @@ static void raid10_unplug(struct blk_plug_cb *cb, bool from_schedule)
* currently.
* 2. If IO spans the reshape position. Need to wait for reshape to pass.
*/
-static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
+static void regular_request_wait(struct mddev *mddev, struct r10conf *conf,
struct bio *bio, sector_t sectors)
{
- /* Bail out if REQ_NOWAIT is set for the bio */
- if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT))
- return false;
+ wait_barrier(conf);
while (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
bio->bi_iter.bi_sector < conf->reshape_progress &&
bio->bi_iter.bi_sector + sectors > conf->reshape_progress) {
allow_barrier(conf);
- if (bio->bi_opf & REQ_NOWAIT)
- return false;
-
mddev_add_trace_msg(conf->mddev, "raid10 wait reshape");
wait_event(conf->wait_barrier,
conf->reshape_progress <= bio->bi_iter.bi_sector ||
conf->reshape_progress >= bio->bi_iter.bi_sector +
sectors);
- wait_barrier(conf, false);
+ wait_barrier(conf);
}
- return true;
}
static bool raid10_read_request(struct mddev *mddev, struct bio *bio,
@@ -1189,10 +1173,7 @@ static bool raid10_read_request(struct mddev *mddev, struct bio *bio,
}
}
- if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
- bio_wouldblock_error(bio);
- return false;
- }
+ regular_request_wait(mddev, conf, bio, r10_bio->sectors);
rdev = read_balance(conf, r10_bio, &max_sectors);
if (!rdev) {
@@ -1213,7 +1194,7 @@ static bool raid10_read_request(struct mddev *mddev, struct bio *bio,
allow_barrier(conf);
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
- wait_barrier(conf, false);
+ wait_barrier(conf);
if (!bio)
goto err_dec_pending;
@@ -1227,8 +1208,6 @@ static bool raid10_read_request(struct mddev *mddev, struct bio *bio,
r10_bio->master_bio = bio;
}
read_bio = bio_alloc_clone(rdev->bdev, bio, gfp, &mddev->bio_set);
- read_bio->bi_opf &= ~REQ_NOWAIT;
-
r10_bio->devs[slot].bio = read_bio;
r10_bio->devs[slot].rdev = rdev;
@@ -1267,7 +1246,6 @@ static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
conf->mirrors[devnum].rdev;
mbio = bio_alloc_clone(rdev->bdev, bio, GFP_NOIO, &mddev->bio_set);
- mbio->bi_opf &= ~REQ_NOWAIT;
if (replacement)
r10_bio->devs[n_copy].repl_bio = mbio;
else
@@ -1346,7 +1324,7 @@ static void wait_blocked_dev(struct mddev *mddev, struct r10bio *r10_bio)
"raid10 %s wait rdev %d blocked",
__func__, blocked_rdev->raid_disk);
md_wait_for_blocked_rdev(blocked_rdev, mddev);
- wait_barrier(conf, false);
+ wait_barrier(conf);
goto retry_wait;
}
}
@@ -1357,29 +1335,19 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
struct r10conf *conf = mddev->private;
int i, k;
int max_sectors = r10_bio->sectors;
- bool nowait = bio->bi_opf & REQ_NOWAIT;
bool atomic = bio->bi_opf & REQ_ATOMIC;
if ((mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE,
bio->bi_iter.bi_sector,
bio_end_sector(bio)))) {
- /* Bail out if REQ_NOWAIT is set for the bio */
- if (nowait) {
- bio_wouldblock_error(bio);
- return false;
- }
-
wait_event_idle(conf->wait_barrier,
!mddev->cluster_ops->area_resyncing(mddev, WRITE,
bio->bi_iter.bi_sector,
bio_end_sector(bio)));
}
- if (!regular_request_wait(mddev, conf, bio, max_sectors)) {
- bio_wouldblock_error(bio);
- return false;
- }
+ regular_request_wait(mddev, conf, bio, max_sectors);
if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
(mddev->reshape_backwards
@@ -1392,10 +1360,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
set_mask_bits(&mddev->sb_flags, 0,
BIT(MD_SB_CHANGE_DEVS) | BIT(MD_SB_CHANGE_PENDING));
md_wakeup_thread(mddev->thread);
- if (nowait) {
- bio_wouldblock_error(bio);
- goto err_allow_barrier;
- }
mddev_add_trace_msg(conf->mddev,
"raid10 wait reshape metadata");
wait_event(mddev->sb_wait,
@@ -1626,11 +1590,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
return -EAGAIN;
- if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
- bio_wouldblock_error(bio);
- md_write_end(mddev);
- return 0;
- }
+ wait_barrier(conf);
/*
* Check reshape again to avoid reshape happens after checking
@@ -1681,7 +1641,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
allow_barrier(conf);
/* Resend the fist split part */
submit_bio_noacct(split);
- wait_barrier(conf, false);
+ wait_barrier(conf);
}
div_u64_rem(bio_end, stripe_size, &remainder);
if (remainder) {
@@ -1701,7 +1661,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
/* Resend the second split part */
submit_bio_noacct(bio);
bio = split;
- wait_barrier(conf, false);
+ wait_barrier(conf);
}
bio_start = bio->bi_iter.bi_sector;
@@ -1859,7 +1819,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
end_disk_offset += geo->stride;
atomic_inc(&first_r10bio->remaining);
raid_end_discard_bio(r10_bio);
- wait_barrier(conf, false);
+ wait_barrier(conf);
goto retry_discard;
}
@@ -2059,7 +2019,7 @@ static void print_conf(struct r10conf *conf)
static void close_sync(struct r10conf *conf)
{
- wait_barrier(conf, false);
+ wait_barrier(conf);
allow_barrier(conf);
mempool_exit(&conf->r10buf_pool);
@@ -4694,7 +4654,7 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
if (need_flush ||
time_after(jiffies, conf->reshape_checkpoint + 10*HZ)) {
/* Need to update reshape_position in metadata */
- wait_barrier(conf, false);
+ wait_barrier(conf);
mddev->reshape_position = conf->reshape_progress;
if (mddev->reshape_backwards)
mddev->curr_resync_completed = raid10_size(mddev, 0, 0)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0c5c9fb0606e..5171e221696a 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5718,10 +5718,6 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
struct bio *orig_bi = bi;
int stripe_sectors;
- /* We need to handle this when io_uring supports discard/trim */
- if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
- return;
-
if (mddev->reshape_position != MaxSector)
/* Skip discard while reshape is happening */
return;
@@ -6191,15 +6187,6 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
pr_debug("raid456: %s, logical %llu to %llu\n", __func__,
bi->bi_iter.bi_sector, ctx->last_sector);
- /* Bail out if conflicts with reshape and REQ_NOWAIT is set */
- if ((bi->bi_opf & REQ_NOWAIT) &&
- get_reshape_loc(mddev, conf, logical_sector) == LOC_INSIDE_RESHAPE) {
- bio_wouldblock_error(bi);
- if (rw == WRITE)
- md_write_end(mddev);
- mempool_free(ctx, conf->ctx_pool);
- return true;
- }
md_account_bio(mddev, &bi);
/*
--
2.43.0
^ permalink raw reply related
* [PATCH v2 7/7] md/raid10: simplify read request error handling
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
raid10_read_request() currently handles bio completion, barrier
handling, and r10_bio lifetime management in several different error
paths. This results in duplicated cleanup logic and increases the risk
of introducing bugs in future modifications.
Make raid10_read_request() return a status to its callers, consolidate
the read error paths, and free r10_bio from a single location in the
callers. Since the callers allocate r10_bio, they should also be
responsible for freeing it when the request fails.
This makes the read path follow the same ownership model as the write
path and simplifies the error handling flow.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- Fix a compilation error (bi -> bio).
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-8-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 45 +++++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index d94c1f28a6f6..01162c483644 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1143,7 +1143,7 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
return true;
}
-static void raid10_read_request(struct mddev *mddev, struct bio *bio,
+static bool raid10_read_request(struct mddev *mddev, struct bio *bio,
struct r10bio *r10_bio)
{
struct r10conf *conf = mddev->private;
@@ -1191,8 +1191,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
- return;
+ return false;
}
rdev = read_balance(conf, r10_bio, &max_sectors);
@@ -1202,8 +1201,8 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
mdname(mddev), b,
(unsigned long long)r10_bio->sector);
}
- raid_end_bio_io(r10_bio);
- return;
+ bio_io_error(bio);
+ goto err_allow_barrier;
}
if (err_rdev)
pr_err_ratelimited("md/raid10:%s: %pg: redirecting sector %llu to another mirror\n",
@@ -1215,10 +1214,8 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
wait_barrier(conf, false);
- if (!bio) {
- set_bit(R10BIO_Returned, &r10_bio->state);
- goto err_handle;
- }
+ if (!bio)
+ goto err_dec_pending;
r10_bio->master_bio = bio;
r10_bio->sectors = max_sectors;
@@ -1244,10 +1241,16 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
read_bio->bi_private = r10_bio;
mddev_trace_remap(mddev, read_bio, r10_bio->sector);
submit_bio_noacct(read_bio);
- return;
-err_handle:
+
+ return true;
+
+err_dec_pending:
atomic_dec(&rdev->nr_pending);
- raid_end_bio_io(r10_bio);
+
+err_allow_barrier:
+ allow_barrier(conf);
+
+ return false;
}
static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
@@ -1538,14 +1541,13 @@ static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
conf->geo.raid_disks);
- ret = true;
if (bio_data_dir(bio) == READ)
- raid10_read_request(mddev, bio, r10_bio);
- else {
+ ret = raid10_read_request(mddev, bio, r10_bio);
+ else
ret = raid10_write_request(mddev, bio, r10_bio);
- if (!ret)
- free_r10bio(r10_bio);
- }
+
+ if (!ret)
+ free_r10bio(r10_bio);
return ret;
}
@@ -1875,6 +1877,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sector_t chunk_mask = (conf->geo.chunk_mask & conf->prev.chunk_mask);
int chunk_sects = chunk_mask + 1;
int sectors = bio_sectors(bio);
+ bool write = bio_data_dir(bio) == WRITE;
if (unlikely(bio->bi_opf & REQ_PREFLUSH)
&& md_flush_request(mddev, bio))
@@ -1898,7 +1901,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sectors = chunk_sects -
(bio->bi_iter.bi_sector &
(chunk_sects - 1));
- if (!__make_request(mddev, bio, sectors))
+ if (!__make_request(mddev, bio, sectors) && write)
md_write_end(mddev);
/* In case raid10d snuck in to freeze_array */
@@ -2866,7 +2869,9 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
rdev_dec_pending(rdev, mddev);
r10_bio->state = 0;
- raid10_read_request(mddev, r10_bio->master_bio, r10_bio);
+ if (!raid10_read_request(mddev, r10_bio->master_bio, r10_bio))
+ free_r10bio(r10_bio);
+
/*
* allow_barrier after re-submit to ensure no sync io
* can be issued while regular io pending.
--
2.43.0
^ permalink raw reply related
* [PATCH v2 6/7] md/raid10: simplify write request error handling
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
raid10_write_request() currently handles bio completion, barrier
handling, and r10_bio lifetime management in several different error
paths. This results in duplicated cleanup logic and increases the risk
of introducing bugs in future modifications.
Move bio_wouldblock_error() handling to the callers of
regular_request_wait(), consolidate the write error paths, and free
r10_bio from a single location in __make_request() when
raid10_write_request() fails.
It remove redundant local copies of r10_bio->sectors and use a single
max_sectors variable throughout the function.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-7-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 58 ++++++++++++++++++++++-----------------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 57813f249578..d94c1f28a6f6 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1123,18 +1123,16 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
struct bio *bio, sector_t sectors)
{
/* Bail out if REQ_NOWAIT is set for the bio */
- if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
- bio_wouldblock_error(bio);
+ if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT))
return false;
- }
+
while (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
bio->bi_iter.bi_sector < conf->reshape_progress &&
bio->bi_iter.bi_sector + sectors > conf->reshape_progress) {
allow_barrier(conf);
- if (bio->bi_opf & REQ_NOWAIT) {
- bio_wouldblock_error(bio);
+ if (bio->bi_opf & REQ_NOWAIT)
return false;
- }
+
mddev_add_trace_msg(conf->mddev, "raid10 wait reshape");
wait_event(conf->wait_barrier,
conf->reshape_progress <= bio->bi_iter.bi_sector ||
@@ -1192,6 +1190,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
}
if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
+ bio_wouldblock_error(bio);
free_r10bio(r10_bio);
return;
}
@@ -1354,8 +1353,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
{
struct r10conf *conf = mddev->private;
int i, k;
- sector_t sectors;
- int max_sectors;
+ int max_sectors = r10_bio->sectors;
+ bool nowait = bio->bi_opf & REQ_NOWAIT;
bool atomic = bio->bi_opf & REQ_ATOMIC;
if ((mddev_is_clustered(mddev) &&
@@ -1363,9 +1362,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
bio->bi_iter.bi_sector,
bio_end_sector(bio)))) {
/* Bail out if REQ_NOWAIT is set for the bio */
- if (bio->bi_opf & REQ_NOWAIT) {
+ if (nowait) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
return false;
}
@@ -1375,28 +1373,25 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
bio_end_sector(bio)));
}
- sectors = r10_bio->sectors;
- if (!regular_request_wait(mddev, conf, bio, sectors)) {
- free_r10bio(r10_bio);
+ if (!regular_request_wait(mddev, conf, bio, max_sectors)) {
+ bio_wouldblock_error(bio);
return false;
}
if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
(mddev->reshape_backwards
? (bio->bi_iter.bi_sector < conf->reshape_safe &&
- bio->bi_iter.bi_sector + sectors > conf->reshape_progress)
- : (bio->bi_iter.bi_sector + sectors > conf->reshape_safe &&
+ bio->bi_iter.bi_sector + max_sectors > conf->reshape_progress)
+ : (bio->bi_iter.bi_sector + max_sectors > conf->reshape_safe &&
bio->bi_iter.bi_sector < conf->reshape_progress))) {
/* Need to update reshape_position in metadata */
mddev->reshape_position = conf->reshape_progress;
set_mask_bits(&mddev->sb_flags, 0,
BIT(MD_SB_CHANGE_DEVS) | BIT(MD_SB_CHANGE_PENDING));
md_wakeup_thread(mddev->thread);
- if (bio->bi_opf & REQ_NOWAIT) {
- allow_barrier(conf);
+ if (nowait) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
- return false;
+ goto err_allow_barrier;
}
mddev_add_trace_msg(conf->mddev,
"raid10 wait reshape metadata");
@@ -1421,8 +1416,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
wait_blocked_dev(mddev, r10_bio);
- max_sectors = r10_bio->sectors;
-
for (i = 0; i < conf->copies; i++) {
int d = r10_bio->devs[i].devnum;
struct md_rdev *rdev, *rrdev;
@@ -1479,15 +1472,15 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
r10_bio->sectors = max_sectors;
if (r10_bio->sectors < bio_sectors(bio)) {
- if (atomic)
- goto err_handle;
+ if (atomic) {
+ bio_io_error(bio);
+ goto err_dec_pending;
+ }
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
- if (!bio) {
- set_bit(R10BIO_Returned, &r10_bio->state);
- goto err_handle;
- }
+ if (!bio)
+ goto err_dec_pending;
r10_bio->master_bio = bio;
}
@@ -1505,7 +1498,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
one_write_done(r10_bio);
return true;
-err_handle:
+err_dec_pending:
for (k = 0; k < i; k++) {
int d = r10_bio->devs[k].devnum;
struct md_rdev *rdev = conf->mirrors[d].rdev;
@@ -1521,7 +1514,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
}
}
- raid_end_bio_io(r10_bio);
+err_allow_barrier:
+ allow_barrier(conf);
+
return false;
}
@@ -1546,8 +1541,11 @@ static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
ret = true;
if (bio_data_dir(bio) == READ)
raid10_read_request(mddev, bio, r10_bio);
- else
+ else {
ret = raid10_write_request(mddev, bio, r10_bio);
+ if (!ret)
+ free_r10bio(r10_bio);
+ }
return ret;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 5/7] md/raid10: replace wait loop with wait_event_idle()
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
The wait loop is equivalent to wait_event_idle() and can be simplified
by usaing it for improving readability.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-6-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 2574f60dd771..57813f249578 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1362,22 +1362,17 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
mddev->cluster_ops->area_resyncing(mddev, WRITE,
bio->bi_iter.bi_sector,
bio_end_sector(bio)))) {
- DEFINE_WAIT(w);
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
free_r10bio(r10_bio);
return false;
}
- for (;;) {
- prepare_to_wait(&conf->wait_barrier,
- &w, TASK_IDLE);
- if (!mddev->cluster_ops->area_resyncing(mddev, WRITE,
- bio->bi_iter.bi_sector, bio_end_sector(bio)))
- break;
- schedule();
- }
- finish_wait(&conf->wait_barrier, &w);
+
+ wait_event_idle(conf->wait_barrier,
+ !mddev->cluster_ops->area_resyncing(mddev, WRITE,
+ bio->bi_iter.bi_sector,
+ bio_end_sector(bio)));
}
sectors = r10_bio->sectors;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 4/7] md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
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 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(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 3480fc7907f0..2574f60dd771 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1487,10 +1487,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (atomic)
goto err_handle;
- allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
- wait_barrier(conf, false);
if (!bio) {
set_bit(R10BIO_Returned, &r10_bio->state);
goto err_handle;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/7] md/raid10: consistently fail atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
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 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(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index bd322eccdc3f..3480fc7907f0 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1356,6 +1356,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
int i, k;
sector_t sectors;
int max_sectors;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
if ((mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1464,16 +1465,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (is_bad) {
int good_sectors;
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC)
- goto err_handle;
-
good_sectors = first_bad - dev_sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1493,6 +1484,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
r10_bio->sectors = max_sectors;
if (r10_bio->sectors < bio_sectors(bio)) {
+ if (atomic)
+ goto err_handle;
+
allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
Atomic writes in RAID1 must fit within a single barrier unit. Advertise
this restriction through the queue limits by setting
atomic_write_hw_unit_max to BARRIER_UNIT_SECTOR_SIZE so that bios which
would cross a barrier-unit boundary are rejected by the block layer
before reaching MD.
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")
Signed-off-by: Abd-Alrhman Masalkhi <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 | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index afe2ca96ad8c..f322048ab3c2 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
int first_clone;
bool write_behind = false;
bool nowait = bio->bi_opf & REQ_NOWAIT;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
bool is_discard = op_is_discard(bio->bi_opf);
sector_t sector = bio->bi_iter.bi_sector;
@@ -1603,20 +1604,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
}
if (is_bad) {
int good_sectors;
-
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC) {
- bio->bi_status = BLK_STS_NOTSUPP;
- bio_endio(bio);
- goto err_dec_pending;
- }
-
good_sectors = first_bad - sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1633,10 +1620,24 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* at a time and thus needs a new bio that can fit the whole payload
* this bio in page sized chunks.
*/
- if (write_behind && mddev->bitmap)
- max_sectors = min_t(int, max_sectors,
- BIO_MAX_VECS * (PAGE_SIZE >> 9));
+ if (write_behind && mddev->bitmap) {
+ if (atomic && max_sectors > BIO_MAX_VECS * (PAGE_SIZE >> 9))
+ /*
+ * Atomic writes cannot be split, so disable
+ * write-behind.
+ */
+ write_behind = false;
+ else
+ max_sectors = min_t(int, max_sectors,
+ BIO_MAX_VECS * (PAGE_SIZE >> 9));
+ }
+
if (max_sectors < bio_sectors(bio)) {
+ if (atomic) {
+ bio_io_error(bio);
+ goto err_dec_pending;
+ }
+
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
if (!bio)
@@ -3229,6 +3230,7 @@ static int raid1_set_limits(struct mddev *mddev)
lim.max_write_zeroes_sectors = 0;
lim.max_hw_wzeroes_unmap_sectors = 0;
lim.logical_block_size = mddev->logical_block_size;
+ lim.atomic_write_hw_unit_max = BARRIER_UNIT_SECTOR_SIZE;
lim.features |= BLK_FEAT_ATOMIC_WRITES;
lim.features |= BLK_FEAT_PCI_P2PDMA;
err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, sashiko-bot
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
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>
---
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-2-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0a3cfdd3f5df..bd322eccdc3f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1365,6 +1365,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
+ free_r10bio(r10_bio);
return false;
}
for (;;) {
@@ -1398,6 +1399,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (bio->bi_opf & REQ_NOWAIT) {
allow_barrier(conf);
bio_wouldblock_error(bio);
+ free_r10bio(r10_bio);
return false;
}
mddev_add_trace_msg(conf->mddev,
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/7] md/raid10: fixes, atomic write handling, and error-path cleanup
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
Hi,
This v2 of series contains a mix of bug fixes and cleanups for RAID10,
along with a related atomic write fix for RAID1.
Changes in v2:
- Expand the commit message to explain why the
allow_barrier()/wait_barrier() pair is no longer needed.
- 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.
- Drop the early atomic write split check from raid10_write_request()
and rely on queue limits instead.
- Fix a compilation error (bi -> bio).
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-1-abd.masalkhi@gmail.com/
Thanks,
Abd-alrhman,
Abd-Alrhman Masalkhi (7):
md/raid10: fix r10bio leak in raid10_write_request() error paths
md/raid1: advertise atomic write limits and handle runtime constraints
md/raid10: consistently fail atomic writes that require splitting
md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
md/raid10: replace wait loop with wait_event_idle()
md/raid10: simplify write request error handling
md/raid10: simplify read request error handling
drivers/md/raid1.c | 36 +++++++-------
drivers/md/raid10.c | 118 +++++++++++++++++++++-----------------------
2 files changed, 74 insertions(+), 80 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Abd-Alrhman Masalkhi @ 2026-06-27 11:25 UTC (permalink / raw)
To: Chen Cheng, Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <da95c106-6416-4abd-8fef-3b71986c17f8@gmail.com>
Hi Chen,
On Sat, Jun 27, 2026 at 18:52 +0800, Chen Cheng wrote:
> 在 2026/6/27 18:46, Abd-Alrhman Masalkhi 写道:
>> On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
>>> From: Chen Cheng <chencheng@fnnas.com>
>>>
>>> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
>>> instead of u64 plain read.
>>>
>> unisgned long is not always u64...
>> I can not see what it fixes, is this just to silence KCSAN? If so,
>> please say so in the commit message.
>
> I need to find KCSAN report , I lost this one, but I think it's easy to
> find locklessly path to call md_sync_action(), so. READ_ONCE is need.
>
Thanks. Adding the KCSAN report would be helpful.
>>
>>> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>>>
>>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>>> ---
>>> drivers/md/md.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>> index c5c50640b684..f4415c1a79d9 100644
>>> --- a/drivers/md/md.c
>>> +++ b/drivers/md/md.c
>>> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
>>> return is_recover ? ACTION_RECOVER : ACTION_IDLE;
>>> }
>>>
>>> enum sync_action md_sync_action(struct mddev *mddev)
>>> {
>>> - unsigned long recovery = mddev->recovery;
>>> + unsigned long recovery = READ_ONCE(mddev->recovery);
>>> enum sync_action active_action;
>>>
>>> /*
>>> * frozen has the highest priority, means running sync_thread will be
>>> * stopped immediately, and no new sync_thread can start.
>>> --
>>> 2.54.0
>>>
>>
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* [PATCH] md/raid5: protect lockless reshape_progress accesses
From: Chen Cheng @ 2026-06-27 10:55 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
During reshape:
- reshape_request() advances conf->reshape_progress.
- reshape_request() itself also reads conf->reshape_progress several times
while calculating sector_nr, writepos, and readpos for the current
reshape step.
- use_new_offset() reads conf->reshape_progress locklessly to decide
whether the current stripe should use data_offset or new_data_offset.
one possible scenario is:
CPU1 CPU2
reshape_request()
-> conf->reshape_progress += delta
ops_run_io()
-> use_new_offset(conf, sh)
-> progress = conf->reshape_progress
-> decide old layout or new layout
-> use data_offset or new_data_offset
reshape_progress is the boundary between old and new layout mapping.
If CPU2 observes an unprotected reshape_progress value, it can choose
the wrong layout offset for the current stripe.
if reshape_request() reads reshape_progress multiple times without
a stable snapshot, one iteration of reshape can calculate its range from
inconsistent progress values.
Fixes: 7a6613810785 ("md/raid5: reshape using largest of old and new chunk size")
The race report:
==================================================================
BUG: KCSAN: data-race in ops_run_io / reshape_request
write to 0xffff89a8d3ef2270 of 8 bytes by task 1299 on cpu 7:
reshape_request+0x1292/0x17b0
raid5_sync_request+0x815/0xa00
md_do_sync.cold+0xf8d/0x1516
md_thread+0x15a/0x2d0
[....]
read to 0xffff89a8d3ef2270 of 8 bytes by task 1292 on cpu 9:
ops_run_io+0xc25/0x1960
handle_stripe+0x2273/0x4570
handle_active_stripes.isra.0+0x6e0/0xa50
raid5d+0x7d5/0xb90
[....]
value changed: 0x0000000000173700 -> 0x0000000000173800
==================================================================
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid5.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index eaee7f206ab8..b6809f1a3ab4 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -1011,11 +1011,11 @@ static void stripe_add_to_batch_list(struct r5conf *conf,
/* Determine if 'data_offset' or 'new_data_offset' should be used
* in this stripe_head.
*/
static int use_new_offset(struct r5conf *conf, struct stripe_head *sh)
{
- sector_t progress = conf->reshape_progress;
+ sector_t progress = READ_ONCE(conf->reshape_progress);
/* Need a memory barrier to make sure we see the value
* of conf->generation, or ->data_offset that was set before
* reshape_progress was updated.
*/
smp_rmb();
@@ -6340,24 +6340,25 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
sector_t writepos, readpos, safepos;
sector_t stripe_addr;
int reshape_sectors;
struct list_head stripes;
sector_t retn;
+ sector_t reshape_progress = READ_ONCE(conf->reshape_progress);
if (sector_nr == 0) {
/* If restarting in the middle, skip the initial sectors */
if (mddev->reshape_backwards &&
- conf->reshape_progress < raid5_size(mddev, 0, 0)) {
+ reshape_progress < raid5_size(mddev, 0, 0)) {
sector_nr = raid5_size(mddev, 0, 0)
- - conf->reshape_progress;
+ - reshape_progress;
} else if (mddev->reshape_backwards &&
- conf->reshape_progress == MaxSector) {
+ reshape_progress == MaxSector) {
/* shouldn't happen, but just in case, finish up.*/
sector_nr = MaxSector;
} else if (!mddev->reshape_backwards &&
- conf->reshape_progress > 0)
- sector_nr = conf->reshape_progress;
+ reshape_progress > 0)
+ sector_nr = reshape_progress;
sector_div(sector_nr, new_data_disks);
if (sector_nr) {
mddev->curr_resync_completed = sector_nr;
sysfs_notify_dirent_safe(mddev->sysfs_completed);
*skipped = 1;
@@ -6377,13 +6378,13 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
* the data about to be copied would over-write the source of
* the data at the front of the range. i.e. one new_stripe
* along from reshape_progress new_maps to after where
* reshape_safe old_maps to
*/
- writepos = conf->reshape_progress;
+ writepos = reshape_progress;
sector_div(writepos, new_data_disks);
- readpos = conf->reshape_progress;
+ readpos = reshape_progress;
sector_div(readpos, data_disks);
safepos = conf->reshape_safe;
sector_div(safepos, data_disks);
if (mddev->reshape_backwards) {
if (WARN_ON(writepos < reshape_sectors))
@@ -6404,11 +6405,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
/* Having calculated the 'writepos' possibly use it
* to set 'stripe_addr' which is where we will write to.
*/
if (mddev->reshape_backwards) {
- if (WARN_ON(conf->reshape_progress == 0))
+ if (WARN_ON(reshape_progress == 0))
return MaxSector;
stripe_addr = writepos;
if (WARN_ON((mddev->dev_sectors &
~((sector_t)reshape_sectors - 1)) -
@@ -6514,14 +6515,21 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
set_bit(STRIPE_HANDLE, &sh->state);
}
list_add(&sh->lru, &stripes);
}
spin_lock_irq(&conf->device_lock);
- if (mddev->reshape_backwards)
- conf->reshape_progress -= reshape_sectors * new_data_disks;
- else
- conf->reshape_progress += reshape_sectors * new_data_disks;
+ if (mddev->reshape_backwards) {
+ sector_t progress = conf->reshape_progress;
+
+ progress -= reshape_sectors * new_data_disks;
+ WRITE_ONCE(conf->reshape_progress, progress);
+ } else {
+ sector_t progress = conf->reshape_progress;
+
+ progress += reshape_sectors * new_data_disks;
+ WRITE_ONCE(conf->reshape_progress, progress);
+ }
spin_unlock_irq(&conf->device_lock);
/* Ok, those stripe are ready. We can start scheduling
* reads on the source stripes.
* The source stripes are determined by mapping the first and last
* block on the destination stripes.
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Chen Cheng @ 2026-06-27 10:52 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <m25x341cro.fsf@gmail.com>
在 2026/6/27 18:46, Abd-Alrhman Masalkhi 写道:
> On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
>> instead of u64 plain read.
>>
> unisgned long is not always u64...
> I can not see what it fixes, is this just to silence KCSAN? If so,
> please say so in the commit message.
I need to find KCSAN report , I lost this one, but I think it's easy to
find locklessly path to call md_sync_action(), so. READ_ONCE is need.
>
>> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/md.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index c5c50640b684..f4415c1a79d9 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
>> return is_recover ? ACTION_RECOVER : ACTION_IDLE;
>> }
>>
>> enum sync_action md_sync_action(struct mddev *mddev)
>> {
>> - unsigned long recovery = mddev->recovery;
>> + unsigned long recovery = READ_ONCE(mddev->recovery);
>> enum sync_action active_action;
>>
>> /*
>> * frozen has the highest priority, means running sync_thread will be
>> * stopped immediately, and no new sync_thread can start.
>> --
>> 2.54.0
>>
>
^ 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