* Re: [PATCH 3/3] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
From: David Sterba @ 2026-05-23 14:23 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, hch, linux-raid
In-Reply-To: <ce02ebf8a9a6f5e2357b838df9d7ecce1f0cfd4b.1779412152.git.wqu@suse.com>
On Fri, May 22, 2026 at 06:53:53PM +0930, Qu Wenruo wrote:
> Recently kernel RAID56 lib is trying to remove the unexpected
> single-data-RAID56 (2 disks RAID5 or 3 disk RAID5) support, meanwhile
> btrfs still supports such setup, which means in the long run btrfs has
> to handle such corner case by ourselves.
>
> Thankfully single-data-RAID56 is really RAID1/RAID1C3, since data and
> P/Q stripes all match each other, rotation also makes no difference.
>
> This patch will disguise those single-data-RAID56 chunks as
> RAID1/RAID1C3 chunks.
I don't think this is the right way to fix it. Calculations are an
implementation detail and should be done at the time of the xor_gen or
raid6_call, not touching the upper level structures related to format.
Instead, please add helpers that take the same parameters and do the
switch if there's the edge case of raid5 or raid6, using memcpy. Thanks.
^ permalink raw reply
* Re: [PATCH RFC] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
From: Christoph Hellwig @ 2026-05-22 12:08 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Christoph Hellwig, Qu Wenruo, linux-btrfs, linux-raid
In-Reply-To: <a8de8d07-d262-4f91-96eb-473fcded6549@gmx.com>
On Fri, May 22, 2026 at 07:00:25PM +0930, Qu Wenruo wrote:
> Not sure what specific workload you may want to verify, but my planned
> workload is:
>
> - Create single-data-RAID56 btrfs
> - Fsstress it
> - Unmount
>
> - Mount with one device missing, readonly
> - Do a readonly scrub, should be no error
> - Retry with the remaining device(s)
fsstress doesn't actually check data ingtegrity. So fsx is usually
a better choice for these kinds of tests. Otherwise this sounds good.
^ permalink raw reply
* Re: [PATCH v2 2/3] md/raid1,raid10: fix error-path detection with md_cloned_bio()
From: Abd-Alrhman Masalkhi @ 2026-05-22 11:02 UTC (permalink / raw)
To: Yu Kuai, Xiao Ni; +Cc: song, xni, neilb, shli, linux-raid, linux-kernel, yukuai
In-Reply-To: <aab7e7ea-737a-4bc6-a50c-f6ed7b38c37d@fygo.io>
On Fri, May 22, 2026 at 12:23 +0800, Yu Kuai wrote:
> Hi,
>
> 在 2026/5/21 17:14, Abd-Alrhman Masalkhi 写道:
>> The original bio resubmitted via bio_list is not executing in the raid1d
>> thread context, so blocking on is_suspended() is correct and will not
>> cause a deadlock. The suspension deadlock risk only exists for
>> md_cloned_bio executing in the raid1d thread context in the error path.
>> And in this case it is always true that if we are executing in the
>> raid1d thread context the bio will already be a md_cloned_bio on entry
>> to md_handle_request(), even if current->bio_list was set, because (and as
>> @Xiao has mentioned) every time we split a bio in the error path we are
>> resubmitting the md_cloned_bio remainder, never the original bio.
>
> I do understand this, if original bio or resubmitted bio only split at
> raid1_read_request, there is no problem. What I feel is not safe is that
> bio can split somewhere else.
>
> This is possible for raid5, However, take a closer look at raid1/10, I
> feel it's not possible for resubmitted io to split in bio_split_to_limits.
> Even if it happens, it's probably someone else to blame.
>
Agreed. Since bio_split_to_limits() has been mentioned, I am wondering
whether a resubmitted md_cloned_bio could end up being split there for
raid456. I have not looked into the raid456 code yet; after finishing
raid1 and raid10, I will move on to the others.
I am considering skipping the redundant call to bio_split_to_limits()
when md_cloned_bio() is true. However, I am not sure whether this would
be safe for raid456.
> --
> Thansk,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH RFC] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
From: Qu Wenruo @ 2026-05-22 9:30 UTC (permalink / raw)
To: Christoph Hellwig, Qu Wenruo; +Cc: linux-btrfs, linux-raid
In-Reply-To: <20260522085211.GA6702@lst.de>
在 2026/5/22 18:22, Christoph Hellwig 写道:
> Hi Qu,
>
> this does looks sensible to me. Maybe add an xfstests case that
> exercises these using loop devices?
Sure, I'll add a test case to verify the single-data-RAID56 behavior for
btrfs.
Not sure what specific workload you may want to verify, but my planned
workload is:
- Create single-data-RAID56 btrfs
- Fsstress it
- Unmount
- Mount with one device missing, readonly
- Do a readonly scrub, should be no error
- Retry with the remaining device(s)
If you have something else to add, I'm pretty happy to add.
Thanks,
Qu
^ permalink raw reply
* [PATCH 2/3] btrfs: remove duplicated block group type assignment
From: Qu Wenruo @ 2026-05-22 9:23 UTC (permalink / raw)
To: linux-btrfs; +Cc: hch, linux-raid
In-Reply-To: <cover.1779412152.git.wqu@suse.com>
In the function fill_dummy_bgs(), bg->flags is assigned twice.
Just remove the second assignment.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/block-group.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index a0cb0db68c9a..41f65a3165f8 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -2627,7 +2627,6 @@ static int fill_dummy_bgs(struct btrfs_fs_info *fs_info)
bg->flags = map->type;
bg->cached = BTRFS_CACHE_FINISHED;
bg->used = map->chunk_len;
- bg->flags = map->type;
bg->space_info = btrfs_find_space_info(fs_info, bg->flags);
ret = btrfs_add_block_group_cache(bg);
/*
--
2.54.0
^ permalink raw reply related
* [PATCH 1/3] btrfs: remove btrfs_chunk_map::io_(align|width) members
From: Qu Wenruo @ 2026-05-22 9:23 UTC (permalink / raw)
To: linux-btrfs; +Cc: hch, linux-raid
In-Reply-To: <cover.1779412152.git.wqu@suse.com>
Those two members are read from on-disk metadata, but never utilized.
And for new chunks we always set those members to BTRFS_STRIPE_LEN
anyway.
Thus there is no need to keep them inside btrfs_chunk_map.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/volumes.c | 4 ----
fs/btrfs/volumes.h | 2 --
2 files changed, 6 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 93a923e4ecaf..daa8f1ddf713 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6019,8 +6019,6 @@ static struct btrfs_block_group *create_chunk(struct btrfs_trans_handle *trans,
map->chunk_len = ctl->chunk_size;
map->stripe_size = ctl->stripe_size;
map->type = type;
- map->io_align = BTRFS_STRIPE_LEN;
- map->io_width = BTRFS_STRIPE_LEN;
map->sub_stripes = ctl->sub_stripes;
map->num_stripes = ctl->num_stripes;
@@ -7583,8 +7581,6 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf,
map->start = logical;
map->chunk_len = length;
map->num_stripes = num_stripes;
- map->io_width = btrfs_chunk_io_width(leaf, chunk);
- map->io_align = btrfs_chunk_io_align(leaf, chunk);
map->type = type;
/*
* We can't use the sub_stripes value, as for profiles other than
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 96904d18f686..f24385b599dc 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -633,8 +633,6 @@ struct btrfs_chunk_map {
u64 chunk_len;
u64 stripe_size;
u64 type;
- int io_align;
- int io_width;
int num_stripes;
int sub_stripes;
struct btrfs_io_stripe stripes[];
--
2.54.0
^ permalink raw reply related
* [PATCH 3/3] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
From: Qu Wenruo @ 2026-05-22 9:23 UTC (permalink / raw)
To: linux-btrfs; +Cc: hch, linux-raid
In-Reply-To: <cover.1779412152.git.wqu@suse.com>
Recently kernel RAID56 lib is trying to remove the unexpected
single-data-RAID56 (2 disks RAID5 or 3 disk RAID5) support, meanwhile
btrfs still supports such setup, which means in the long run btrfs has
to handle such corner case by ourselves.
Thankfully single-data-RAID56 is really RAID1/RAID1C3, since data and
P/Q stripes all match each other, rotation also makes no difference.
This patch will disguise those single-data-RAID56 chunks as
RAID1/RAID1C3 chunks.
This is done at two timings:
- Chunk read
- Chunk allocation
This is done by introducing btrfs_chunk_map::on_disk_type member, which
stores the type read from the on-disk metadata.
Meanwhile btrfs_chunk_map::type is calculated using on_disk_type.
For most profiles @type matches @on_disk_type, but for
single-data-RAID56, the @type will be RAID1/RAID1C3.
This method has a minimal impact on the fs, all other operations like
scrub and read-repair, are all based on the chunk map type, so the
disguise method will require no extra modification to those call sites.
Although there are still some locations that are checking against
block_group->flags, e.g. scrub. Those call sites will still get extra
limits assuming the bg is RAID56. But it should not cause any extra
problem.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/block-group.c | 2 +-
fs/btrfs/volumes.c | 21 ++++++++++++++++++---
fs/btrfs/volumes.h | 8 ++++++++
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 41f65a3165f8..5c9fc7ed4c3e 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -2624,7 +2624,7 @@ static int fill_dummy_bgs(struct btrfs_fs_info *fs_info)
/* Fill dummy cache as FULL */
bg->length = map->chunk_len;
- bg->flags = map->type;
+ bg->flags = map->on_disk_type;
bg->cached = BTRFS_CACHE_FINISHED;
bg->used = map->chunk_len;
bg->space_info = btrfs_find_space_info(fs_info, bg->flags);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index daa8f1ddf713..a754ab7a538f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6000,6 +6000,19 @@ struct btrfs_chunk_map *btrfs_alloc_chunk_map(int num_stripes, gfp_t gfp)
return map;
}
+static void set_real_chunk_type(struct btrfs_chunk_map *map)
+{
+ map->type = map->on_disk_type;
+ if (likely((map->on_disk_type & BTRFS_BLOCK_GROUP_RAID56_MASK) == 0 ||
+ nr_data_stripes(map) > 1))
+ return;
+ if (map->on_disk_type & BTRFS_BLOCK_GROUP_RAID5)
+ map->type |= BTRFS_BLOCK_GROUP_RAID1;
+ else
+ map->type |= BTRFS_BLOCK_GROUP_RAID1C3;
+ map->type &= ~BTRFS_BLOCK_GROUP_RAID56_MASK;
+}
+
static struct btrfs_block_group *create_chunk(struct btrfs_trans_handle *trans,
struct alloc_chunk_ctl *ctl,
struct btrfs_device_info *devices_info)
@@ -6018,9 +6031,10 @@ static struct btrfs_block_group *create_chunk(struct btrfs_trans_handle *trans,
map->start = start;
map->chunk_len = ctl->chunk_size;
map->stripe_size = ctl->stripe_size;
- map->type = type;
+ map->on_disk_type = type;
map->sub_stripes = ctl->sub_stripes;
map->num_stripes = ctl->num_stripes;
+ set_real_chunk_type(map);
for (int i = 0; i < ctl->ndevs; i++) {
for (int j = 0; j < ctl->dev_stripes; j++) {
@@ -6199,7 +6213,7 @@ int btrfs_chunk_alloc_add_chunk_item(struct btrfs_trans_handle *trans,
btrfs_set_stack_chunk_length(chunk, bg->length);
btrfs_set_stack_chunk_owner(chunk, BTRFS_EXTENT_TREE_OBJECTID);
btrfs_set_stack_chunk_stripe_len(chunk, BTRFS_STRIPE_LEN);
- btrfs_set_stack_chunk_type(chunk, map->type);
+ btrfs_set_stack_chunk_type(chunk, map->on_disk_type);
btrfs_set_stack_chunk_num_stripes(chunk, map->num_stripes);
btrfs_set_stack_chunk_io_align(chunk, BTRFS_STRIPE_LEN);
btrfs_set_stack_chunk_io_width(chunk, BTRFS_STRIPE_LEN);
@@ -7581,7 +7595,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf,
map->start = logical;
map->chunk_len = length;
map->num_stripes = num_stripes;
- map->type = type;
+ map->on_disk_type = type;
/*
* We can't use the sub_stripes value, as for profiles other than
* RAID10, they may have 0 as sub_stripes for filesystems created by
@@ -7592,6 +7606,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf,
*/
map->sub_stripes = btrfs_raid_array[index].sub_stripes;
map->verified_stripes = 0;
+ set_real_chunk_type(map);
if (num_stripes > 0)
map->stripe_size = btrfs_calc_stripe_length(map);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index f24385b599dc..f75f516afe0d 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -632,7 +632,15 @@ struct btrfs_chunk_map {
u64 start;
u64 chunk_len;
u64 stripe_size;
+ /*
+ * The real type that is utilized during logical address mapping.
+ *
+ * For most profiles it matches @on_disk_type, but for single-data-RAID56,
+ * the real type will be set to RAID1/RAID1C3, to avoid unsupported
+ * operations from raid56 lib.
+ */
u64 type;
+ u64 on_disk_type;
int num_stripes;
int sub_stripes;
struct btrfs_io_stripe stripes[];
--
2.54.0
^ permalink raw reply related
* [PATCH 0/3] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
From: Qu Wenruo @ 2026-05-22 9:23 UTC (permalink / raw)
To: linux-btrfs; +Cc: hch, linux-raid
[CHANGELOG]
RFC->v1:
- Remove unused btrfs_chunk_map::io_(align|width) members
To make space for the new member.
- Fix a duplicated bg->flags assignment in fill_dummy_bgs()
Just exposed by the last patch which also touches that code.
- Use a dedicated member to record the on-disk chunk type
So it's less fragile than the RFC patch.
The single-data-RAID56 (2 disks raid5 or 3 disks raid6) is not a feature
that should be supported in the first place, thus raid56 is going to
remove such support.
Meanwhile rejecting single-data-RAID56 will cause existing btrfs users
unable to mount their fs, even if there is only a single
empty chunk with that flag, mostly caused by some degraded mount.
So to avoid impacting existing users, implement an internal
single-data-RAID56 to RAID1/RAID1C3 mapping, to co-operate with the
upstream raid56 lib changes.
Qu Wenruo (3):
btrfs: remove btrfs_chunk_map::io_(align|width) members
btrfs: remove duplicated block group type assignment
btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
fs/btrfs/block-group.c | 3 +--
fs/btrfs/volumes.c | 25 ++++++++++++++++++-------
fs/btrfs/volumes.h | 10 ++++++++--
3 files changed, 27 insertions(+), 11 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH RFC] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
From: Christoph Hellwig @ 2026-05-22 8:52 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, hch, linux-raid
In-Reply-To: <a1d63733465229936351804f3760803d5894a962.1779274630.git.wqu@suse.com>
Hi Qu,
this does looks sensible to me. Maybe add an xfstests case that
exercises these using loop devices?
^ permalink raw reply
* Re: [PATCH v2 0/3] md/raid1,raid10: fix deadlock and bio accounting in read error path
From: Yu Kuai @ 2026-05-22 5:23 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, xni, neilb, shli, yukuai
Cc: linux-raid, linux-kernel
In-Reply-To: <20260501114652.590037-1-abd.masalkhi@gmail.com>
在 2026/5/1 19:46, Abd-Alrhman Masalkhi 写道:
> Hi,
>
> This series revisits an issue in the read error recovery path for
> raid1 and raid10 when bios are split and resubmitted.
>
> In v1, the approach was to avoid splitting bios in the raid1 thread
> to prevent recursion and potential deadlocks. However, this was not
> ideal and too restrictive.
>
> As suggested by Yu Kuai, this series instead handles the problem in
> md_handle_request() and allows bio splitting.
>
> Link to Yu Kuai' email: https://lore.kernel.org/linux-raid/m2lde74dtw.fsf@gmail.com/T/#m714020a38b60fc5f84b9a24f0c46acbe5d7342d6
>
> This series fixes the above by:
> - handling md cloned bios explicitly in md_handle_request()
> - using md_cloned_bio() to reliably detect the error path
> - fixing bio accounting to avoid double/missing accounting
>
> Changes in v2:
> - Split fixes into separate patches for clarity.
> - Use md_cloned_bio() consistently to detect cloned bios.
> - Recognize that raid10 has the same issue and fix it in this series
> - Allow splitting bios.
> - Handle md cloned bios explicitly in md_handle_request()
> - Link v1: https://lore.kernel.org/linux-raid/20260427103446.300378-1-abd.masalkhi@gmail.com/
>
> Abd-Alrhman Masalkhi (3):
> md/raid1,raid10: fix deadlock in read error recovery path
> md/raid1,raid10: fix error-path detection with md_cloned_bio()
> md/raid1,raid10: fix bio accounting for split md cloned bios
>
> drivers/md/md.c | 25 ++++++++++++++++---------
> drivers/md/md.h | 5 +++++
> drivers/md/raid1.c | 15 +++++++++++----
> drivers/md/raid10.c | 28 ++++++++++++++++++----------
> 4 files changed, 50 insertions(+), 23 deletions(-)
Applied to md-7.2
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: cleanup reshape stripes when too many devices fail
From: Yu Kuai @ 2026-05-22 4:58 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260518123449.534287-1-chencheng@fnnas.com>
Hi,
在 2026/5/18 20:34, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> When a raid5/6 reshape is in progress and the array loses more than
> max_degraded devices, raid5_error() sets MD_BROKEN and MD_RECOVERY_INTR,
> but the reshape stripes reshape_request() handed out are never released.
> The "s.failed > conf->max_degraded" branch of handle_stripe() calls
> handle_failed_stripe() / handle_failed_sync() for user IO and resync,
> but has no equivalent for the expand case, so three kinds of stripes
> leak conf->reshape_stripes and mddev->recovery_active:
>
> 1. Destination stripes with skipped_disk == 0: STRIPE_EXPANDING +
> STRIPE_EXPAND_READY set, but on a broken array the normal
> completion at "s.expanded && !reconstruct_state && s.locked == 0"
> may never fire.
>
> 2. Destination stripes with skipped_disk == 1: only STRIPE_EXPANDING
> set, no STRIPE_HANDLE. They sit idle in the cache waiting for
> source data that can no longer be read; handle_stripe() is never
> called on them directly.
>
> 3. Source stripes (STRIPE_EXPAND_SOURCE) hit the failure branch but
> the bit is never cleared and the destinations they feed are never
> released.
>
> md_do_sync() exits its main loop on MD_RECOVERY_INTR but then blocks
> forever at
>
> wait_event(mddev->recovery_wait,
> !atomic_read(&mddev->recovery_active));
>
> A concurrent "echo frozen > sync_action" then blocks in
> stop_sync_thread() waiting for MD_RECOVERY_RUNNING to clear, and the
> array becomes unstoppable without a reboot.
>
> Reproducer:
>
> DEVS=(/dev/sdb /dev/sdc /dev/sdd /dev/sde /dev/sdf)
> for i in 0 1 2 3 4; do
> s=$(blockdev --getsz ${DEVS[$i]})
> dmsetup create dust$i --table "0 $s dust ${DEVS[$i]} 0 4096"
> dmsetup message dust$i 0 quiet
> done
> mdadm -C /dev/md0 -e 1.2 -l 5 -n 4 -c 64 --assume-clean \
> /dev/mapper/dust{0..3}
> for b in $(seq 0 8191); do
> dmsetup message dust0 0 addbadblock $b
> dmsetup message dust1 0 addbadblock $b
> done
> mdadm --manage /dev/md0 --add /dev/mapper/dust4
> mdadm --grow /dev/md0 -n 5 --backup-file=/tmp/grow.backup &
> while [[ $(cat /sys/block/md0/md/sync_action) != reshape ]]; do
> sleep 0.1
> done
> dmsetup message dust0 0 enable
> dmsetup message dust1 0 enable
> sleep 5
> echo frozen > /sys/block/md0/md/sync_action # hangs forever
>
> Before the fix, the two tasks deadlock against each other:
>
> task:md0_reshape state:D
> schedule
> md_do_sync.cold+0x818/0xc25 # wait_event(recovery_wait,
> md_thread # !recovery_active)
> kthread
>
> task:bash state:D
> schedule
> stop_sync_thread+0x1a3/0x350 # wait_event(resync_wait,
> action_store # !MD_RECOVERY_RUNNING)
> md_attr_store
> kernfs_fop_write_iter
> vfs_write
> ksys_write
>
> After the fix handle_stripe() releases the leaked reshape stripes via
> the new handle_failed_reshape(), recovery_active drains to zero,
> md_do_sync() prints
>
> md/raid:md0: Cannot continue operation (2/5 failed).
> md: md0: reshape interrupted.
>
> clears MD_RECOVERY_RUNNING and returns; the "echo frozen" write
> returns in <1s; "mdadm --stop /dev/md0" completes normally and no
> task is left in D state.
>
> Fix it by adding handle_failed_reshape(), called from handle_stripe()
> when the failure branch fires on a reshape stripe. If sh is a
> destination, the helper drops STRIPE_EXPANDING / STRIPE_EXPAND_READY,
> decrements conf->reshape_stripes, wakes wait_for_reshape and calls
> md_done_sync() to return the sectors reshape_request() accounted on
> recovery_active. If sh is a source, the helper drops
> STRIPE_EXPAND_SOURCE and walks sh's non-parity data disks using the
> same raid5_compute_blocknr() / raid5_compute_sector() mapping
> handle_stripe_expansion() uses to forward data, looks up each matching
> destination with R5_GAS_NOBLOCK | R5_GAS_NOQUIESCE and applies the
> destination cleanup to it.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 91 insertions(+)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0d76e82f4506..f7d159b46a01 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -4594,10 +4594,96 @@ static void handle_stripe_expansion(struct r5conf *conf, struct stripe_head *sh)
> }
> /* done submitting copies, wait for them to complete */
> async_tx_quiesce(&tx);
> }
>
> +/*
> + * handle_failed_reshape - drop reshape state when too many devices have failed
> + *
> + * Called from handle_stripe() in the "s.failed > conf->max_degraded" branch
> + * when sh is participating in a reshape. raid5_error() has set MD_BROKEN
> + * and MD_RECOVERY_INTR); The reshape stripes that reshape_request() handed out
> + * must be released, otherwise they leak conf->reshape_stripes and
> + * mddev->recovery_active, and md_do_sync() hangs forever at
> + * wait_event(mddev->recovery_wait, !atomic_read(&mddev->recovery_active)).
> + *
> + * Three kinds of stripes can reach this path:
> + *
> + * 1. Destination stripes with skipped_disk = 0 in reshape_request()
> + * - the new stripe maps entirely past the old array end, so its
> + * blocks are zero-filled in place without any source read.
> + * STRIPE_EXPANDING, STRIPE_EXPAND_READY and STRIPE_HANDLE are all set,
> + * handle_stripe() sees them with s.expanded == 1.
> + *
> + * 2. Destination stripes with skipped_disk = 1 - the new stripe
> + * overlaps existing data and still needs source blocks copied in by
> + * handle_stripe_expansion(). Only STRIPE_EXPANDING is set, *not*
> + * STRIPE_HANDLE, so they sit idle in the stripe cache until a successful
> + * source expand re-handles them. In the failure path no one ever does,
> + * so handle_stripe() will never see them on its own; they are cleaned up
> + * from the source side in step (b) below.
> + *
> + * 3. Source stripes (STRIPE_EXPAND_SOURCE) - reach handle_stripe() via the
> + * read-error path once the source members start returning EIO and
> + * raid5_error() marks them Faulty.
> + *
> + * Handling:
> + *
> + * (a) If STRIPE_EXPANDING is set on sh, clear it together with
> + * STRIPE_EXPAND_READY, atomic_dec conf->reshape_stripes, wake
> + * wait_for_reshape and md_done_sync(RAID5_STRIPE_SECTORS) to return
> + * the sectors reshape_request() accounted on recovery_active.
> + *
> + * (b) If STRIPE_EXPAND_SOURCE is set on sh, clear it and walk sh's
> + * non-parity disks the same way handle_stripe_expansion() does
> + * (raid5_compute_blocknr previous=1 -> raid5_compute_sector previous=0)
> + * to find each destination, look it up with
> + * R5_GAS_NOBLOCK | R5_GAS_NOQUIESCE and apply step (a) to it.
> + * A NULL lookup means the destination never contributed to
> + * reshape_stripes - nothing to release.
> + */
> +static void handle_failed_reshape(struct r5conf *conf, struct stripe_head *sh)
> +{
> + int i;
> +
> + if (test_and_clear_bit(STRIPE_EXPANDING, &sh->state)) {
> + atomic_dec(&conf->reshape_stripes);
> + wake_up(&conf->wait_for_reshape);
> + md_done_sync(conf->mddev, RAID5_STRIPE_SECTORS(conf));
> + }
> +
> + clear_bit(STRIPE_EXPAND_READY, &sh->state);
I feel this is not that simple, can you first describe reshape sh lifetime
first? And then we can check in each state the handle is correct, for example:
After setting STRIPE_EXPAND_READY, and new writes submitted, the STRIPE_EXPANDING
is cleared while STRIPE_EXPAND_READY is still set.
Normally if write succeed, the STRIPE_EXPAND_READY will be cleared, also
reshape_stripes and recovery_active counters will be decreased. However,
If the array is broken in this case, and above write failed, then is it possible
the sh will be analysed again and the above checking miss this case.
> +
> + if (test_and_clear_bit(STRIPE_EXPAND_SOURCE, &sh->state)) {
> + for (i = 0; i < sh->disks; i++) {
> + int dd_idx;
> + struct stripe_head *sh2;
> + sector_t bn, sec;
> +
> + if (i == sh->pd_idx)
> + continue;
> + if (conf->level == 6 && i == sh->qd_idx)
> + continue;
> +
> + bn = raid5_compute_blocknr(sh, i, 1);
> + sec = raid5_compute_sector(conf, bn, 0, &dd_idx, NULL);
> + sh2 = raid5_get_active_stripe(conf, NULL, sec,
> + R5_GAS_NOBLOCK | R5_GAS_NOQUIESCE);
> + if (!sh2)
> + continue;
> + if (test_and_clear_bit(STRIPE_EXPANDING, &sh2->state)) {
> + atomic_dec(&conf->reshape_stripes);
> + wake_up(&conf->wait_for_reshape);
> + md_done_sync(conf->mddev,
> + RAID5_STRIPE_SECTORS(conf));
> + }
> + clear_bit(STRIPE_EXPAND_READY, &sh2->state);
> + raid5_release_stripe(sh2);
> + }
> + }
> +}
> +
> static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
> {
> struct r5conf *conf = sh->raid_conf;
> int disks = sh->disks;
> struct r5dev *dev;
> @@ -5001,10 +5087,15 @@ static void handle_stripe(struct stripe_head *sh)
> break_stripe_batch_list(sh, 0);
> if (s.to_read+s.to_write+s.written)
> handle_failed_stripe(conf, sh, &s, disks);
> if (s.syncing + s.replacing)
> handle_failed_sync(conf, sh, &s);
> + if (s.expanding || s.expanded) {
> + handle_failed_reshape(conf, sh);
> + s.expanding = 0;
> + s.expanded = 0;
> + }
> }
>
> /* Now we check to see if any write operations have recently
> * completed
> */
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/3] md/raid1,raid10: fix error-path detection with md_cloned_bio()
From: Yu Kuai @ 2026-05-22 4:23 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, Xiao Ni
Cc: song, xni, neilb, shli, linux-raid, linux-kernel, yukuai
In-Reply-To: <m2pl2pkttv.fsf@gmail.com>
Hi,
在 2026/5/21 17:14, Abd-Alrhman Masalkhi 写道:
> The original bio resubmitted via bio_list is not executing in the raid1d
> thread context, so blocking on is_suspended() is correct and will not
> cause a deadlock. The suspension deadlock risk only exists for
> md_cloned_bio executing in the raid1d thread context in the error path.
> And in this case it is always true that if we are executing in the
> raid1d thread context the bio will already be a md_cloned_bio on entry
> to md_handle_request(), even if current->bio_list was set, because (and as
> @Xiao has mentioned) every time we split a bio in the error path we are
> resubmitting the md_cloned_bio remainder, never the original bio.
I do understand this, if original bio or resubmitted bio only split at
raid1_read_request, there is no problem. What I feel is not safe is that
bio can split somewhere else.
This is possible for raid5, However, take a closer look at raid1/10, I
feel it's not possible for resubmitted io to split in bio_split_to_limits.
Even if it happens, it's probably someone else to blame.
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH 01/19] btrfs: require at least 4 devices for RAID 6
From: Qu Wenruo @ 2026-05-22 0:27 UTC (permalink / raw)
To: Andrew Morton, Qu Wenruo
Cc: Christoph Hellwig, H. Peter Anvin, kreijack, David Sterba,
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: <20260521171730.7872482df453975cf60ce7dc@linux-foundation.org>
在 2026/5/22 09:47, Andrew Morton 写道:
> On Wed, 20 May 2026 18:11:09 +0930 Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>>
>>
>> 在 2026/5/18 14:42, Christoph Hellwig 写道:
>>> On Fri, May 15, 2026 at 12:59:34PM -0700, H. Peter Anvin wrote:
>>>> I don't think this is a good idea. Error out; it is the btrfs maintainers' job to ensure user data isn't lost.
>>>>
>>>> The RAID-6 code has *never* supported only 3 units, and if it ever worked for *any* of the implementations it was purely by accident. Speaking as the original author I should know; this was deliberate as in some cases the degenerate case (3) would have required extra trays in the code to no user benefit.
>>>>
>>>> I would not be surprised if the kernel crashed or corrupted the page cache in that case.
>>>
>>> It does, that's why I wanted to exclude it. Anyway, for the about to be
>>> resent version I'll drop this btrfs patch over the stated objection and
>>> will otherwise not change anything. This means the (IMHO hypothetical)
>>> users of this configuration will get a WARN_ON_ONCE triggered, but
>>> otherwise keep working (or rather not working) as before.
>>>
>>
>> For the btrfs part, I believe I can get the current 2-disk-raid5 and
>> 3-disk-raid6 to fallback to raid1 inside btrfs.
>>
>> I hope the btrfs part can be finished and reach the next merge window,
>> but I'm not 100% sure.
>>
>> What is the planned cycle to merge this raid5/6 cleanup?
>
> At present it's on track for the 7.2-rc1 merge window. Does that suit?
The current btrfs fix (*) is pretty small, I believe we can get it into
the next merge window, as long as we got enough review on it.
*:
https://lore.kernel.org/linux-btrfs/a1d63733465229936351804f3760803d5894a962.1779274630.git.wqu@suse.com/T/#u
^ permalink raw reply
* Re: [PATCH 01/19] btrfs: require at least 4 devices for RAID 6
From: Andrew Morton @ 2026-05-22 0:17 UTC (permalink / raw)
To: Qu Wenruo
Cc: Christoph Hellwig, H. Peter Anvin, kreijack, David Sterba,
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: <f46636c8-80ba-4802-a6a0-74cbc35e7bee@gmx.com>
On Wed, 20 May 2026 18:11:09 +0930 Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
> 在 2026/5/18 14:42, Christoph Hellwig 写道:
> > On Fri, May 15, 2026 at 12:59:34PM -0700, H. Peter Anvin wrote:
> >> I don't think this is a good idea. Error out; it is the btrfs maintainers' job to ensure user data isn't lost.
> >>
> >> The RAID-6 code has *never* supported only 3 units, and if it ever worked for *any* of the implementations it was purely by accident. Speaking as the original author I should know; this was deliberate as in some cases the degenerate case (3) would have required extra trays in the code to no user benefit.
> >>
> >> I would not be surprised if the kernel crashed or corrupted the page cache in that case.
> >
> > It does, that's why I wanted to exclude it. Anyway, for the about to be
> > resent version I'll drop this btrfs patch over the stated objection and
> > will otherwise not change anything. This means the (IMHO hypothetical)
> > users of this configuration will get a WARN_ON_ONCE triggered, but
> > otherwise keep working (or rather not working) as before.
> >
>
> For the btrfs part, I believe I can get the current 2-disk-raid5 and
> 3-disk-raid6 to fallback to raid1 inside btrfs.
>
> I hope the btrfs part can be finished and reach the next merge window,
> but I'm not 100% sure.
>
> What is the planned cycle to merge this raid5/6 cleanup?
At present it's on track for the 7.2-rc1 merge window. Does that suit?
^ permalink raw reply
* Re: [PATCH v2 2/3] md/raid1,raid10: fix error-path detection with md_cloned_bio()
From: Abd-Alrhman Masalkhi @ 2026-05-21 9:14 UTC (permalink / raw)
To: Yu Kuai, Xiao Ni; +Cc: song, xni, neilb, shli, linux-raid, linux-kernel, yukuai
In-Reply-To: <4ff43e8f-6405-4ce8-8cdf-f71e0bce4d3f@fygo.io>
Hi Kuai
On Thu, May 21, 2026 at 15:29 +0800, Yu Kuai wrote:
> Hi,
>
> 在 2026/5/21 15:08, Xiao Ni 写道:
>> On Thu, May 21, 2026 at 9:25 AM Yu Kuai <yukuai@fnnas.com> wrote:
>>> Hi,
>>>
>>> 在 2026/5/1 19:46, Abd-Alrhman Masalkhi 写道:
>>>> Detect the error path using md_cloned_bio() instead of relying
>>>> on r1_bio in raid1 or r10_bio->read_slot in raid10, which may be
>>>> NULL or -1 after splitting and resubmitting a failed bio.
>>>>
>>>> As a result, the error path may not be recognized and memory
>>>> allocations can incorrectly use GFP_NOIO instead of
>>>> (GFP_NOIO | __GFP_HIGH), which can lead to a deadlock under
>>>> memory pressure.
>>> I don't think this patch will fix this problem. Because split bio
>>> is a new bio that will be resubmit by md_submit_bio(), such bio is
>>> not a md_cloned_bio yet until md_account_bio().
>> Hi Kuai
>>
>> bio = bio_submit_split_bioset(bio, max_sectors,
>> &conf->bio_split);
>> bio_submit_split_bioset returns the split bio and the remainder bio
>> will be submitted again which is already a md_cloned_bio.
>
> Okay, I understand now this is a special case that current->bio_list is
> NULL. However, the problem still exist in the case original IO split again
> while resubmitting, and in this case current->bio_list will be set so new
> split io will be resubmit by md_submit_bio(). Which means patch 1 can't
> fix this case as well.
The original bio resubmitted via bio_list is not executing in the raid1d
thread context, so blocking on is_suspended() is correct and will not
cause a deadlock. The suspension deadlock risk only exists for
md_cloned_bio executing in the raid1d thread context in the error path.
And in this case it is always true that if we are executing in the
raid1d thread context the bio will already be a md_cloned_bio on entry
to md_handle_request(), even if current->bio_list was set, because (and as
@Xiao has mentioned) every time we split a bio in the error path we are
resubmitting the md_cloned_bio remainder, never the original bio.
>
> Perhaps it's better to check task_struct instead of bio_set.
>
>>
>> Regards
>> Xiao
>>>> Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
>>>> Fixes: 545250f24809 ("md/raid10: simplify handle_read_error()")
>>>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>>>> ---
>>>> This patch depends on patch 1.
>>>>
>>>> Changes in v2:
>>>> - New patch.
>>>> ---
>>>> drivers/md/raid1.c | 13 ++++++++++---
>>>> drivers/md/raid10.c | 20 ++++++++++++++------
>>>> 2 files changed, 24 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>>>> index cc9914bd15c1..c52ecd38c163 100644
>>>> --- a/drivers/md/raid1.c
>>>> +++ b/drivers/md/raid1.c
>>>> @@ -1321,11 +1321,18 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>>>> bool r1bio_existed = !!r1_bio;
>>>>
>>>> /*
>>>> - * If r1_bio is set, we are blocking the raid1d thread
>>>> - * so there is a tiny risk of deadlock. So ask for
>>>> + * An md cloned bio indicates we are in the error path.
>>>> + * This is more reliable than checking r1_bio, which might
>>>> + * be NULL even in the error path if a failed bio was split.
>>>> + */
>>>> + bool err_path = md_cloned_bio(mddev, bio);
>>>> +
>>>> + /*
>>>> + * If we are in the error path, we are blocking the raid1d
>>>> + * thread so there is a tiny risk of deadlock. So ask for
>>>> * emergency memory if needed.
>>>> */
>>>> - gfp_t gfp = r1_bio ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
>>>> + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
>>>>
>>>> /*
>>>> * Still need barrier for READ in case that whole
>>>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>>>> index 3a591e60a144..8c6fc398260e 100644
>>>> --- a/drivers/md/raid10.c
>>>> +++ b/drivers/md/raid10.c
>>>> @@ -1155,7 +1155,20 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
>>>> char b[BDEVNAME_SIZE];
>>>> int slot = r10_bio->read_slot;
>>>> struct md_rdev *err_rdev = NULL;
>>>> - gfp_t gfp = GFP_NOIO;
>>>> +
>>>> + /*
>>>> + * An md cloned bio indicates we are in the error path.
>>>> + * This is more reliable than checking slot, which might
>>>> + * be -1 even in the error path if a failed bio was split.
>>>> + */
>>>> + bool err_path = md_cloned_bio(mddev, bio);
>>>> +
>>>> + /*
>>>> + * If we are in the error path, we are blocking the raid10d
>>>> + * thread so there is a tiny risk of deadlock. So ask for
>>>> + * emergency memory if needed.
>>>> + */
>>>> + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
>>>>
>>>> if (slot >= 0 && r10_bio->devs[slot].rdev) {
>>>> /*
>>>> @@ -1166,11 +1179,6 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
>>>> * we lose the device name in error messages.
>>>> */
>>>> int disk;
>>>> - /*
>>>> - * As we are blocking raid10, it is a little safer to
>>>> - * use __GFP_HIGH.
>>>> - */
>>>> - gfp = GFP_NOIO | __GFP_HIGH;
>>>>
>>>> disk = r10_bio->devs[slot].devnum;
>>>> err_rdev = conf->mirrors[disk].rdev;
>>> --
>>> Thansk,
>>> Kuai
>>>
> --
> Thansk,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v2 2/3] md/raid1,raid10: fix error-path detection with md_cloned_bio()
From: Yu Kuai @ 2026-05-21 7:29 UTC (permalink / raw)
To: Xiao Ni
Cc: Abd-Alrhman Masalkhi, song, xni, neilb, shli, linux-raid,
linux-kernel, yukuai
In-Reply-To: <CAF-RUYVK34e4ShHLd4nYGBdB3yxX4BBkS-woxBnmCuj_6vUisg@mail.gmail.com>
Hi,
在 2026/5/21 15:08, Xiao Ni 写道:
> On Thu, May 21, 2026 at 9:25 AM Yu Kuai <yukuai@fnnas.com> wrote:
>> Hi,
>>
>> 在 2026/5/1 19:46, Abd-Alrhman Masalkhi 写道:
>>> Detect the error path using md_cloned_bio() instead of relying
>>> on r1_bio in raid1 or r10_bio->read_slot in raid10, which may be
>>> NULL or -1 after splitting and resubmitting a failed bio.
>>>
>>> As a result, the error path may not be recognized and memory
>>> allocations can incorrectly use GFP_NOIO instead of
>>> (GFP_NOIO | __GFP_HIGH), which can lead to a deadlock under
>>> memory pressure.
>> I don't think this patch will fix this problem. Because split bio
>> is a new bio that will be resubmit by md_submit_bio(), such bio is
>> not a md_cloned_bio yet until md_account_bio().
> Hi Kuai
>
> bio = bio_submit_split_bioset(bio, max_sectors,
> &conf->bio_split);
> bio_submit_split_bioset returns the split bio and the remainder bio
> will be submitted again which is already a md_cloned_bio.
Okay, I understand now this is a special case that current->bio_list is
NULL. However, the problem still exist in the case original IO split again
while resubmitting, and in this case current->bio_list will be set so new
split io will be resubmit by md_submit_bio(). Which means patch 1 can't
fix this case as well.
Perhaps it's better to check task_struct instead of bio_set.
>
> Regards
> Xiao
>>> Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
>>> Fixes: 545250f24809 ("md/raid10: simplify handle_read_error()")
>>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>>> ---
>>> This patch depends on patch 1.
>>>
>>> Changes in v2:
>>> - New patch.
>>> ---
>>> drivers/md/raid1.c | 13 ++++++++++---
>>> drivers/md/raid10.c | 20 ++++++++++++++------
>>> 2 files changed, 24 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>>> index cc9914bd15c1..c52ecd38c163 100644
>>> --- a/drivers/md/raid1.c
>>> +++ b/drivers/md/raid1.c
>>> @@ -1321,11 +1321,18 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>>> bool r1bio_existed = !!r1_bio;
>>>
>>> /*
>>> - * If r1_bio is set, we are blocking the raid1d thread
>>> - * so there is a tiny risk of deadlock. So ask for
>>> + * An md cloned bio indicates we are in the error path.
>>> + * This is more reliable than checking r1_bio, which might
>>> + * be NULL even in the error path if a failed bio was split.
>>> + */
>>> + bool err_path = md_cloned_bio(mddev, bio);
>>> +
>>> + /*
>>> + * If we are in the error path, we are blocking the raid1d
>>> + * thread so there is a tiny risk of deadlock. So ask for
>>> * emergency memory if needed.
>>> */
>>> - gfp_t gfp = r1_bio ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
>>> + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
>>>
>>> /*
>>> * Still need barrier for READ in case that whole
>>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>>> index 3a591e60a144..8c6fc398260e 100644
>>> --- a/drivers/md/raid10.c
>>> +++ b/drivers/md/raid10.c
>>> @@ -1155,7 +1155,20 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
>>> char b[BDEVNAME_SIZE];
>>> int slot = r10_bio->read_slot;
>>> struct md_rdev *err_rdev = NULL;
>>> - gfp_t gfp = GFP_NOIO;
>>> +
>>> + /*
>>> + * An md cloned bio indicates we are in the error path.
>>> + * This is more reliable than checking slot, which might
>>> + * be -1 even in the error path if a failed bio was split.
>>> + */
>>> + bool err_path = md_cloned_bio(mddev, bio);
>>> +
>>> + /*
>>> + * If we are in the error path, we are blocking the raid10d
>>> + * thread so there is a tiny risk of deadlock. So ask for
>>> + * emergency memory if needed.
>>> + */
>>> + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
>>>
>>> if (slot >= 0 && r10_bio->devs[slot].rdev) {
>>> /*
>>> @@ -1166,11 +1179,6 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
>>> * we lose the device name in error messages.
>>> */
>>> int disk;
>>> - /*
>>> - * As we are blocking raid10, it is a little safer to
>>> - * use __GFP_HIGH.
>>> - */
>>> - gfp = GFP_NOIO | __GFP_HIGH;
>>>
>>> disk = r10_bio->devs[slot].devnum;
>>> err_rdev = conf->mirrors[disk].rdev;
>> --
>> Thansk,
>> Kuai
>>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/3] md/raid1,raid10: fix error-path detection with md_cloned_bio()
From: Xiao Ni @ 2026-05-21 7:08 UTC (permalink / raw)
To: yukuai
Cc: Abd-Alrhman Masalkhi, song, xni, neilb, shli, linux-raid,
linux-kernel
In-Reply-To: <71990de4-1113-4b7d-8a50-ca1d82802a03@fygo.io>
On Thu, May 21, 2026 at 9:25 AM Yu Kuai <yukuai@fnnas.com> wrote:
>
> Hi,
>
> 在 2026/5/1 19:46, Abd-Alrhman Masalkhi 写道:
> > Detect the error path using md_cloned_bio() instead of relying
> > on r1_bio in raid1 or r10_bio->read_slot in raid10, which may be
> > NULL or -1 after splitting and resubmitting a failed bio.
> >
> > As a result, the error path may not be recognized and memory
> > allocations can incorrectly use GFP_NOIO instead of
> > (GFP_NOIO | __GFP_HIGH), which can lead to a deadlock under
> > memory pressure.
>
> I don't think this patch will fix this problem. Because split bio
> is a new bio that will be resubmit by md_submit_bio(), such bio is
> not a md_cloned_bio yet until md_account_bio().
Hi Kuai
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
bio_submit_split_bioset returns the split bio and the remainder bio
will be submitted again which is already a md_cloned_bio.
Regards
Xiao
>
> >
> > Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
> > Fixes: 545250f24809 ("md/raid10: simplify handle_read_error()")
> > Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> > ---
> > This patch depends on patch 1.
> >
> > Changes in v2:
> > - New patch.
> > ---
> > drivers/md/raid1.c | 13 ++++++++++---
> > drivers/md/raid10.c | 20 ++++++++++++++------
> > 2 files changed, 24 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > index cc9914bd15c1..c52ecd38c163 100644
> > --- a/drivers/md/raid1.c
> > +++ b/drivers/md/raid1.c
> > @@ -1321,11 +1321,18 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> > bool r1bio_existed = !!r1_bio;
> >
> > /*
> > - * If r1_bio is set, we are blocking the raid1d thread
> > - * so there is a tiny risk of deadlock. So ask for
> > + * An md cloned bio indicates we are in the error path.
> > + * This is more reliable than checking r1_bio, which might
> > + * be NULL even in the error path if a failed bio was split.
> > + */
> > + bool err_path = md_cloned_bio(mddev, bio);
> > +
> > + /*
> > + * If we are in the error path, we are blocking the raid1d
> > + * thread so there is a tiny risk of deadlock. So ask for
> > * emergency memory if needed.
> > */
> > - gfp_t gfp = r1_bio ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
> > + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
> >
> > /*
> > * Still need barrier for READ in case that whole
> > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> > index 3a591e60a144..8c6fc398260e 100644
> > --- a/drivers/md/raid10.c
> > +++ b/drivers/md/raid10.c
> > @@ -1155,7 +1155,20 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> > char b[BDEVNAME_SIZE];
> > int slot = r10_bio->read_slot;
> > struct md_rdev *err_rdev = NULL;
> > - gfp_t gfp = GFP_NOIO;
> > +
> > + /*
> > + * An md cloned bio indicates we are in the error path.
> > + * This is more reliable than checking slot, which might
> > + * be -1 even in the error path if a failed bio was split.
> > + */
> > + bool err_path = md_cloned_bio(mddev, bio);
> > +
> > + /*
> > + * If we are in the error path, we are blocking the raid10d
> > + * thread so there is a tiny risk of deadlock. So ask for
> > + * emergency memory if needed.
> > + */
> > + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
> >
> > if (slot >= 0 && r10_bio->devs[slot].rdev) {
> > /*
> > @@ -1166,11 +1179,6 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> > * we lose the device name in error messages.
> > */
> > int disk;
> > - /*
> > - * As we are blocking raid10, it is a little safer to
> > - * use __GFP_HIGH.
> > - */
> > - gfp = GFP_NOIO | __GFP_HIGH;
> >
> > disk = r10_bio->devs[slot].devnum;
> > err_rdev = conf->mirrors[disk].rdev;
>
> --
> Thansk,
> Kuai
>
^ permalink raw reply
* Re: [PATCH] raid10_handle_discard() reuses r10bio objects from r10bio_pool.
From: Yu Kuai @ 2026-05-21 5:32 UTC (permalink / raw)
To: Chen Cheng, yukuai; +Cc: linux-raid, linux-kernel
In-Reply-To: <20260515093019.3436882-1-chencheng@fnnas.com>
在 2026/5/15 17:30, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> put_all_bios() always drops devs[i].bio, but it only drops
> devs[i].repl_bio when r10_bio->read_slot < 0. If discard reuses an
> r10bio that was previously used for a read, read_slot can still be
> non-negative, and discard cleanup can skip bio_put() on repl_bio.
>
> Reset read_slot to -1 when preparing an r10bio for discard so the
> replacement bio is always released correctly.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid10.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 39085e7dd6d2..7dc2a5a127e8 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1727,6 +1727,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
> r10_bio->mddev = mddev;
> r10_bio->state = 0;
> r10_bio->sectors = 0;
> + r10_bio->read_slot = -1;
> memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
> wait_blocked_dev(mddev, r10_bio);
>
Applied with the subject fix:
md/raid10: reset read_slot when reusing r10bio for discard
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v2 1/2] md/raid10: make r10bio_pool use fixed-size objects
From: Yu Kuai @ 2026-05-21 5:18 UTC (permalink / raw)
To: Chen Cheng, yukuai; +Cc: linux-raid, linux-kernel
In-Reply-To: <20260515092707.3436464-2-chencheng@fnnas.com>
Hi,
在 2026/5/15 17:27, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> raid10 currently allocates r10bio_pool objects with conf->geo.raid_disks,
> which makes regular r10bio objects geometry-dependent.
>
> That model breaks down across reshape. mempool objects are preallocated and
> reused, so a reshape that changes the number of raid disks can leave old
> r10bio objects in the regular I/O pool with a devs[] array sized for the
> previous geometry. After the geometry switch, those stale objects may be
> reused or later freed under the new layout, creating a width mismatch
> between the reused r10bio and the current array geometry.
>
> For example, during a 4-disk to 5-disk reshape, an r10bio allocated before
> the geometry switch has room for only 4 devs[] entries. After reshape
> updates conf->geo.raid_disks to 5, that stale object can be reused under
> the new geometry. Code such as __make_request(), put_all_bios(), and
> find_bio_disk() may then access devs[] using the new geometry and step
> past the end of the old 4-slot object, leading to slab out-of-bounds
> accesses.
>
> The root problem is that regular r10bio pool objects are geometry-dependent,
> while mempool elements are preallocated and reused across requests.
>
> Switch r10bio_pool to a fixed-size kmalloc mempool so regular I/O objects no
> longer carry an allocation width tied to the current geometry. Use the same
> fixed-size allocation rule for the standalone r10bio allocated from
> r10buf_pool_alloc().
>
> Because reshape updates live array state such as conf->mirrors, conf->geo,
> reshape_progress, and reshape_safe, the geometry switch must happen only
> after normal I/O has gone fully quiet. raise_barrier() alone is not strong
> enough here: freeze_array() also marks that an array freeze is in progress,
> flushes pending writes, and waits until in-flight I/O has either completed
> or been queued.
>
> Freeze the array before switching reshape geometry, rebuild r10bio_pool for
> the new width inside that freeze window, and switch raid10_quiesce() to use
> freeze_array()/unfreeze_array() as well. This keeps new requests from reusing
> stale-width regular I/O objects after the geometry change.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid10.c | 57 +++++++++++++++++++++++++++++++++------------
> drivers/md/raid10.h | 2 +-
> 2 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 39085e7dd6d2..886bbe6b1ebc 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -103,13 +103,28 @@ static inline struct r10bio *get_resync_r10bio(struct bio *bio)
> return get_resync_pages(bio)->raid_bio;
> }
>
> -static void * r10bio_pool_alloc(gfp_t gfp_flags, void *data)
> +static inline unsigned int calc_r10bio_pool_disks(struct mddev *mddev)
> {
> - struct r10conf *conf = data;
> - int size = offsetof(struct r10bio, devs[conf->geo.raid_disks]);
> + /* If delta_disks < 0, use bigger r10bio->devs[] is ok. */
> + return mddev->raid_disks + max(0, mddev->delta_disks);
> +}
> +
> +static inline int calc_r10bio_size(struct mddev *mddev)
> +{
> + return offsetof(struct r10bio, devs[calc_r10bio_pool_disks(mddev)]);
> +}
> +
> +static mempool_t *create_r10bio_pool(struct mddev *mddev)
> +{
> + int size = calc_r10bio_size(mddev);
> +
> + return mempool_create_kmalloc_pool(NR_RAID_BIOS, size);
> +}
> +
> +static struct r10bio *alloc_r10bio(struct mddev *mddev, gfp_t gfp_flags)
> +{
> + int size = calc_r10bio_size(mddev);
>
> - /* allocate a r10bio with room for raid_disks entries in the
> - * bios array */
> return kzalloc(size, gfp_flags);
> }
>
> @@ -137,7 +152,7 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> int nalloc, nalloc_rp;
> struct resync_pages *rps;
>
> - r10_bio = r10bio_pool_alloc(gfp_flags, conf);
> + r10_bio = alloc_r10bio(conf->mddev, gfp_flags);
> if (!r10_bio)
> return NULL;
>
> @@ -277,7 +292,7 @@ static void free_r10bio(struct r10bio *r10_bio)
> struct r10conf *conf = r10_bio->mddev->private;
>
> put_all_bios(conf, r10_bio);
> - mempool_free(r10_bio, &conf->r10bio_pool);
> + mempool_free(r10_bio, conf->r10bio_pool);
> }
>
> static void put_buf(struct r10bio *r10_bio)
> @@ -1531,7 +1546,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
> struct r10conf *conf = mddev->private;
> struct r10bio *r10_bio;
>
> - r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
> + r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
>
> r10_bio->master_bio = bio;
> r10_bio->sectors = sectors;
> @@ -1723,7 +1738,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
> (last_stripe_index << geo->chunk_shift);
>
> retry_discard:
> - r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
> + r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
> r10_bio->mddev = mddev;
> r10_bio->state = 0;
> r10_bio->sectors = 0;
> @@ -3823,7 +3838,7 @@ static void raid10_free_conf(struct r10conf *conf)
> if (!conf)
> return;
>
> - mempool_exit(&conf->r10bio_pool);
> + mempool_destroy(conf->r10bio_pool);
> kfree(conf->mirrors);
> kfree(conf->mirrors_old);
> kfree(conf->mirrors_new);
> @@ -3870,9 +3885,8 @@ static struct r10conf *setup_conf(struct mddev *mddev)
>
> conf->geo = geo;
> conf->copies = copies;
> - err = mempool_init(&conf->r10bio_pool, NR_RAID_BIOS, r10bio_pool_alloc,
> - rbio_pool_free, conf);
> - if (err)
> + conf->r10bio_pool = create_r10bio_pool(mddev);
> + if (!conf->r10bio_pool)
> goto out;
>
> err = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
> @@ -4131,9 +4145,9 @@ static void raid10_quiesce(struct mddev *mddev, int quiesce)
> struct r10conf *conf = mddev->private;
>
> if (quiesce)
> - raise_barrier(conf, 0);
> + freeze_array(conf, 0);
> else
> - lower_barrier(conf);
> + unfreeze_array(conf);
> }
>
> static int raid10_resize(struct mddev *mddev, sector_t sectors)
> @@ -4365,6 +4379,7 @@ static int raid10_start_reshape(struct mddev *mddev)
> struct md_rdev *rdev;
> int spares = 0;
> int ret;
> + mempool_t *new_pool;
>
> if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
> return -EBUSY;
> @@ -4400,7 +4415,17 @@ static int raid10_start_reshape(struct mddev *mddev)
> if (spares < mddev->delta_disks)
> return -EINVAL;
>
> + freeze_array(conf, 0);
freeze array here is incorrect. see the comments of freeze_array:
wait for pending IO requests to coplete or be rqueued for re-try.
Which means there can still be IO pending for re-try.
> conf->offset_diff = min_offset_diff;
> + if (mddev->delta_disks > 0) {
> + new_pool = create_r10bio_pool(mddev);
> + if (!new_pool) {
> + unfreeze_array(conf);
> + return -ENOMEM;
> + }
> + mempool_destroy(conf->r10bio_pool);
> + conf->r10bio_pool = new_pool;
> + }
> spin_lock_irq(&conf->device_lock);
> if (conf->mirrors_new) {
> memcpy(conf->mirrors_new, conf->mirrors,
> @@ -4417,6 +4442,7 @@ static int raid10_start_reshape(struct mddev *mddev)
> sector_t size = raid10_size(mddev, 0, 0);
> if (size < mddev->array_sectors) {
> spin_unlock_irq(&conf->device_lock);
> + unfreeze_array(conf);
> pr_warn("md/raid10:%s: array size must be reduce before number of disks\n",
> mdname(mddev));
> return -EINVAL;
> @@ -4427,6 +4453,7 @@ static int raid10_start_reshape(struct mddev *mddev)
> conf->reshape_progress = 0;
> conf->reshape_safe = conf->reshape_progress;
> spin_unlock_irq(&conf->device_lock);
> + unfreeze_array(conf);
>
> if (mddev->delta_disks && mddev->bitmap) {
> struct mdp_superblock_1 *sb = NULL;
> diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
> index ec79d87fb92f..b711626a5db7 100644
> --- a/drivers/md/raid10.h
> +++ b/drivers/md/raid10.h
> @@ -87,7 +87,7 @@ struct r10conf {
> */
> wait_queue_head_t wait_barrier;
>
> - mempool_t r10bio_pool;
> + mempool_t *r10bio_pool;
> mempool_t r10buf_pool;
> struct page *tmppage;
> struct bio_set bio_split;
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] raid10_handle_discard() reuses r10bio objects from r10bio_pool.
From: Xiao Ni @ 2026-05-21 2:10 UTC (permalink / raw)
To: Chen Cheng; +Cc: Yu Kuai, linux-raid, linux-kernel
In-Reply-To: <20260515093019.3436882-1-chencheng@fnnas.com>
On Fri, May 15, 2026 at 6:11 PM Chen Cheng <chencheng@fnnas.com> wrote:
>
> From: Chen Cheng <chencheng@fnnas.com>
>
> put_all_bios() always drops devs[i].bio, but it only drops
> devs[i].repl_bio when r10_bio->read_slot < 0. If discard reuses an
> r10bio that was previously used for a read, read_slot can still be
> non-negative, and discard cleanup can skip bio_put() on repl_bio.
>
> Reset read_slot to -1 when preparing an r10bio for discard so the
> replacement bio is always released correctly.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid10.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 39085e7dd6d2..7dc2a5a127e8 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1727,6 +1727,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
> r10_bio->mddev = mddev;
> r10_bio->state = 0;
> r10_bio->sectors = 0;
> + r10_bio->read_slot = -1;
> memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
> wait_blocked_dev(mddev, r10_bio);
>
> --
> 2.54.0
>
Hi Chen
The title usually uses a format such as md/raid10: xxxx
This patch looks good to me.
Reviewed-by: Xiao Ni <xiao@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/3] md/raid1,raid10: fix deadlock in read error recovery path
From: Yu Kuai @ 2026-05-21 1:28 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, xni, neilb, shli
Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <20260501114652.590037-2-abd.masalkhi@gmail.com>
在 2026/5/1 19:46, Abd-Alrhman Masalkhi 写道:
> raid1d and raid10d may resubmit a split md cloned bio while handling
> a read error. In this case, resubmitting the bio can lead to a deadlock
> if the array is suspended before md_handle_request() acquires an
> active_io reference via percpu_ref_tryget_live().
>
> Since the cloned bio already holds an active_io reference,
> trying to acquire another reference via percpu_ref_tryget_live()
> can lead to a deadlock while the array is suspended.
>
> Fix this by using percpu_ref_get() for md cloned bios.
>
> Fixes: bb2a9acefaf9 ("md/raid1: switch to use md_account_bio() for io accounting")
> Fixes: 820455238366 ("md/raid10: switch to use md_account_bio() for io accounting")
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
> ---
> Changes in v2:
> - Use md_cloned_bio() consistently to detect cloned bios.
> - Recognize that raid10 has the same issue and fix it in this series
> - Allow splitting bios.
> - Handle md cloned bios explicitly in md_handle_request()
> - Link v1:https://lore.kernel.org/linux-raid/20260427103446.300378-1-abd.masalkhi@gmail.com/
>
> Please let me know if I should add a Suggested-by tag for Yu Kuai,
> as the solution approach was suggested during review.
>
> Link to Yu Kuai' email:https://lore.kernel.org/linux-raid/m2lde74dtw.fsf@gmail.com/T/#m714020a38b60fc5f84b9a24f0c46acbe5d7342d6
>
> Thanks
> Abd-alrhman
> ---
> drivers/md/md.c | 25 ++++++++++++++++---------
> drivers/md/md.h | 5 +++++
> 2 files changed, 21 insertions(+), 9 deletions(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v2 3/3] md/raid1,raid10: fix bio accounting for split md cloned bios
From: Yu Kuai @ 2026-05-21 1:26 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, xni, neilb, shli
Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <20260501114652.590037-4-abd.masalkhi@gmail.com>
Hi,
在 2026/5/1 19:46, Abd-Alrhman Masalkhi 写道:
> Use md_cloned_bio() to control bio accounting instead of relying
> on r1bio_existed in raid1 or the io_accounting flag in raid10.
>
> The previous logic does not reliably reflect whether a bio is an
> md cloned bio. When a failed bio is split and resubmitted via
> bio_submit_split_bioset() on the error path, this can lead to either
> double accounting for md cloned bios, or missing accounting for bios
> returned from bio_submit_split_bioset()
>
> Fix this by using md_cloned_bio() to detect md cloned bios and
> skip accounting accordingly.
>
> Fixes: bb2a9acefaf9 ("md/raid1: switch to use md_account_bio() for io accounting")
> Fixes: 820455238366 ("md/raid10: switch to use md_account_bio() for io accounting")
The same as patch 2, I don't think this patch fix any problem. And I
think current IO accounting is correct.
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> This patch depends on patch 1.
>
> Changes in v2:
> - New patch.
> ---
> drivers/md/raid1.c | 2 +-
> drivers/md/raid10.c | 8 ++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index c52ecd38c163..dfaf34141325 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1396,7 +1396,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> }
>
> r1_bio->read_disk = rdisk;
> - if (!r1bio_existed) {
> + if (likely(!md_cloned_bio(mddev, bio))) {
> md_account_bio(mddev, &bio);
> r1_bio->master_bio = bio;
> }
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 8c6fc398260e..93af7bbc9005 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1146,7 +1146,7 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
> }
>
> static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> - struct r10bio *r10_bio, bool io_accounting)
> + struct r10bio *r10_bio)
> {
> struct r10conf *conf = mddev->private;
> struct bio *read_bio;
> @@ -1226,7 +1226,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> }
> slot = r10_bio->read_slot;
>
> - if (io_accounting) {
> + if (likely(!md_cloned_bio(mddev, bio))) {
> md_account_bio(mddev, &bio);
> r10_bio->master_bio = bio;
> }
> @@ -1552,7 +1552,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
> conf->geo.raid_disks);
>
> if (bio_data_dir(bio) == READ)
> - raid10_read_request(mddev, bio, r10_bio, true);
> + raid10_read_request(mddev, bio, r10_bio);
> else
> raid10_write_request(mddev, bio, r10_bio);
> }
> @@ -2872,7 +2872,7 @@ 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, false);
> + raid10_read_request(mddev, r10_bio->master_bio, r10_bio);
> /*
> * allow_barrier after re-submit to ensure no sync io
> * can be issued while regular io pending.
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/3] md/raid1,raid10: fix error-path detection with md_cloned_bio()
From: Yu Kuai @ 2026-05-21 1:24 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, xni, neilb, shli
Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <20260501114652.590037-3-abd.masalkhi@gmail.com>
Hi,
在 2026/5/1 19:46, Abd-Alrhman Masalkhi 写道:
> Detect the error path using md_cloned_bio() instead of relying
> on r1_bio in raid1 or r10_bio->read_slot in raid10, which may be
> NULL or -1 after splitting and resubmitting a failed bio.
>
> As a result, the error path may not be recognized and memory
> allocations can incorrectly use GFP_NOIO instead of
> (GFP_NOIO | __GFP_HIGH), which can lead to a deadlock under
> memory pressure.
I don't think this patch will fix this problem. Because split bio
is a new bio that will be resubmit by md_submit_bio(), such bio is
not a md_cloned_bio yet until md_account_bio().
>
> Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
> Fixes: 545250f24809 ("md/raid10: simplify handle_read_error()")
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> This patch depends on patch 1.
>
> Changes in v2:
> - New patch.
> ---
> drivers/md/raid1.c | 13 ++++++++++---
> drivers/md/raid10.c | 20 ++++++++++++++------
> 2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index cc9914bd15c1..c52ecd38c163 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1321,11 +1321,18 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> bool r1bio_existed = !!r1_bio;
>
> /*
> - * If r1_bio is set, we are blocking the raid1d thread
> - * so there is a tiny risk of deadlock. So ask for
> + * An md cloned bio indicates we are in the error path.
> + * This is more reliable than checking r1_bio, which might
> + * be NULL even in the error path if a failed bio was split.
> + */
> + bool err_path = md_cloned_bio(mddev, bio);
> +
> + /*
> + * If we are in the error path, we are blocking the raid1d
> + * thread so there is a tiny risk of deadlock. So ask for
> * emergency memory if needed.
> */
> - gfp_t gfp = r1_bio ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
> + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
>
> /*
> * Still need barrier for READ in case that whole
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 3a591e60a144..8c6fc398260e 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1155,7 +1155,20 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> char b[BDEVNAME_SIZE];
> int slot = r10_bio->read_slot;
> struct md_rdev *err_rdev = NULL;
> - gfp_t gfp = GFP_NOIO;
> +
> + /*
> + * An md cloned bio indicates we are in the error path.
> + * This is more reliable than checking slot, which might
> + * be -1 even in the error path if a failed bio was split.
> + */
> + bool err_path = md_cloned_bio(mddev, bio);
> +
> + /*
> + * If we are in the error path, we are blocking the raid10d
> + * thread so there is a tiny risk of deadlock. So ask for
> + * emergency memory if needed.
> + */
> + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
>
> if (slot >= 0 && r10_bio->devs[slot].rdev) {
> /*
> @@ -1166,11 +1179,6 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> * we lose the device name in error messages.
> */
> int disk;
> - /*
> - * As we are blocking raid10, it is a little safer to
> - * use __GFP_HIGH.
> - */
> - gfp = GFP_NOIO | __GFP_HIGH;
>
> disk = r10_bio->devs[slot].devnum;
> err_rdev = conf->mirrors[disk].rdev;
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid10: fix divide-by-zero in setup_geo() with zero far_copies
From: Yuhao Jiang @ 2026-05-20 16:43 UTC (permalink / raw)
To: yukuai
Cc: yukuai, Junrui Luo, Song Liu, Li Nan, NeilBrown, Jonathan Brassow,
linux-raid, linux-kernel, stable
In-Reply-To: <6224b47c-9a7e-4bbf-90ce-4b98691ceaa3@fygo.com>
Hi Kuai,
So in this case, how should my name be shown on this security patch?
Because I reported this bug. Most maintainers added my name in the
reported-by tag.
Thanks,
Yuhao
On Wed, May 20, 2026 at 6:52 AM Yu Kuai <yukuai@fnnas.com> wrote:
>
> Hi,
>
> 在 2026/4/28 16:37, Yuhao Jiang 写道:
> > Hi Kuai,
> >
> > Looks like different maintainers have different rules. :(
> > Can you send me the patchwork resource?
>
> Usually just a link to lore url is enough.
>
> >
> > Thanks.
> >
> > On Tue, Apr 28, 2026 at 4:32 PM Yu Kuai <yukuai@fnnas.com> wrote:
> >> Hi,
> >>
> >> 在 2026/4/19 13:59, Yuhao Jiang 写道:
> >>> Hi Kuai,
> >>>
> >>> This report was reported by me, so Junrui added me as Reported-by.
> >> This is fine, however, please do not add downstream reported-by tag.
> >> If you want to add the reported-by tag, please report the problem to
> >> patchwork first. :)
> >>
> >>> Thanks,
> >>>
> >>> On Sun, Apr 19, 2026 at 12:43 AM Yu Kuai <yukuai@fnnas.com> wrote:
> >>>
> >>> Hi,
> >>>
> >>> 在 2026/4/16 11:39, Junrui Luo 写道:
> >>> > setup_geo() extracts near_copies (nc) and far_copies (fc) from the
> >>> > user-provided layout parameter without checking for zero. When fc=0
> >>> > with the "improved" far set layout selected, 'geo->far_set_size =
> >>> > disks / fc' triggers a divide-by-zero.
> >>> >
> >>> > Validate nc and fc immediately after extraction, returning -1 if
> >>> > either is zero.
> >>> >
> >>> > Fixes: 475901aff158 ("MD RAID10: Improve redundancy for 'far'
> >>> and 'offset' algorithms (part 1)")
> >>> > Reported-by: Yuhao Jiang<danisjiang@gmail.com>
> >>>
> >>> So again I can't find a report, and Reported-by usually should be
> >>> followed
> >>> by a Closes link to the original report.
> >>>
> >>> Applied with Reported-by tag removed.
> >>>
> >>> > Cc:stable@vger.kernel.org <mailto:Cc%3Astable@vger.kernel.org>
> >>> > Signed-off-by: Junrui Luo<moonafterrain@outlook.com>
> >>> > ---
> >>> > drivers/md/raid10.c | 2 ++
> >>> > 1 file changed, 2 insertions(+)
> >>>
> >>> --
> >>> Thansk,
> >>> Kuai
> >>>
> >>>
> >>>
> >>> --
> >>> Yuhao Jiang
> >> --
> >> Thansk,
> >> Kuai
> >
> >
> --
> Thansk,
> Kuai
--
Yuhao Jiang
^ permalink raw reply
* Re: [PATCH 2/2] lib/raid6: use kmalloc() in raid6_select_algo()
From: David Laight @ 2026-05-20 13:06 UTC (permalink / raw)
To: Mike Rapoport (Microsoft)
Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, linux-kernel, linux-mm,
linux-raid
In-Reply-To: <20260520-lib-v1-2-cb3045bef2d8@kernel.org>
On Wed, 20 May 2026 11:17:52 +0300
"Mike Rapoport (Microsoft)" <rppt@kernel.org> wrote:
> raid6_select_algo() allocates an order 3 (8 pages) buffer that is used
> as a scratch area for selection of the best algorithm.
Should this code really be using a 4k buffer rather than a PAGE_SIZE one?
-- David
>
> For such large allocations kmalloc() would fall back to alloc_pages() but
> still kmalloc() is a better API as it does not require unnecessary
> castings and may provide more debugging possibilities.
>
> Replace __get_free_pages() call with kmalloc().
>
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
> lib/raid6/algos.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c
> index 799e0e5eac26..89e627c62e30 100644
> --- a/lib/raid6/algos.c
> +++ b/lib/raid6/algos.c
> @@ -12,6 +12,7 @@
> */
>
> #include <linux/raid/pq.h>
> +#include <linux/slab.h>
> #ifndef __KERNEL__
> #include <sys/mman.h>
> #include <stdio.h>
> @@ -129,7 +130,6 @@ const struct raid6_recov_calls *const raid6_recov_algos[] = {
> #endif
>
> #define RAID6_TEST_DISKS 8
> -#define RAID6_TEST_DISKS_ORDER 3
>
> static inline const struct raid6_recov_calls *raid6_choose_recov(void)
> {
> @@ -250,7 +250,7 @@ int __init raid6_select_algo(void)
> int i, cycle;
>
> /* prepare the buffer and fill it circularly with gfmul table */
> - disk_ptr = (char *)__get_free_pages(GFP_KERNEL, RAID6_TEST_DISKS_ORDER);
> + disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
> if (!disk_ptr) {
> pr_err("raid6: Yikes! No memory available.\n");
> return -ENOMEM;
> @@ -275,7 +275,7 @@ int __init raid6_select_algo(void)
> /* select raid recover functions */
> rec_best = raid6_choose_recov();
>
> - free_pages((unsigned long)disk_ptr, RAID6_TEST_DISKS_ORDER);
> + kfree(disk_ptr);
>
> return gen_best && rec_best ? 0 : -EINVAL;
> }
>
^ 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