* Re: [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: Song Chen @ 2026-04-26 14:14 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, pmladek, joe.lawrence, rostedt, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260420144429.57b45f2beece690bceea96ec@kernel.org>
Hi Hiramatsu san,
On 4/20/26 13:44, Masami Hiramatsu (Google) wrote:
> Hi Song,
>
> On Wed, 15 Apr 2026 15:01:37 +0800
> chensong_2000@189.cn wrote:
>
>> From: Song Chen <chensong_2000@189.cn>
>>
>> The current notifier chain implementation uses a single-linked list
>> (struct notifier_block *next), which only supports forward traversal
>> in priority order. This makes it difficult to handle cleanup/teardown
>> scenarios that require notifiers to be called in reverse priority order.
>
> What about introducing a new notification callback API that allows you
> to describe dependencies between callback functions?
>
> For example, when registering a callback, you could register a string
> as an ID and specify whether to call it before or after that ID,
> or you could register a comparison function that is called when adding
> to a list. (I prefer @name and @depends fields so that it can be easily
> maintained.)
>
> This would allow for better dependency building when adding to the list.
>
Is the new notification callback API going to replace
blocking_notifier_chain in module loader? or an expansion inside
blocking_notifier_chain but introducing less complexity?
>>
>> A concrete example is the ordering dependency between ftrace and
>> livepatch during module load/unload. see the detail here [1].
>
> If this only concerns notification callback issues with the ftrace
> and livepatch modules, it's far more robust to simply call the
> necessary processing directly when the modules load and unload,
> rather than registering notification callbacks externally.
>
> There are fprobe, kprobe and its trace-events, all of them are using
> ftrace as its fundation layer. In this case, I always needs to
> consider callback order when a module is unloaded.
>
> If ftrace is working as a part of module callbacks, it will conflict
> with fprobe/kprobe module callback. Of course we can reorder it with
> modifying its priority. But this is ugly, because when we introduce
> a new other feature which depends on another layer, we need to
> reorder the callback's priority number on the list.
>
> Based on the above, I don't think this can be resolved simply by
> changing the list of notification callbacks to a bidirectional list.
>
> Thank you,
>
understood, many thanks for your proposal, i will think about it.
best regards,
Song
^ permalink raw reply
* Re: [RFC PATCH 2/2] kernel/module: Decouple klp and ftrace from load_module
From: Song Chen @ 2026-04-26 14:26 UTC (permalink / raw)
To: Masami Hiramatsu (Google), Petr Mladek
Cc: Petr Pavlu, rafael, lenb, mturquette, sboyd, viresh.kumar, agk,
snitzer, mpatocka, bmarzins, song, yukuai, linan122, jason.wessel,
danielt, dianders, horms, davem, edumazet, kuba, pabeni, paulmck,
frederic, mcgrof, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, joe.lawrence, rostedt, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260420112707.aa3627ca9f975eeaf7d8ea0e@kernel.org>
Hi,
On 4/20/26 10:27, Masami Hiramatsu (Google) wrote:
> On Thu, 16 Apr 2026 16:49:32 +0200
> Petr Mladek <pmladek@suse.com> wrote:
>
>> On Thu 2026-04-16 13:18:30, Petr Pavlu wrote:
>>> On 4/15/26 8:43 AM, Song Chen wrote:
>>>> On 4/14/26 22:33, Petr Pavlu wrote:
>>>>> On 4/13/26 10:07 AM, chensong_2000@189.cn wrote:
>>>>>> diff --git a/include/linux/module.h b/include/linux/module.h
>>>>>> index 14f391b186c6..0bdd56f9defd 100644
>>>>>> --- a/include/linux/module.h
>>>>>> +++ b/include/linux/module.h
>>>>>> @@ -308,6 +308,14 @@ enum module_state {
>>>>>> MODULE_STATE_COMING, /* Full formed, running module_init. */
>>>>>> MODULE_STATE_GOING, /* Going away. */
>>>>>> MODULE_STATE_UNFORMED, /* Still setting it up. */
>>>>>> + MODULE_STATE_FORMED,
>>>>>
>>>>> I don't see a reason to add a new module state. Why is it necessary and
>>>>> how does it fit with the existing states?
>>>>>
>>>> because once notifier fails in state MODULE_STATE_UNFORMED (now only ftrace has someting to do in this state), notifier chain will roll back by calling blocking_notifier_call_chain_robust, i'm afraid MODULE_STATE_GOING is going to jeopardise the notifers which don't handle it appropriately, like:
>>>>
>>>> case MODULE_STATE_COMING:
>>>> kmalloc();
>>>> case MODULE_STATE_GOING:
>>>> kfree();
>>>
>>> My understanding is that the current module "state machine" operates as
>>> follows. Transitions marked with an asterisk (*) are announced via the
>>> module notifier.
>>>
>>> ---> UNFORMED --*> COMING --*> LIVE --*> GOING -.
>>> ^ | ^ |
>>> | '---------------------* |
>>> '---------------------------------------'
>>>
>>> The new code aims to replace the current ftrace_module_init() call in
>>> load_module(). To achieve this, it adds a notification for the UNFORMED
>>> state (only when loading a module) and introduces a new FORMED state for
>>> rollback. FORMED is purely a fake state because it never appears in
>>> module::state. The new structure is as follows:
>>>
>>> ,--*> (FORMED)
>>> |
>>> --*> UNFORMED --*> COMING --*> LIVE --*> GOING -.
>>> ^ | ^ |
>>> | '---------------------* |
>>> '---------------------------------------'
>>>
>>> I'm afraid this is quite complex and inconsistent. Unless it can be kept
>>> simple, we would be just replacing one special handling with a different
>>> complexity, which is not worth it.
>>
>>>>>
>>>>>> + if (err)
>>>>>> + goto ddebug_cleanup;
>>>>>> /* Finally it's fully formed, ready to start executing. */
>>>>>> err = complete_formation(mod, info);
>>>>>> - if (err)
>>>>>> + if (err) {
>>>>>> + blocking_notifier_call_chain_reverse(&module_notify_list,
>>>>>> + MODULE_STATE_FORMED, mod);
>>>>>> goto ddebug_cleanup;
>>>>>> + }
>>>>>> - err = prepare_coming_module(mod);
>>>>>> + err = prepare_module_state_transaction(mod,
>>>>>> + MODULE_STATE_COMING, MODULE_STATE_GOING);
>>>>>> if (err)
>>>>>> goto bug_cleanup;
>>>>>> @@ -3522,7 +3519,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>>>>> destroy_params(mod->kp, mod->num_kp);
>>>>>> blocking_notifier_call_chain(&module_notify_list,
>>>>>> MODULE_STATE_GOING, mod);
>>>>>
>>>>> My understanding is that all notifier chains for MODULE_STATE_GOING
>>>>> should be reversed.
>>>> yes, all, from lowest priority notifier to highest.
>>>> I will resend patch 1 which was failed due to my proxy setting.
>>>
>>> What I meant here is that the call:
>>>
>>> blocking_notifier_call_chain(&module_notify_list, MODULE_STATE_GOING, mod);
>>>
>>> should be replaced with:
>>>
>>> blocking_notifier_call_chain_reverse(&module_notify_list, MODULE_STATE_GOING, mod);
>>>
>>>>
>>>>>
>>>>>> - klp_module_going(mod);
>>>>>> bug_cleanup:
>>>>>> mod->state = MODULE_STATE_GOING;
>>>>>> /* module_bug_cleanup needs module_mutex protection */
>>>>>
>>>>> The patch removes the klp_module_going() cleanup call in load_module().
>>>>> Similarly, the ftrace_release_mod() call under the ddebug_cleanup label
>>>>> should be removed and appropriately replaced with a cleanup via
>>>>> a notifier.
>>>>>
>>>> err = prepare_module_state_transaction(mod,
>>>> MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
>>>> if (err)
>>>> goto ddebug_cleanup;
>>>>
>>>> ftrace will be cleanup in blocking_notifier_call_chain_robust rolling back.
>>>>
>>>> err = prepare_module_state_transaction(mod,
>>>> MODULE_STATE_COMING, MODULE_STATE_GOING);
>>>>
>>>> each notifier including ftrace and klp will be cleanup in blocking_notifier_call_chain_robust rolling back.
>>>>
>>>> if all notifiers are successful in MODULE_STATE_COMING, they all will be clean up in
>>>> coming_cleanup:
>>>> mod->state = MODULE_STATE_GOING;
>>>> destroy_params(mod->kp, mod->num_kp);
>>>> blocking_notifier_call_chain(&module_notify_list,
>>>> MODULE_STATE_GOING, mod);
>>>>
>>>> if something wrong underneath.
>>>
>>> My point is that the patch leaves a call to ftrace_release_mod() in
>>> load_module(), which I expected to be handled via a notifier.
>>
>> I think that I have got it. The ftrace code needs two notifiers when
>> the module is being loaded and two when it is going.
>>
>> This is why Sond added the new state. But I think that we would
>> need two new states to call:
>>
>> + ftrace_module_init() in MODULE_STATE_UNFORMED
>> + ftrace_module_enable() in MODULE_STATE_FORMED
>>
>> and
>>
>> + ftrace_free_mem() in MODULE_STATE_PRE_GOING
>> + ftrace_free_mem() in MODULE_STATE_GOING
>>
>>
>> By using the ascii art:
>>
>> -*> UNFORMED -*> FORMED -> COMING -*> LIVE -*> PRE_GOING -*> GOING -.
>> | | | ^ ^ ^
>> | | '----------------' | |
>> | '--------------------------------------' |
>> '------------------------------------------------------'
>>
>>
>> But I think that this is not worth it.
>
> Agree.
>
> If this needs to be ordered so strictly, why we will use a "single"
> module notifier chain for this complex situation?
>
> I think the notifier call chain is just for notice a single signal,
> instead of sending several different signals, especially if there is
> any dependency among the callbacks.
>
> If notification callbacks need to be ordered, they are currently
> sorted by representing priority numerically, but this is quite
> fragile for updating. It has to look up other registered priorities
> and adjust the order among dependencies each time. For this reason,
> this mechanism is not suitable for global ordering. (It's like line
> numbers in BASIC.)
> It is probably only useful for representing dependencies between
> two components maintained by the same maintainer.
>
> I'm against a general-purpose system that makes everything modular.
> It unnecessarily complicates things. If there are processes that
> require strict ordering, especially processes that must be performed
> before each stage as part of the framework, they should be called
> directly from the framework, not via notification callbacks.
>
> This makes it simpler and more robust to maintain.
>
> Only the framework's end users should utilize notification callbacks.
>
> Thank you,
>
>
my motivation is to decouple ftrace and klp from module loader and make
blocking_notifier_chain more generic, but it doesn't become generic
completely. I understand your and Petr's comments and agree.
Thanks
Best regards
Song
>>
>> Best Regards,
>> Petr
>>
>
>
^ permalink raw reply
* [PATCH] md/raid1: fix bio splitting in raid1 thread to avoid recursion and deadlock
From: Abd-Alrhman Masalkhi @ 2026-04-27 10:34 UTC (permalink / raw)
To: song, yukuai, shli, neilb, linux-raid, linux-kernel; +Cc: Abd-Alrhman Masalkhi
Splitting a bio while executing in the raid1 thread can lead to
recursion, as task->bio_list is NULL in this context.
In addition, resubmitting an md_cloned_bio after splitting may lead to
a deadlock if the array is suspended before the md driver calls
percpu_ref_tryget_live(&mddev->active_io) on it's path to
pers->make_request().
Avoid splitting the bio in this context and require that it is either
read in full or not at all.
This prevents recursion and avoids potential deadlocks during array
suspension.
Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
I sent an email about this issue two days ago, but at the time I was not
sure whether it was a real problem or a misunderstanding on my part.
After further analysis, it appears that this issue can occur.
Apologies for the earlier confusion, and thank you for your time.
Abd-Alrhman
---
drivers/md/raid1.c | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index cc9914bd15c1..14f6d6625811 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -607,7 +607,7 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
/* choose the first disk even if it has some bad blocks. */
read_len = raid1_check_read_range(rdev, this_sector, &len);
- if (read_len > 0) {
+ if (read_len > 0 && (!*max_sectors || read_len == r1_bio->sectors)) {
update_read_sectors(conf, disk, this_sector, read_len);
*max_sectors = read_len;
return disk;
@@ -704,8 +704,13 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
}
if (bb_disk != -1) {
- *max_sectors = bb_read_len;
- update_read_sectors(conf, bb_disk, this_sector, bb_read_len);
+ if (!*max_sectors || bb_read_len == r1_bio->sectors) {
+ *max_sectors = bb_read_len;
+ update_read_sectors(conf, bb_disk, this_sector,
+ bb_read_len);
+ } else {
+ bb_disk = -1;
+ }
}
return bb_disk;
@@ -852,8 +857,9 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
* disks and disks with bad blocks for now. Only pay attention to key disk
* choice.
*
- * 3) If we've made it this far, now look for disks with bad blocks and choose
- * the one with most number of sectors.
+ * 3) If we've made it this far and *max_sectors is 0 (i.e., we are tolerant
+ * of bad blocks), look for disks with bad blocks and choose the one with
+ * the most sectors.
*
* 4) If we are all the way at the end, we have no choice but to use a disk even
* if it is write mostly.
@@ -882,11 +888,13 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio,
/*
* If we are here it means we didn't find a perfectly good disk so
* now spend a bit more time trying to find one with the most good
- * sectors.
+ * sectors. but only if we are tolerant of bad blocks.
*/
- disk = choose_bb_rdev(conf, r1_bio, max_sectors);
- if (disk >= 0)
- return disk;
+ if (!*max_sectors) {
+ disk = choose_bb_rdev(conf, r1_bio, max_sectors);
+ if (disk >= 0)
+ return disk;
+ }
return choose_slow_rdev(conf, r1_bio, max_sectors);
}
@@ -1346,7 +1354,14 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
/*
* make_request() can abort the operation when read-ahead is being
* used and no empty request is available.
+ *
+ * If we allow splitting the bio while executing in the raid1 thread,
+ * we may end up recursing (current->bio_list is NULL), and we might
+ * also deadlock if we try to suspend the array, since we are
+ * resubmitting an md_cloned_bio. Therefore, we must be read either
+ * all the sectors or none.
*/
+ max_sectors = r1bio_existed;
rdisk = read_balance(conf, r1_bio, &max_sectors);
if (rdisk < 0) {
/* couldn't find anywhere to read from */
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] md/raid1: fix bio splitting in raid1 thread to avoid recursion and deadlock
From: Paul Menzel @ 2026-04-27 14:49 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: song, yukuai, shli, neilb, linux-raid, linux-kernel
In-Reply-To: <20260427103446.300378-1-abd.masalkhi@gmail.com>
Dear Abd-Alrhman,
Thank you for your patch.
Am 27.04.26 um 12:34 schrieb Abd-Alrhman Masalkhi:
> Splitting a bio while executing in the raid1 thread can lead to
> recursion, as task->bio_list is NULL in this context.
>
> In addition, resubmitting an md_cloned_bio after splitting may lead to
> a deadlock if the array is suspended before the md driver calls
> percpu_ref_tryget_live(&mddev->active_io) on it's path to
> pers->make_request().
>
> Avoid splitting the bio in this context and require that it is either
> read in full or not at all.
>
> This prevents recursion and avoids potential deadlocks during array
> suspension.
Do you have a reproducer?
> Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> I sent an email about this issue two days ago, but at the time I was not
> sure whether it was a real problem or a misunderstanding on my part.
>
> After further analysis, it appears that this issue can occur.
>
> Apologies for the earlier confusion, and thank you for your time.
>
> Abd-Alrhman
I suggest to always share the URL (lore.kernel.org), when referencing
another thread. If relevant, maybe even reference your message with a
Link: tag in the commit message.
> ---
> drivers/md/raid1.c | 33 ++++++++++++++++++++++++---------
> 1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index cc9914bd15c1..14f6d6625811 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -607,7 +607,7 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>
> /* choose the first disk even if it has some bad blocks. */
> read_len = raid1_check_read_range(rdev, this_sector, &len);
> - if (read_len > 0) {
> + if (read_len > 0 && (!*max_sectors || read_len == r1_bio->sectors)) {
> update_read_sectors(conf, disk, this_sector, read_len);
> *max_sectors = read_len;
> return disk;
> @@ -704,8 +704,13 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
> }
>
> if (bb_disk != -1) {
> - *max_sectors = bb_read_len;
> - update_read_sectors(conf, bb_disk, this_sector, bb_read_len);
> + if (!*max_sectors || bb_read_len == r1_bio->sectors) {
> + *max_sectors = bb_read_len;
> + update_read_sectors(conf, bb_disk, this_sector,
> + bb_read_len);
> + } else {
> + bb_disk = -1;
> + }
> }
>
> return bb_disk;
> @@ -852,8 +857,9 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
> * disks and disks with bad blocks for now. Only pay attention to key disk
> * choice.
> *
> - * 3) If we've made it this far, now look for disks with bad blocks and choose
> - * the one with most number of sectors.
> + * 3) If we've made it this far and *max_sectors is 0 (i.e., we are tolerant
> + * of bad blocks), look for disks with bad blocks and choose the one with
> + * the most sectors.
> *
> * 4) If we are all the way at the end, we have no choice but to use a disk even
> * if it is write mostly.
> @@ -882,11 +888,13 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio,
> /*
> * If we are here it means we didn't find a perfectly good disk so
> * now spend a bit more time trying to find one with the most good
> - * sectors.
> + * sectors. but only if we are tolerant of bad blocks.
s/but/But/
> */
> - disk = choose_bb_rdev(conf, r1_bio, max_sectors);
> - if (disk >= 0)
> - return disk;
> + if (!*max_sectors) {
> + disk = choose_bb_rdev(conf, r1_bio, max_sectors);
> + if (disk >= 0)
> + return disk;
> + }
>
> return choose_slow_rdev(conf, r1_bio, max_sectors);
> }
> @@ -1346,7 +1354,14 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> /*
> * make_request() can abort the operation when read-ahead is being
> * used and no empty request is available.
> + *
> + * If we allow splitting the bio while executing in the raid1 thread,
> + * we may end up recursing (current->bio_list is NULL), and we might
> + * also deadlock if we try to suspend the array, since we are
> + * resubmitting an md_cloned_bio. Therefore, we must be read either
… we must read …
> + * all the sectors or none.
> */
> + max_sectors = r1bio_existed;
Excuse my ignorance, but I do not get why a bool is assigned to an int
representing the maximum sector value.
> rdisk = read_balance(conf, r1_bio, &max_sectors);
> if (rdisk < 0) {
> /* couldn't find anywhere to read from */
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH] md/raid1: fix bio splitting in raid1 thread to avoid recursion and deadlock
From: Abd-Alrhman Masalkhi @ 2026-04-27 17:44 UTC (permalink / raw)
To: Paul Menzel; +Cc: song, yukuai, shli, neilb, linux-raid, linux-kernel
In-Reply-To: <c859ab17-cfc6-4e54-9a79-8b0d2b145adc@molgen.mpg.de>
Hi Paul,
Thank you for the feedback.
On Mon, Apr 27, 2026 at 16:49 +0200, Paul Menzel wrote:
> Dear Abd-Alrhman,
>
>
> Thank you for your patch.
>
> Am 27.04.26 um 12:34 schrieb Abd-Alrhman Masalkhi:
>> Splitting a bio while executing in the raid1 thread can lead to
>> recursion, as task->bio_list is NULL in this context.
>>
>> In addition, resubmitting an md_cloned_bio after splitting may lead to
>> a deadlock if the array is suspended before the md driver calls
>> percpu_ref_tryget_live(&mddev->active_io) on it's path to
>> pers->make_request().
>>
>> Avoid splitting the bio in this context and require that it is either
>> read in full or not at all.
>>
>> This prevents recursion and avoids potential deadlocks during array
>> suspension.
>
> Do you have a reproducer?
I found this issue while reviewing the code and trying to understand the
read path.
The problem can be triggered when the first rdev cannot complete the
md_cloned_bio successfully, and RAID1 selects another rdev that cannot
fulfil the entire request. In that case, raid1_read_request() will split
the bio (the md_cloned_bio) via bio_submit_split_bioset(), which in turn
calls submit_bio_noacct_nocheck(). Since current->bio_list is NULL in
this context, this leads to raid1_read_request() being called again,
resulting in recursion.
I have also created a test to confirm that this can occur by modifying
the code with the following patch:
drivers/md/raid1.c | 52 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 46 insertions(+), 6 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index cc9914bd15c1..145e3ad0b1b8 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -362,11 +362,34 @@ static int find_bio_disk(struct r1bio *r1_bio, struct bio *bio)
static void raid1_end_read_request(struct bio *bio)
{
+ static int tmp = 75;
int uptodate = !bio->bi_status;
struct r1bio *r1_bio = bio->bi_private;
struct r1conf *conf = r1_bio->mddev->private;
struct md_rdev *rdev = conf->mirrors[r1_bio->read_disk].rdev;
+ if (tmp > 2) {
+ tmp --;
+ pr_info("--tmp = %d\n", tmp);
+ } else if (tmp == 2) {
+ if (r1_bio->sectors > 2 && uptodate) {
+ pr_info("I will start omitting errors\n");
+ bio->bi_status = BLK_STS_IOERR;
+ uptodate = false;
+ tmp = 0;
+ }
+ } else {
+ if (r1_bio->sectors > 2) {
+ if (tmp) {
+ bio->bi_status = BLK_STS_IOERR;
+ uptodate = false;
+ tmp = false;
+ } else {
+ tmp = true;
+ }
+ }
+ }
+
/*
* this branch is our 'one mirror IO has finished' event handler:
*/
@@ -607,7 +630,7 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
/* choose the first disk even if it has some bad blocks. */
read_len = raid1_check_read_range(rdev, this_sector, &len);
- if (read_len > 0) {
+ if (read_len > 0 && (!*max_sectors || read_len == r1_bio->sectors)) {
update_read_sectors(conf, disk, this_sector, read_len);
*max_sectors = read_len;
return disk;
@@ -704,8 +727,12 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
}
if (bb_disk != -1) {
- *max_sectors = bb_read_len;
- update_read_sectors(conf, bb_disk, this_sector, bb_read_len);
+ if (!*max_sectors || bb_read_len == r1_bio->sectors) {
+ *max_sectors = bb_read_len;
+ update_read_sectors(conf, bb_disk, this_sector, bb_read_len);
+ } else {
+ bb_disk = -1;
+ }
}
return bb_disk;
@@ -884,9 +911,11 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio,
* now spend a bit more time trying to find one with the most good
* sectors.
*/
- disk = choose_bb_rdev(conf, r1_bio, max_sectors);
- if (disk >= 0)
- return disk;
+ if (!*max_sectors) {
+ disk = choose_bb_rdev(conf, r1_bio, max_sectors);
+ if (disk >= 0)
+ return disk;
+ }
return choose_slow_rdev(conf, r1_bio, max_sectors);
}
@@ -1320,6 +1349,11 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
int rdisk;
bool r1bio_existed = !!r1_bio;
+ if (mddev->thread && mddev->thread->tsk == current) {
+ pr_info("taask->bio_list = %p, %d\n", current->bio_list,
+ r1bio_existed);
+ }
+
/*
* If r1_bio is set, we are blocking the raid1d thread
* so there is a tiny risk of deadlock. So ask for
@@ -1347,6 +1381,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
* make_request() can abort the operation when read-ahead is being
* used and no empty request is available.
*/
+ max_sectors = r1bio_existed;
rdisk = read_balance(conf, r1_bio, &max_sectors);
if (rdisk < 0) {
/* couldn't find anywhere to read from */
@@ -1376,7 +1411,12 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
mddev->bitmap_ops->wait_behind_writes(mddev);
}
+ if (r1bio_existed)
+ max_sectors = max_sectors - 1;
+
if (max_sectors < bio_sectors(bio)) {
+ /* we are not allowed */
+ /* BUG_ON(r1bio_existed); */
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
if (!bio) {
--
2.43.0
using trace-cmd, it shows the recursion clearly, its output:
raid1_read_request() {
bio_submit_split_bioset() {
bio_split() {
bio_alloc_clone() {
bio_alloc_bioset() {
mempool_alloc_noprof() {
__cond_resched();
mempool_alloc_slab() {
kmem_cache_alloc_noprof();
}
}
bio_associate_blkg() {
__rcu_read_lock();
kthread_blkcg();
bio_associate_blkg_from_css() {
__rcu_read_lock();
__rcu_read_unlock();
}
__rcu_read_unlock();
}
}
bio_clone_blkg_association() {
bio_associate_blkg_from_css() {
__rcu_read_lock();
__rcu_read_unlock();
__rcu_read_lock();
__rcu_read_unlock();
}
}
}
}
bio_chain();
should_fail_bio();
submit_bio_noacct_nocheck() {
blk_cgroup_bio_start();
__submit_bio() {
__rcu_read_lock();
__rcu_read_unlock();
md_submit_bio() {
bio_split_to_limits() {
bio_split_rw() {
bio_split_io_at();
bio_submit_split();
}
}
md_handle_request() {
__rcu_read_lock();
__rcu_read_unlock();
raid1_make_request() {
raid1_read_request() {
_printk() {
vprintk() {
vprintk_default() {
vprintk_emit() {
panic_on_other_cpu();
nbcon_get_default_prio() {
panic_on_this_cpu();
nbcon_get_cpu_emergency_nesting() {
printk_percpu_data_ready();
}
}
is_printk_legacy_deferred() {
is_printk_cpu_sync_owner();
}
vprintk_store() {
local_clock();
printk_parse_prefix();
is_printk_force_console();
prb_reserve() {
data_alloc() {
data_push_tail();
}
space_used.isra.0();
}
printk_sprint() {
printk_parse_prefix();
}
prb_final_commit() {
desc_update_last_finalized() {
_prb_read_valid() {
desc_read_finalized_seq();
}
_prb_read_valid() {
desc_read_finalized_seq();
panic_on_this_cpu();
}
}
}
}
console_trylock() {
panic_on_other_cpu();
__printk_safe_enter();
down_trylock() {
_raw_spin_lock_irqsave();
_raw_spin_unlock_irqrestore();
}
__printk_safe_exit();
}
console_unlock() {
nbcon_get_default_prio() {
panic_on_this_cpu();
nbcon_get_cpu_emergency_nesting() {
printk_percpu_data_ready();
}
}
is_printk_legacy_deferred() {
is_printk_cpu_sync_owner();
}
console_flush_one_record() {
nbcon_get_default_prio() {
panic_on_this_cpu();
nbcon_get_cpu_emergency_nesting() {
printk_percpu_data_ready();
}
}
is_printk_legacy_deferred() {
is_printk_cpu_sync_owner();
}
__srcu_read_lock();
printk_get_next_message() {
prb_read_valid() {
_prb_read_valid() {
desc_read_finalized_seq();
get_data();
desc_read_finalized_seq();
}
}
}
panic_on_other_cpu();
__srcu_read_unlock();
}
console_flush_one_record() {
nbcon_get_default_prio() {
panic_on_this_cpu();
nbcon_get_cpu_emergency_nesting() {
printk_percpu_data_ready();
}
}
is_printk_legacy_deferred() {
is_printk_cpu_sync_owner();
}
__srcu_read_lock();
printk_get_next_message() {
prb_read_valid() {
_prb_read_valid() {
desc_read_finalized_seq();
panic_on_this_cpu();
}
}
}
__srcu_read_unlock();
}
__printk_safe_enter();
up() {
_raw_spin_lock_irqsave();
_raw_spin_unlock_irqrestore();
}
__printk_safe_exit();
prb_read_valid() {
_prb_read_valid() {
desc_read_finalized_seq();
panic_on_this_cpu();
}
}
}
__wake_up_klogd();
}
}
}
}
mempool_alloc_noprof() {
__cond_resched();
mempool_kmalloc() {
__kmalloc_noprof();
}
}
md_account_bio() {
__rcu_read_lock();
__rcu_read_unlock();
bio_alloc_clone() {
bio_alloc_bioset() {
mempool_alloc_noprof() {
__cond_resched();
mempool_alloc_slab() {
kmem_cache_alloc_noprof();
}
}
bio_associate_blkg() {
__rcu_read_lock();
kthread_blkcg();
bio_associate_blkg_from_css() {
__rcu_read_lock();
__rcu_read_unlock();
}
__rcu_read_unlock();
}
}
bio_clone_blkg_association() {
bio_associate_blkg_from_css() {
__rcu_read_lock();
__rcu_read_unlock();
__rcu_read_lock();
__rcu_read_unlock();
}
}
}
bio_start_io_acct() {
update_io_ticks();
}
}
bio_alloc_clone() {
bio_alloc_bioset() {
mempool_alloc_noprof() {
__cond_resched();
mempool_alloc_slab() {
kmem_cache_alloc_noprof();
}
}
bio_associate_blkg() {
__rcu_read_lock();
kthread_blkcg();
bio_associate_blkg_from_css() {
__rcu_read_lock();
__rcu_read_unlock();
}
__rcu_read_unlock();
}
}
bio_clone_blkg_association() {
bio_associate_blkg_from_css() {
__rcu_read_lock();
__rcu_read_unlock();
__rcu_read_lock();
__rcu_read_unlock();
}
}
}
submit_bio_noacct() {
__cond_resched();
submit_bio_noacct_nocheck() {
blk_cgroup_bio_start();
}
}
}
}
__rcu_read_lock();
__rcu_read_unlock();
}
}
__rcu_read_lock();
__rcu_read_unlock();
}
__submit_bio() {
blk_mq_submit_bio() {
__rcu_read_lock();
__rcu_read_unlock();
bio_split_rw() {
bio_split_io_at();
bio_submit_split();
}
blk_attempt_plug_merge();
blk_mq_sched_bio_merge();
__blk_mq_alloc_requests() {
blk_mq_get_tag() {
__blk_mq_get_tag();
}
blk_mq_rq_ctx_init.isra.0();
}
ktime_get();
update_io_ticks();
blk_add_rq_to_plug();
}
}
}
}
bio_alloc_clone() {
bio_alloc_bioset() {
mempool_alloc_noprof() {
__cond_resched();
mempool_alloc_slab() {
kmem_cache_alloc_noprof() {
__slab_alloc.isra.0() {
___slab_alloc();
}
}
}
}
bio_associate_blkg() {
__rcu_read_lock();
kthread_blkcg();
bio_associate_blkg_from_css() {
__rcu_read_lock();
__rcu_read_unlock();
}
__rcu_read_unlock();
}
}
bio_clone_blkg_association() {
bio_associate_blkg_from_css() {
__rcu_read_lock();
__rcu_read_unlock();
__rcu_read_lock();
__rcu_read_unlock();
}
}
}
submit_bio_noacct() {
__cond_resched();
submit_bio_noacct_nocheck() {
blk_cgroup_bio_start();
__submit_bio() {
blk_mq_submit_bio() {
__rcu_read_lock();
__rcu_read_unlock();
bio_split_rw() {
bio_split_io_at();
bio_submit_split();
}
blk_attempt_plug_merge();
blk_mq_sched_bio_merge();
__blk_mq_alloc_requests() {
blk_mq_get_tag() {
__blk_mq_get_tag();
}
blk_mq_rq_ctx_init.isra.0();
}
update_io_ticks() {
bdev_count_inflight();
}
blk_add_rq_to_plug();
}
}
}
}
}
>
>> Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> I sent an email about this issue two days ago, but at the time I was not
>> sure whether it was a real problem or a misunderstanding on my part.
>>
>> After further analysis, it appears that this issue can occur.
>>
>> Apologies for the earlier confusion, and thank you for your time.
>>
>> Abd-Alrhman
>
> I suggest to always share the URL (lore.kernel.org), when referencing
> another thread. If relevant, maybe even reference your message with a
> Link: tag in the commit message.
Yes, i will make sure to do that next time. Here is the link:
https://lore.kernel.org/linux-raid/20260425142938.5555-1-abd.masalkhi@gmail.com/T
>
>> ---
>> drivers/md/raid1.c | 33 ++++++++++++++++++++++++---------
>> 1 file changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index cc9914bd15c1..14f6d6625811 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -607,7 +607,7 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>>
>> /* choose the first disk even if it has some bad blocks. */
>> read_len = raid1_check_read_range(rdev, this_sector, &len);
>> - if (read_len > 0) {
>> + if (read_len > 0 && (!*max_sectors || read_len == r1_bio->sectors)) {
>> update_read_sectors(conf, disk, this_sector, read_len);
>> *max_sectors = read_len;
>> return disk;
>> @@ -704,8 +704,13 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>> }
>>
>> if (bb_disk != -1) {
>> - *max_sectors = bb_read_len;
>> - update_read_sectors(conf, bb_disk, this_sector, bb_read_len);
>> + if (!*max_sectors || bb_read_len == r1_bio->sectors) {
>> + *max_sectors = bb_read_len;
>> + update_read_sectors(conf, bb_disk, this_sector,
>> + bb_read_len);
>> + } else {
>> + bb_disk = -1;
>> + }
>> }
>>
>> return bb_disk;
>> @@ -852,8 +857,9 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
>> * disks and disks with bad blocks for now. Only pay attention to key disk
>> * choice.
>> *
>> - * 3) If we've made it this far, now look for disks with bad blocks and choose
>> - * the one with most number of sectors.
>> + * 3) If we've made it this far and *max_sectors is 0 (i.e., we are tolerant
>> + * of bad blocks), look for disks with bad blocks and choose the one with
>> + * the most sectors.
>> *
>> * 4) If we are all the way at the end, we have no choice but to use a disk even
>> * if it is write mostly.
>> @@ -882,11 +888,13 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio,
>> /*
>> * If we are here it means we didn't find a perfectly good disk so
>> * now spend a bit more time trying to find one with the most good
>> - * sectors.
>> + * sectors. but only if we are tolerant of bad blocks.
>
> s/but/But/
>
I will fix this in v2.
>> */
>> - disk = choose_bb_rdev(conf, r1_bio, max_sectors);
>> - if (disk >= 0)
>> - return disk;
>> + if (!*max_sectors) {
>> + disk = choose_bb_rdev(conf, r1_bio, max_sectors);
>> + if (disk >= 0)
>> + return disk;
>> + }
>>
>> return choose_slow_rdev(conf, r1_bio, max_sectors);
>> }
>> @@ -1346,7 +1354,14 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>> /*
>> * make_request() can abort the operation when read-ahead is being
>> * used and no empty request is available.
>> + *
>> + * If we allow splitting the bio while executing in the raid1 thread,
>> + * we may end up recursing (current->bio_list is NULL), and we might
>> + * also deadlock if we try to suspend the array, since we are
>> + * resubmitting an md_cloned_bio. Therefore, we must be read either
>
> … we must read …
>
I will fix this in v2.
>> + * all the sectors or none.
>> */
>> + max_sectors = r1bio_existed;
>
> Excuse my ignorance, but I do not get why a bool is assigned to an int
> representing the maximum sector value.
>
I modified read_balance() to interpret *max_sectors as a flag. If it is
0, the read path is allowed to be tolerant of bad blocks; otherwise, it
is not. In both cases, *max_sectors will eventually be updated to the
maximum number of readable sectors if a suitable disk is found.
I used r1bio_existed to initialize this value, so assigning
max_sectors = r1bio_existed effectively encodes this behavior
without introducing an additional parameter.
>> rdisk = read_balance(conf, r1_bio, &max_sectors);
>> if (rdisk < 0) {
>> /* couldn't find anywhere to read from */
>
>
> Kind regards,
>
> Paul
--
Best Regards,
Abd-Alrhman
^ permalink raw reply related
* Re: [PATCH] md/raid1: fix bio splitting in raid1 thread to avoid recursion and deadlock
From: Abd-Alrhman Masalkhi @ 2026-04-28 8:16 UTC (permalink / raw)
To: Paul Menzel; +Cc: song, yukuai, shli, neilb, linux-raid, linux-kernel
In-Reply-To: <c859ab17-cfc6-4e54-9a79-8b0d2b145adc@molgen.mpg.de>
Hi Paul,
On Mon, Apr 27, 2026 at 16:49 +0200, Paul Menzel wrote:
> Dear Abd-Alrhman,
>
>
> Thank you for your patch.
>
RAID 10 seems to have a similar issue, i will fix it too.
> Am 27.04.26 um 12:34 schrieb Abd-Alrhman Masalkhi:
>> Splitting a bio while executing in the raid1 thread can lead to
>> recursion, as task->bio_list is NULL in this context.
>>
>> In addition, resubmitting an md_cloned_bio after splitting may lead to
>> a deadlock if the array is suspended before the md driver calls
>> percpu_ref_tryget_live(&mddev->active_io) on it's path to
>> pers->make_request().
>>
>> Avoid splitting the bio in this context and require that it is either
>> read in full or not at all.
>>
>> This prevents recursion and avoids potential deadlocks during array
>> suspension.
>
> Do you have a reproducer?
>
>> Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> I sent an email about this issue two days ago, but at the time I was not
>> sure whether it was a real problem or a misunderstanding on my part.
>>
>> After further analysis, it appears that this issue can occur.
>>
>> Apologies for the earlier confusion, and thank you for your time.
>>
>> Abd-Alrhman
>
> I suggest to always share the URL (lore.kernel.org), when referencing
> another thread. If relevant, maybe even reference your message with a
> Link: tag in the commit message.
>
>> ---
>> drivers/md/raid1.c | 33 ++++++++++++++++++++++++---------
>> 1 file changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index cc9914bd15c1..14f6d6625811 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -607,7 +607,7 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>>
>> /* choose the first disk even if it has some bad blocks. */
>> read_len = raid1_check_read_range(rdev, this_sector, &len);
>> - if (read_len > 0) {
>> + if (read_len > 0 && (!*max_sectors || read_len == r1_bio->sectors)) {
>> update_read_sectors(conf, disk, this_sector, read_len);
>> *max_sectors = read_len;
>> return disk;
>> @@ -704,8 +704,13 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>> }
>>
>> if (bb_disk != -1) {
>> - *max_sectors = bb_read_len;
>> - update_read_sectors(conf, bb_disk, this_sector, bb_read_len);
>> + if (!*max_sectors || bb_read_len == r1_bio->sectors) {
>> + *max_sectors = bb_read_len;
>> + update_read_sectors(conf, bb_disk, this_sector,
>> + bb_read_len);
>> + } else {
>> + bb_disk = -1;
>> + }
>> }
>>
>> return bb_disk;
>> @@ -852,8 +857,9 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
>> * disks and disks with bad blocks for now. Only pay attention to key disk
>> * choice.
>> *
>> - * 3) If we've made it this far, now look for disks with bad blocks and choose
>> - * the one with most number of sectors.
>> + * 3) If we've made it this far and *max_sectors is 0 (i.e., we are tolerant
>> + * of bad blocks), look for disks with bad blocks and choose the one with
>> + * the most sectors.
>> *
>> * 4) If we are all the way at the end, we have no choice but to use a disk even
>> * if it is write mostly.
>> @@ -882,11 +888,13 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio,
>> /*
>> * If we are here it means we didn't find a perfectly good disk so
>> * now spend a bit more time trying to find one with the most good
>> - * sectors.
>> + * sectors. but only if we are tolerant of bad blocks.
>
> s/but/But/
>
>> */
>> - disk = choose_bb_rdev(conf, r1_bio, max_sectors);
>> - if (disk >= 0)
>> - return disk;
>> + if (!*max_sectors) {
>> + disk = choose_bb_rdev(conf, r1_bio, max_sectors);
>> + if (disk >= 0)
>> + return disk;
>> + }
>>
>> return choose_slow_rdev(conf, r1_bio, max_sectors);
>> }
>> @@ -1346,7 +1354,14 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>> /*
>> * make_request() can abort the operation when read-ahead is being
>> * used and no empty request is available.
>> + *
>> + * If we allow splitting the bio while executing in the raid1 thread,
>> + * we may end up recursing (current->bio_list is NULL), and we might
>> + * also deadlock if we try to suspend the array, since we are
>> + * resubmitting an md_cloned_bio. Therefore, we must be read either
>
> … we must read …
>
>> + * all the sectors or none.
>> */
>> + max_sectors = r1bio_existed;
>
> Excuse my ignorance, but I do not get why a bool is assigned to an int
> representing the maximum sector value.
>
>> rdisk = read_balance(conf, r1_bio, &max_sectors);
>> if (rdisk < 0) {
>> /* couldn't find anywhere to read from */
>
>
> Kind regards,
>
> Paul
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v13 0/3] md/md-bitmap: restore bitmap grow through sysfs
From: Yu Kuai @ 2026-04-28 8:21 UTC (permalink / raw)
To: Su Yue
Cc: Song Liu, glass.su, Li Nan, Xiao Ni, linux-raid, linux-kernel,
yukuai
In-Reply-To: <qzo35t50.fsf@damenly.org>
Hi,
在 2026/4/25 16:41, Su Yue 写道:
> On Sat 25 Apr 2026 at 10:46, Yu Kuai <yukuai@fnnas.com> wrote:
>
>> mdadm --grow adds an internal bitmap by writing bitmap/location for an
>> array that currently has no bitmap. That requires the bitmap directory
>> and location attribute to exist before the classic bitmap backend is
>> created.
>>
>> This series separates bitmap backend lifetime from bitmap sysfs
>> lifetime,
>> splits the sysfs layout into common and backend-specific groups, and
>> adds
>> a small "none" bitmap backend. The none backend keeps bitmap/location
>> available while no real bitmap is active, and the location store path
>> can
>> then switch between the none backend and the classic bitmap backend
>> without tearing down the common bitmap sysfs directory.
>>
>> Patch 1 factors bitmap creation and destruction into helpers that do not
>> touch sysfs registration.
>>
>> Patch 2 splits the classic bitmap sysfs files into a common group and an
>> internal-bitmap group, and converts bitmap backend operations to use a
>> sysfs group array.
>>
>> Patch 3 adds the none backend and uses it to restore mdadm --grow bitmap
>> addition through bitmap/location.
>>
>> Changes since v12:
>> - Keep the factoring patch focused on no-sysfs bitmap lifetime helpers.
>> - Make bitmap operation lookup depend only on the current bitmap id
>> matching the installed backend.
>> - Trim the none backend to only the operations required by the active
>> call paths.
>> - Rework bitmap/location error handling with explicit cleanup labels.
>> - Restore the none backend after bitmap removal and creation/load
>> failures so bitmap/location stays available.
>>
>> Validation:
>> - create a RAID1 array with --bitmap=none
>> - verify /sys/block/md0/md/bitmap/location exists and reports "none"
>> - mdadm --grow /dev/md0 --bitmap=internal
>> - verify location switches to "+8", mdadm reports "Intent Bitmap:
>> Internal", and /proc/mdstat reports a bitmap
>> - mdadm --grow /dev/md0 --bitmap=none
>> - verify location switches back to "none" and only the common
>> location
>> attribute remains under md/bitmap
>> - repeat the internal/none switch once more
>> - Checked the QEMU serial log for panic, Oops, BUG, WARNING, Call Trace,
>> RCU stall, and hung-task patterns; none were found.
>>
>> Yu Kuai (3):
>> md: factor bitmap creation away from sysfs handling
>> md/md-bitmap: split bitmap sysfs groups
>> md/md-bitmap: add a none backend for bitmap grow
>>
>
> Thanks for all.
> Would you like to add tag Fixes: fb8cc3b0d9db for the whole series
> while merging?
Sure, sorry that I forgot the fix tag.
>
> --
> Su
>
>> drivers/md/md-bitmap.c | 131 +++++++++++++++++++++++++++++++++++----
>> drivers/md/md-bitmap.h | 2 +-
>> drivers/md/md-llbitmap.c | 7 ++-
>> drivers/md/md.c | 125 ++++++++++++++++++++++++++-----------
>> drivers/md/md.h | 3 +
>> 5 files changed, 218 insertions(+), 50 deletions(-)
Applied with following fix tag for patch 3:
Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
>>
>> base-commit: c85d314b135ff569c1031f2ef8e40368bcfe72ac
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid1: fix len reuse across rdevs in choose_first_rdev()
From: Yu Kuai @ 2026-04-28 8:23 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, paul.e.luse, xni, yukuai
Cc: linux-raid, linux-kernel
In-Reply-To: <20260426093506.14316-1-abd.masalkhi@gmail.com>
Hi,
在 2026/4/26 17:35, Abd-Alrhman Masalkhi 写道:
> choose_first_rdev() initializes the variable len before iterating over
> all rdevs, but passes it by reference to raid1_check_read_range(), which
> it might update *len and return 0 depending on the layout of the bad
> block region. As a result, 'len' can be modified during the first
> iteration and reused for subsequent rdevs, causing later devices to be
> evaluated with an incorrect length value.
>
> Fixes: 31a73331752d3 ("md/raid1: factor out read_first_rdev() from read_balance()")
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> drivers/md/raid1.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index b549be9174bb..5f5dbf79c903 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -591,12 +591,12 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
> int *max_sectors)
> {
> sector_t this_sector = r1_bio->sector;
> - int len = r1_bio->sectors;
> int disk;
>
> for (disk = 0 ; disk < conf->raid_disks * 2 ; disk++) {
> struct md_rdev *rdev;
> int read_len;
> + int len = r1_bio->sectors;
>
> if (r1_bio->bios[disk] == IO_BLOCKED)
> continue;
This patch is wrong, choose_first_rdev() is used when raid1_should_read_first() is true,
meaning the read overlaps an unsynced/resyncing area. Reset len can cause the problem that
reading the same area can return different data.
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md: skip redundant raid_disks update when value is unchanged
From: Yu Kuai @ 2026-04-28 8:25 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song; +Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <20260425085843.3725-1-abd.masalkhi@gmail.com>
Hi,
在 2026/4/25 16:58, Abd-Alrhman Masalkhi 写道:
> Calling update_raid_disks() with the same value as the current one
> can trigger unnecessary work. For example, RAID1 will reallocate
> resources such as the mempool for r1bio.
>
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> It returns -EINVAL for the same value. If silent success is preferred
> instead, please let me know so I adjust its behavior.
> ---
> drivers/md/md.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 0e55639211f2..cb66c4ebbafa 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -4409,9 +4409,12 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
> err = mddev_suspend_and_lock(mddev);
> if (err)
> return err;
> - if (mddev->pers)
> - err = update_raid_disks(mddev, n);
> - else if (mddev->reshape_position != MaxSector) {
> + if (mddev->pers) {
> + if (n != mddev->raid_disks)
> + err = update_raid_disks(mddev, n);
> + else
> + err = -EINVAL;
Changing return value in this case is not expected, especially from
success to failure.
> + } else if (mddev->reshape_position != MaxSector) {
> struct md_rdev *rdev;
> int olddisks = mddev->raid_disks - mddev->delta_disks;
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: fix race between reshape and chunk-aligned read
From: Yu Kuai @ 2026-04-28 8:29 UTC (permalink / raw)
To: FengWei Shih, song; +Cc: linan122, linux-raid, linux-kernel, yukuai
In-Reply-To: <d298e159-1e47-4c97-80cd-41c4c1c74a04@synology.com>
Hi,
在 2026/4/21 15:09, FengWei Shih 写道:
> Hi Kuai,
>
> Yu Kuai 於 2026/4/19 下午 01:14 寫道:
>> Hi,
>>
>> 在 2026/4/9 13:17, FengWei Shih 写道:
>>> raid5_make_request() checks mddev->reshape_position to decide whether
>>> to allow chunk-aligned reads. However in raid5_start_reshape(), the
>>> layout configuration (raid_disks, algorithm, etc.) is updated before
>>> mddev->reshape_position is set:
>>>
>>> reshape (raid5_start_reshape) read (raid5_make_request)
>>> ============================== ===========================
>>> write_seqcount_begin
>>> update raid_disks, algorithm...
>>> set conf->reshape_progress
>>> write_seqcount_end
>>> check mddev->reshape_position
>>> * still MaxSector, allow
>>> raid5_read_one_chunk()
>>> * use new layout
>>> raid5_quiesce()
>>> set mddev->reshape_position
>> While we're here, I think it's pretty ugly to disable
>> raid5_read_one_chunk
>> when reshape is not fully done. So a better solution should be:
>>
>> - data behind reshape: read with new layout
>> - data ahead of reshape: read with old layout, reshape will also need
>> to wait
>> for this IO to be done, before reshape can make progress.
>> - data intersecting the active reshape window: wait for reshape to
>> make progress.
>
> Thanks for the feedback. I agree that using reshape_progress to
> distinguish
> the three cases (ahead/behind/inside reshape) would be more refined.
>
> But disabling chunk-aligned reads during reshape was already the
> existing design.
> In the original code, the check is at the caller level:
>
> if (rw == READ && mddev->degraded == 0 &&
> mddev->reshape_position == MaxSector) {
> bi = chunk_aligned_read(mddev, bi);
> }
>
> My patch is focused on fixing the race condition in the existing
> lockless check of whether
> reshape is in progress. So just to confirm: are you suggesting we add
> a mechanism to
> allow chunk-aligned reads during reshape (based on reshape progress),
> rather
> than simply disabling them?
Yes, I think this is a huge performance improvement. And in this case it's totally
fine to do chunk aligned read with old layout since reshape do not start yet.
>
>>> Since reshape_position is not yet updated, raid5_make_request()
>>> considers no reshape is in progress and proceeds with the
>>> chunk-aligned path, but the layout has already changed, causing
>>> raid5_compute_sector() to return an incorrect physical address.
>>>
>>> Fix this by reading conf->reshape_progress under gen_lock in
>>> raid5_read_one_chunk() and falling back to the stripe path if a
>>> reshape is in progress.
>>>
>>> Signed-off-by: FengWei Shih <dannyshih@synology.com>
>>> ---
>>> drivers/md/raid5.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>>> index a8e8d431071b..bded2b86f0ef 100644
>>> --- a/drivers/md/raid5.c
>>> +++ b/drivers/md/raid5.c
>>> @@ -5421,6 +5421,11 @@ static int raid5_read_one_chunk(struct mddev
>>> *mddev, struct bio *raid_bio)
>>> sector_t sector, end_sector;
>>> int dd_idx;
>>> bool did_inc;
>>> + int seq;
>>> +
>>> + seq = read_seqcount_begin(&conf->gen_lock);
>>> + if (unlikely(conf->reshape_progress != MaxSector))
>>> + return 0;
>>> if (!in_chunk_boundary(mddev, raid_bio)) {
>>> pr_debug("%s: non aligned\n", __func__);
>>> @@ -5431,6 +5436,9 @@ static int raid5_read_one_chunk(struct mddev
>>> *mddev, struct bio *raid_bio)
>>> &dd_idx, NULL);
>>> end_sector = sector + bio_sectors(raid_bio);
>>> + if (read_seqcount_retry(&conf->gen_lock, seq))
>>> + return 0;
>>> +
>>> if (r5c_big_stripe_cached(conf, sector))
>>> return 0;
> Thanks,
> FengWei Shih
>
> Disclaimer: The contents of this e-mail message and any attachments
> are confidential and are intended solely for addressee. The
> information may also be legally privileged. This transmission is sent
> in trust, for the sole purpose of delivery to the intended recipient.
> If you have received this transmission in error, any use, reproduction
> or dissemination of this transmission is strictly prohibited. If you
> are not the intended recipient, please immediately notify the sender
> by reply e-mail or phone and delete this message and its attachments,
> if any.
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid10: fix divide-by-zero in setup_geo() with zero far_copies
From: Yu Kuai @ 2026-04-28 8:32 UTC (permalink / raw)
To: Yuhao Jiang
Cc: Junrui Luo, Song Liu, Li Nan, NeilBrown, Jonathan Brassow,
linux-raid, linux-kernel, stable, yukuai
In-Reply-To: <CAHYQsXRN6uof4yyDR6qGteQ=wZTt86VUx7km6k=LbNAQ3wxGiQ@mail.gmail.com>
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
^ permalink raw reply
* Re: [RFC PATCH] dm-raid: only requeue bios when dm is suspending.
From: Yu Kuai @ 2026-04-28 8:35 UTC (permalink / raw)
To: Benjamin Marzinski, Yang Xiuwei
Cc: Yu Kuai, Li Nan, Song Liu, linux-raid, dm-devel, Xiao Ni,
Nigel Croxon, yukuai
In-Reply-To: <20260414190350.1946406-1-bmarzins@redhat.com>
Hi,
在 2026/4/15 3:03, Benjamin Marzinski 写道:
> returning DM_MAPIO_REQUEUE from the target map() function only requeues
> the bio during noflush suspends. During regular operations or during
> flushing suspends, it fails the bio. Failing the bio during flushing
> suspends is the correct behavior here. We cannot handle the bio, and we
> cannot suspends while it is outstanding. But during normal operations,
> we should not push the bio back to do. Instead, wait for the reshape
> to be resumed.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>
> Yang Xiuwei, if you are still able to see I/O errors during LVM testing,
> does this patch fix them?
>
> drivers/md/dm-raid.c | 7 +++++++
> drivers/md/md.h | 1 +
> drivers/md/raid5.c | 6 ++++--
> 3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index 4bacdc499984..cac61d57e7e2 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -3831,6 +3831,7 @@ static void raid_presuspend(struct dm_target *ti)
> * resume, raid_postsuspend() is too late.
> */
> set_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
> + WRITE_ONCE(mddev->dm_suspending, 1);
>
> if (!reshape_interrupted(mddev))
> return;
> @@ -3847,6 +3848,9 @@ static void raid_presuspend(struct dm_target *ti)
> static void raid_presuspend_undo(struct dm_target *ti)
> {
> struct raid_set *rs = ti->private;
> + struct mddev *mddev = &rs->md;
> +
> + WRITE_ONCE(mddev->dm_suspending, 0);
>
> clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
> }
> @@ -3854,6 +3858,7 @@ static void raid_presuspend_undo(struct dm_target *ti)
> static void raid_postsuspend(struct dm_target *ti)
> {
> struct raid_set *rs = ti->private;
> + struct mddev *mddev = &rs->md;
>
> if (!test_and_set_bit(RT_FLAG_RS_SUSPENDED, &rs->runtime_flags)) {
> /*
> @@ -3864,6 +3869,8 @@ static void raid_postsuspend(struct dm_target *ti)
> mddev_suspend(&rs->md, false);
> rs->md.ro = MD_RDONLY;
> }
> + WRITE_ONCE(mddev->dm_suspending, 0);
> +
> }
>
> static void attempt_restore_of_faulty_devices(struct raid_set *rs)
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index ac84289664cd..e8d7332c5cb9 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -463,6 +463,7 @@ struct mddev {
> int delta_disks, new_level, new_layout;
> int new_chunk_sectors;
> int reshape_backwards;
> + int dm_suspending;
This patch looks fine, however, can you also optimize it by a new
flag instead a new int field ?
>
> struct md_thread __rcu *thread; /* management thread */
> struct md_thread __rcu *sync_thread; /* doing resync or reconstruct */
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 8854e024f311..d528263f92a3 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6042,8 +6042,10 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
> raid5_release_stripe(sh);
> out:
> if (ret == STRIPE_SCHEDULE_AND_RETRY && reshape_interrupted(mddev)) {
> - bi->bi_status = BLK_STS_RESOURCE;
> - ret = STRIPE_WAIT_RESHAPE;
> + if (!mddev_is_dm(mddev) || READ_ONCE(mddev->dm_suspending)) {
> + bi->bi_status = BLK_STS_RESOURCE;
> + ret = STRIPE_WAIT_RESHAPE;
> + }
> pr_err_ratelimited("dm-raid456: io across reshape position while reshape can't make progress");
> }
> return ret;
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid10: fix divide-by-zero in setup_geo() with zero far_copies
From: Yuhao Jiang @ 2026-04-28 8:37 UTC (permalink / raw)
To: yukuai
Cc: Junrui Luo, Song Liu, Li Nan, NeilBrown, Jonathan Brassow,
linux-raid, linux-kernel, stable
In-Reply-To: <282278bc-7d71-4049-89f4-a9f3968504dd@fnnas.com>
Hi Kuai,
Looks like different maintainers have different rules. :(
Can you send me the patchwork resource?
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
--
Yuhao Jiang
^ permalink raw reply
* Re: [PATCH] md/raid1: fix bio splitting in raid1 thread to avoid recursion and deadlock
From: Yu Kuai @ 2026-04-28 8:54 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, shli, neilb, linux-raid, linux-kernel,
yukuai
In-Reply-To: <20260427103446.300378-1-abd.masalkhi@gmail.com>
Hi,
在 2026/4/27 18:34, Abd-Alrhman Masalkhi 写道:
> Splitting a bio while executing in the raid1 thread can lead to
> recursion, as task->bio_list is NULL in this context.
>
> In addition, resubmitting an md_cloned_bio after splitting may lead to
> a deadlock if the array is suspended before the md driver calls
> percpu_ref_tryget_live(&mddev->active_io) on it's path to
> pers->make_request().
I don't understand, I agree this is problematic in the suspend case, but
what's wrong with task->bio_list being NULL? This can only cause the reverse
order because the split bio will submit first. However this is not a big deal
as this is the slow error patch.
If suspend is the only problem here, the simple fix is to add checking
in md_handle_request().
>
> Avoid splitting the bio in this context and require that it is either
> read in full or not at all.
>
> This prevents recursion and avoids potential deadlocks during array
> suspension.
>
> Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> I sent an email about this issue two days ago, but at the time I was not
> sure whether it was a real problem or a misunderstanding on my part.
>
> After further analysis, it appears that this issue can occur.
>
> Apologies for the earlier confusion, and thank you for your time.
>
> Abd-Alrhman
> ---
> drivers/md/raid1.c | 33 ++++++++++++++++++++++++---------
> 1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index cc9914bd15c1..14f6d6625811 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -607,7 +607,7 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>
> /* choose the first disk even if it has some bad blocks. */
> read_len = raid1_check_read_range(rdev, this_sector, &len);
> - if (read_len > 0) {
> + if (read_len > 0 && (!*max_sectors || read_len == r1_bio->sectors)) {
> update_read_sectors(conf, disk, this_sector, read_len);
> *max_sectors = read_len;
> return disk;
> @@ -704,8 +704,13 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
> }
>
> if (bb_disk != -1) {
> - *max_sectors = bb_read_len;
> - update_read_sectors(conf, bb_disk, this_sector, bb_read_len);
> + if (!*max_sectors || bb_read_len == r1_bio->sectors) {
> + *max_sectors = bb_read_len;
> + update_read_sectors(conf, bb_disk, this_sector,
> + bb_read_len);
> + } else {
> + bb_disk = -1;
> + }
> }
>
> return bb_disk;
> @@ -852,8 +857,9 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
> * disks and disks with bad blocks for now. Only pay attention to key disk
> * choice.
> *
> - * 3) If we've made it this far, now look for disks with bad blocks and choose
> - * the one with most number of sectors.
> + * 3) If we've made it this far and *max_sectors is 0 (i.e., we are tolerant
> + * of bad blocks), look for disks with bad blocks and choose the one with
> + * the most sectors.
> *
> * 4) If we are all the way at the end, we have no choice but to use a disk even
> * if it is write mostly.
> @@ -882,11 +888,13 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio,
> /*
> * If we are here it means we didn't find a perfectly good disk so
> * now spend a bit more time trying to find one with the most good
> - * sectors.
> + * sectors. but only if we are tolerant of bad blocks.
> */
> - disk = choose_bb_rdev(conf, r1_bio, max_sectors);
> - if (disk >= 0)
> - return disk;
> + if (!*max_sectors) {
> + disk = choose_bb_rdev(conf, r1_bio, max_sectors);
> + if (disk >= 0)
> + return disk;
> + }
>
> return choose_slow_rdev(conf, r1_bio, max_sectors);
> }
> @@ -1346,7 +1354,14 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> /*
> * make_request() can abort the operation when read-ahead is being
> * used and no empty request is available.
> + *
> + * If we allow splitting the bio while executing in the raid1 thread,
> + * we may end up recursing (current->bio_list is NULL), and we might
> + * also deadlock if we try to suspend the array, since we are
> + * resubmitting an md_cloned_bio. Therefore, we must be read either
> + * all the sectors or none.
> */
> + max_sectors = r1bio_existed;
> rdisk = read_balance(conf, r1_bio, &max_sectors);
> if (rdisk < 0) {
> /* couldn't find anywhere to read from */
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH 0/3] md/raid1: small cleanups in MD and raid1 drivers
From: Yu Kuai @ 2026-04-28 9:01 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song; +Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <20260423101303.48196-1-abd.masalkhi@gmail.com>
在 2026/4/23 18:13, Abd-Alrhman Masalkhi 写道:
> Hi,
>
> This small series contains three cleanups in the MD and raid1 drivers.
>
> Thanks,
> Abd-Alrhman
>
> Abd-Alrhman Masalkhi (3):
> md/raid1: replace wait loop with wait_event_idle() in
> raid1_write_request()
> md: use mddev_is_dm() instead of open-coding gendisk checks
> md: use ATTRIBUTE_GROUPS() for md default sysfs attributes
>
> drivers/md/md.c | 16 ++++------------
> drivers/md/raid1.c | 15 ++++-----------
> 2 files changed, 8 insertions(+), 23 deletions(-)
Applied to md-7.1
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid1: fix bio splitting in raid1 thread to avoid recursion and deadlock
From: Abd-Alrhman Masalkhi @ 2026-04-28 9:46 UTC (permalink / raw)
To: Yu Kuai, song, shli, neilb, linux-raid, linux-kernel, yukuai
In-Reply-To: <2cf6f585-a0de-4c84-9cfc-05e1f6fde549@fnnas.com>
Hi Kuai,
Thank you for the feedback.
On Tue, Apr 28, 2026 at 16:54 +0800, Yu Kuai wrote:
> Hi,
>
> 在 2026/4/27 18:34, Abd-Alrhman Masalkhi 写道:
>> Splitting a bio while executing in the raid1 thread can lead to
>> recursion, as task->bio_list is NULL in this context.
>>
>> In addition, resubmitting an md_cloned_bio after splitting may lead to
>> a deadlock if the array is suspended before the md driver calls
>> percpu_ref_tryget_live(&mddev->active_io) on it's path to
>> pers->make_request().
>
> I don't understand, I agree this is problematic in the suspend case, but
> what's wrong with task->bio_list being NULL? This can only cause the reverse
> order because the split bio will submit first. However this is not a big deal
> as this is the slow error patch.
>
> If suspend is the only problem here, the simple fix is to add checking
> in md_handle_request().
>
I meant that when current->bio_list is NULL, raid1_read_request()
can recurse into itself, as shown in the following trace-cmd output:
raid1_read_request() { <---
bio_submit_split_bioset() {
bio_split() {..}
bio_chain();
submit_bio_noacct_nocheck() {
__submit_bio() {
md_submit_bio() {
md_handle_request() {
raid1_make_request() {
raid1_read_request() { <---
md_account_bio() {
If this behavior is not an issue, I will follow your suggestion and
only add the check in md_handle_request().
>>
>> Avoid splitting the bio in this context and require that it is either
>> read in full or not at all.
>>
>> This prevents recursion and avoids potential deadlocks during array
>> suspension.
>>
>> Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> I sent an email about this issue two days ago, but at the time I was not
>> sure whether it was a real problem or a misunderstanding on my part.
>>
>> After further analysis, it appears that this issue can occur.
>>
>> Apologies for the earlier confusion, and thank you for your time.
>>
>> Abd-Alrhman
>> ---
>> drivers/md/raid1.c | 33 ++++++++++++++++++++++++---------
>> 1 file changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index cc9914bd15c1..14f6d6625811 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -607,7 +607,7 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>>
>> /* choose the first disk even if it has some bad blocks. */
>> read_len = raid1_check_read_range(rdev, this_sector, &len);
>> - if (read_len > 0) {
>> + if (read_len > 0 && (!*max_sectors || read_len == r1_bio->sectors)) {
>> update_read_sectors(conf, disk, this_sector, read_len);
>> *max_sectors = read_len;
>> return disk;
>> @@ -704,8 +704,13 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>> }
>>
>> if (bb_disk != -1) {
>> - *max_sectors = bb_read_len;
>> - update_read_sectors(conf, bb_disk, this_sector, bb_read_len);
>> + if (!*max_sectors || bb_read_len == r1_bio->sectors) {
>> + *max_sectors = bb_read_len;
>> + update_read_sectors(conf, bb_disk, this_sector,
>> + bb_read_len);
>> + } else {
>> + bb_disk = -1;
>> + }
>> }
>>
>> return bb_disk;
>> @@ -852,8 +857,9 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
>> * disks and disks with bad blocks for now. Only pay attention to key disk
>> * choice.
>> *
>> - * 3) If we've made it this far, now look for disks with bad blocks and choose
>> - * the one with most number of sectors.
>> + * 3) If we've made it this far and *max_sectors is 0 (i.e., we are tolerant
>> + * of bad blocks), look for disks with bad blocks and choose the one with
>> + * the most sectors.
>> *
>> * 4) If we are all the way at the end, we have no choice but to use a disk even
>> * if it is write mostly.
>> @@ -882,11 +888,13 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio,
>> /*
>> * If we are here it means we didn't find a perfectly good disk so
>> * now spend a bit more time trying to find one with the most good
>> - * sectors.
>> + * sectors. but only if we are tolerant of bad blocks.
>> */
>> - disk = choose_bb_rdev(conf, r1_bio, max_sectors);
>> - if (disk >= 0)
>> - return disk;
>> + if (!*max_sectors) {
>> + disk = choose_bb_rdev(conf, r1_bio, max_sectors);
>> + if (disk >= 0)
>> + return disk;
>> + }
>>
>> return choose_slow_rdev(conf, r1_bio, max_sectors);
>> }
>> @@ -1346,7 +1354,14 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>> /*
>> * make_request() can abort the operation when read-ahead is being
>> * used and no empty request is available.
>> + *
>> + * If we allow splitting the bio while executing in the raid1 thread,
>> + * we may end up recursing (current->bio_list is NULL), and we might
>> + * also deadlock if we try to suspend the array, since we are
>> + * resubmitting an md_cloned_bio. Therefore, we must be read either
>> + * all the sectors or none.
>> */
>> + max_sectors = r1bio_existed;
>> rdisk = read_balance(conf, r1_bio, &max_sectors);
>> if (rdisk < 0) {
>> /* couldn't find anywhere to read from */
>
> --
> Thansk,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: skip redundant raid_disks update when value is unchanged
From: Abd-Alrhman Masalkhi @ 2026-04-28 9:56 UTC (permalink / raw)
To: Yu Kuai, song; +Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <9748ccf8-7d4f-495f-8083-eb640008f073@fnnas.com>
On Tue, Apr 28, 2026 at 16:25 +0800, Yu Kuai wrote:
> Hi,
>
> 在 2026/4/25 16:58, Abd-Alrhman Masalkhi 写道:
>> Calling update_raid_disks() with the same value as the current one
>> can trigger unnecessary work. For example, RAID1 will reallocate
>> resources such as the mempool for r1bio.
>>
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> It returns -EINVAL for the same value. If silent success is preferred
>> instead, please let me know so I adjust its behavior.
>> ---
>> drivers/md/md.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 0e55639211f2..cb66c4ebbafa 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -4409,9 +4409,12 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
>> err = mddev_suspend_and_lock(mddev);
>> if (err)
>> return err;
>> - if (mddev->pers)
>> - err = update_raid_disks(mddev, n);
>> - else if (mddev->reshape_position != MaxSector) {
>> + if (mddev->pers) {
>> + if (n != mddev->raid_disks)
>> + err = update_raid_disks(mddev, n);
>> + else
>> + err = -EINVAL;
>
> Changing return value in this case is not expected, especially from
> success to failure.
>
I’ll adjust the behavior to return success instead.
>> + } else if (mddev->reshape_position != MaxSector) {
>> struct md_rdev *rdev;
>> int olddisks = mddev->raid_disks - mddev->delta_disks;
>>
>
> --
> Thansk,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md/raid1: fix len reuse across rdevs in choose_first_rdev()
From: Abd-Alrhman Masalkhi @ 2026-04-28 11:53 UTC (permalink / raw)
To: Yu Kuai, song, paul.e.luse, xni, yukuai; +Cc: linux-raid, linux-kernel
In-Reply-To: <5c71cb6b-d860-4bcb-a900-a27544be7a17@fnnas.com>
Hi Kaui,
On Tue, Apr 28, 2026 at 16:23 +0800, Yu Kuai wrote:
> Hi,
>
> 在 2026/4/26 17:35, Abd-Alrhman Masalkhi 写道:
>> choose_first_rdev() initializes the variable len before iterating over
>> all rdevs, but passes it by reference to raid1_check_read_range(), which
>> it might update *len and return 0 depending on the layout of the bad
>> block region. As a result, 'len' can be modified during the first
>> iteration and reused for subsequent rdevs, causing later devices to be
>> evaluated with an incorrect length value.
>>
>> Fixes: 31a73331752d3 ("md/raid1: factor out read_first_rdev() from read_balance()")
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> drivers/md/raid1.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index b549be9174bb..5f5dbf79c903 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -591,12 +591,12 @@ static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
>> int *max_sectors)
>> {
>> sector_t this_sector = r1_bio->sector;
>> - int len = r1_bio->sectors;
>> int disk;
>>
>> for (disk = 0 ; disk < conf->raid_disks * 2 ; disk++) {
>> struct md_rdev *rdev;
>> int read_len;
>> + int len = r1_bio->sectors;
>>
>> if (r1_bio->bios[disk] == IO_BLOCKED)
>> continue;
>
> This patch is wrong, choose_first_rdev() is used when raid1_should_read_first() is true,
> meaning the read overlaps an unsynced/resyncing area. Reset len can cause the problem that
> reading the same area can return different data.
>
Thank you for the detailed explanation. After carefully re-reading the
code and your feedback, I understand why the patch is wrong.
> --
> Thansk,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* [PATCH v2] md: skip redundant raid_disks update when value is unchanged
From: Abd-Alrhman Masalkhi @ 2026-04-28 13:05 UTC (permalink / raw)
To: song, yukuai; +Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
Calling update_raid_disks() with the same value as the current one
can trigger unnecessary work. For example, RAID1 will reallocate
resources such as the mempool for r1bio.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/md/md.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index ac71640ff3a8..21b8ad17685f 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4409,9 +4409,10 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
err = mddev_suspend_and_lock(mddev);
if (err)
return err;
- if (mddev->pers)
- err = update_raid_disks(mddev, n);
- else if (mddev->reshape_position != MaxSector) {
+ if (mddev->pers) {
+ if (n != mddev->raid_disks)
+ err = update_raid_disks(mddev, n);
+ } else if (mddev->reshape_position != MaxSector) {
struct md_rdev *rdev;
int olddisks = mddev->raid_disks - mddev->delta_disks;
--
2.43.0
^ permalink raw reply related
* [GIT PULL] md-7.1-20260428
From: Yu Kuai @ 2026-04-28 14:33 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, linux-raid, Song Liu, Li Nan, Xiao Ni,
Abd-Alrhman Masalkhi, Benjamin Marzinski, Junrui Luo, Keith Busch
Hi Jens,
Please consider pulling the following changes into your block-7.1
branch.
This pull request contains:
Bug Fixes:
- Fix a raid5 UAF on IO across the reshape position.
- Avoid failing RAID1/RAID10 devices for invalid IO errors.
- Fix RAID10 divide-by-zero when far_copies is zero.
- Restore bitmap grow through sysfs.
Cleanups:
- Use mddev_is_dm() instead of open-coding gendisk checks.
- Use ATTRIBUTE_GROUPS() for md default sysfs attributes.
- Replace open-coded wait loops with wait_event helpers.
Others:
- Add Xiao Ni as md/raid reviewer.
Thanks,
Kuai
---
The following changes since commit 0898a817621a2f0cddca8122d9b974003fe5036d:
cdrom, scsi: sr: propagate read-only status to block layer via set_disk_ro() (2026-04-27 15:52:51 -0600)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/mdraid/linux.git tags/md-7.1-20260428
for you to fetch changes up to 3b2f70eab5a2cd15e27b1447e66e45302b28ff2c:
md: use ATTRIBUTE_GROUPS() for md default sysfs attributes (2026-04-28 20:44:38 +0800)
----------------------------------------------------------------
Abd-Alrhman Masalkhi (5):
md: replace wait loop with wait_event() in md_handle_request()
md: use mddev_lock_nointr() in mddev_suspend_and_lock_nointr()
md/raid1: replace wait loop with wait_event_idle() in raid1_write_request()
md: use mddev_is_dm() instead of open-coding gendisk checks
md: use ATTRIBUTE_GROUPS() for md default sysfs attributes
Benjamin Marzinski (1):
md/raid5: Fix UAF on IO across the reshape position
Junrui Luo (1):
md/raid10: fix divide-by-zero in setup_geo() with zero far_copies
Keith Busch (1):
md/raid1,raid10: don't fail devices for invalid IO errors
Xiao Ni (1):
MAINTAINERS: Add Xiao Ni as md/raid reviewer
Yu Kuai (3):
md: factor bitmap creation away from sysfs handling
md/md-bitmap: split bitmap sysfs groups
md/md-bitmap: add a none backend for bitmap grow
MAINTAINERS | 1 +
drivers/md/md-bitmap.c | 131 ++++++++++++++++++++++++++++++----
drivers/md/md-bitmap.h | 2 +-
drivers/md/md-llbitmap.c | 7 +-
drivers/md/md.c | 182 ++++++++++++++++++++++++++---------------------
drivers/md/md.h | 6 +-
drivers/md/raid1-10.c | 7 +-
drivers/md/raid1.c | 15 ++--
drivers/md/raid10.c | 2 +
drivers/md/raid5.c | 7 +-
10 files changed, 251 insertions(+), 109 deletions(-)
^ permalink raw reply
* Re: [GIT PULL] md-7.1-20260428
From: Jens Axboe @ 2026-04-28 14:41 UTC (permalink / raw)
To: Yu Kuai
Cc: linux-block, linux-raid, Song Liu, Li Nan, Xiao Ni,
Abd-Alrhman Masalkhi, Benjamin Marzinski, Junrui Luo, Keith Busch
In-Reply-To: <20260428143340.1088943-1-yukuai@fnnas.com>
On 4/28/26 8:33 AM, Yu Kuai wrote:
> Hi Jens,
>
> Please consider pulling the following changes into your block-7.1
> branch.
>
> This pull request contains:
>
> Bug Fixes:
> - Fix a raid5 UAF on IO across the reshape position.
> - Avoid failing RAID1/RAID10 devices for invalid IO errors.
> - Fix RAID10 divide-by-zero when far_copies is zero.
> - Restore bitmap grow through sysfs.
>
> Cleanups:
> - Use mddev_is_dm() instead of open-coding gendisk checks.
> - Use ATTRIBUTE_GROUPS() for md default sysfs attributes.
> - Replace open-coded wait loops with wait_event helpers.
>
> Others:
> - Add Xiao Ni as md/raid reviewer.
Two notes:
1) Why are you rebasing the tree right before sending it? There
should be zero need to do that, please don't.
2) Please switch to https://patch.msgid.link/ for you Link tags,
if they are just links to the patch submission. That makes it
clear this is the case, just the patch. You can use lore for
actual bug reports etc.
Pulled, but please change the above two things going forward.
--
Jens Axboe
^ permalink raw reply
* [PATCH v2] dm-raid: only requeue bios when dm is suspending.
From: Benjamin Marzinski @ 2026-04-28 23:20 UTC (permalink / raw)
To: Yu Kuai, Song Liu
Cc: linux-raid, dm-devel, Yang Xiuwei, Xiao Ni, Li Nan, Nigel Croxon
returning DM_MAPIO_REQUEUE from the target map() function only requeues
the bio during noflush suspends. During regular operations or during
flushing suspends, it fails the bio. Failing the bio during flushing
suspends is the correct behavior here. We cannot handle the bio, and we
cannot suspends while it is outstanding. But during normal operations,
we should not push the bio back to dm. Instead, wait for the reshape
to be resumed.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
Changes from v1:
- Track the dm device's suspending state in mddev->flags instead of
adding a new integer to mddev.
drivers/md/dm-raid.c | 6 ++++++
drivers/md/md.h | 2 ++
drivers/md/raid5.c | 7 +++++--
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index c5dc083c7244..8f5a5e1342a9 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3831,6 +3831,7 @@ static void raid_presuspend(struct dm_target *ti)
* resume, raid_postsuspend() is too late.
*/
set_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
+ set_bit(MD_DM_SUSPENDING, &mddev->flags);
if (!reshape_interrupted(mddev))
return;
@@ -3847,13 +3848,16 @@ static void raid_presuspend(struct dm_target *ti)
static void raid_presuspend_undo(struct dm_target *ti)
{
struct raid_set *rs = ti->private;
+ struct mddev *mddev = &rs->md;
+ clear_bit(MD_DM_SUSPENDING, &mddev->flags);
clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
}
static void raid_postsuspend(struct dm_target *ti)
{
struct raid_set *rs = ti->private;
+ struct mddev *mddev = &rs->md;
if (!test_and_set_bit(RT_FLAG_RS_SUSPENDED, &rs->runtime_flags)) {
/*
@@ -3864,6 +3868,8 @@ static void raid_postsuspend(struct dm_target *ti)
mddev_suspend(&rs->md, false);
rs->md.ro = MD_RDONLY;
}
+ clear_bit(MD_DM_SUSPENDING, &mddev->flags);
+
}
static void attempt_restore_of_faulty_devices(struct raid_set *rs)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 52c378086046..9e5100609d12 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -346,6 +346,7 @@ struct md_cluster_operations;
* @MD_HAS_SUPERBLOCK: There is persistence sb in member disks.
* @MD_FAILLAST_DEV: Allow last rdev to be removed.
* @MD_SERIALIZE_POLICY: Enforce write IO is not reordered, just used by raid1.
+ * @MD_DM_SUSPENDING: This DM raid device is suspending.
*
* change UNSUPPORTED_MDDEV_FLAGS for each array type if new flag is added
*/
@@ -365,6 +366,7 @@ enum mddev_flags {
MD_HAS_SUPERBLOCK,
MD_FAILLAST_DEV,
MD_SERIALIZE_POLICY,
+ MD_DM_SUSPENDING,
};
enum mddev_sb_flags {
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0d76e82f4506..65ae7d8930fc 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6042,8 +6042,11 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
raid5_release_stripe(sh);
out:
if (ret == STRIPE_SCHEDULE_AND_RETRY && reshape_interrupted(mddev)) {
- bi->bi_status = BLK_STS_RESOURCE;
- ret = STRIPE_WAIT_RESHAPE;
+ if (!mddev_is_dm(mddev) ||
+ test_bit(MD_DM_SUSPENDING, &mddev->flags)) {
+ bi->bi_status = BLK_STS_RESOURCE;
+ ret = STRIPE_WAIT_RESHAPE;
+ }
pr_err_ratelimited("dm-raid456: io across reshape position while reshape can't make progress");
}
return ret;
--
2.53.0
^ permalink raw reply related
* [PATCH] md/raid5: Fix bio retry on interrupted reshape
From: Nigel Croxon @ 2026-04-29 11:10 UTC (permalink / raw)
To: song, yukuai, linux-raid
When a bio encounters LOC_INSIDE_RESHAPE during a reshape that is
interrupted (stopped or unable to progress), the code sets
bi->bi_status = BLK_STS_RESOURCE to signal the block layer for retry.
However, bio_endio() is never called, so the block layer never
receives the completion notification and the retry never happens.
This causes I/O to hang when a filesystem is layered over RAID5 and
reshape gets stuck.
Fix this by calling bio_endio(bi) before md_free_cloned_bio(bi) so
the block layer is properly notified of the BLK_STS_RESOURCE status
and can retry the request.
Tested stripes and stripe size conversions under load comparing
files multiple times during each conversion (i.e. MD reshape) on
ext4 after dropping caches degrading the RaidLV each time and
no data corruption.
Fixes: https://lwn.net/Articles/757123/
Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
---
drivers/md/raid5.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 6e79829c5acb..9a3475429ef4 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6217,6 +6217,7 @@ static bool raid5_make_request(struct mddev
*mddev, struct bio * bi)
mempool_free(ctx, conf->ctx_pool);
if (res == STRIPE_WAIT_RESHAPE) {
+ bio_endio(bi);
md_free_cloned_bio(bi);
return false;
}
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] md/raid5: Fix bio retry on interrupted reshape
From: Paul Menzel @ 2026-04-29 13:07 UTC (permalink / raw)
To: Nigel Croxon; +Cc: song, yukuai, linux-raid
In-Reply-To: <f861791b-d539-45ce-8575-5ec74d58d15d@redhat.com>
Dear Nigel,
Thank you for your patch.
Am 29.04.26 um 13:10 schrieb Nigel Croxon:
> When a bio encounters LOC_INSIDE_RESHAPE during a reshape that is
> interrupted (stopped or unable to progress), the code sets
> bi->bi_status = BLK_STS_RESOURCE to signal the block layer for retry.
> However, bio_endio() is never called, so the block layer never
> receives the completion notification and the retry never happens.
>
> This causes I/O to hang when a filesystem is layered over RAID5 and
> reshape gets stuck.
>
> Fix this by calling bio_endio(bi) before md_free_cloned_bio(bi) so
> the block layer is properly notified of the BLK_STS_RESOURCE status
> and can retry the request.
>
> Tested stripes and stripe size conversions under load comparing
> files multiple times during each conversion (i.e. MD reshape) on
> ext4 after dropping caches degrading the RaidLV each time and
I thought RaidLV misspelled Raid V (Raid 5), so should you resend, maybe
write it as RAID LV.
> no data corruption.
>
> Fixes: https://lwn.net/Articles/757123/
Which paragraph/comment exactly?
> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
> ---
> drivers/md/raid5.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 6e79829c5acb..9a3475429ef4 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6217,6 +6217,7 @@ static bool raid5_make_request(struct mddev
> *mddev, struct bio * bi)
>
> mempool_free(ctx, conf->ctx_pool);
> if (res == STRIPE_WAIT_RESHAPE) {
> + bio_endio(bi);
> md_free_cloned_bio(bi);
> return false;
> }
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* Suggest me a cost effective SATA SSD?
From: Andy Smith @ 2026-04-29 22:52 UTC (permalink / raw)
To: linux-raid
Hi,
I've inherited a system with a pair of these:
Model Family: Crucial/Micron Client SSDs
Device Model: CT4000BX500SSD1
User Capacity: 4,000,787,030,016 bytes [4.00 TB]
Their write performance is terrible. Struggling to get 20MB/s sequential
write. Their TBW count is low so it's not an issue of excessive write
cycles. I've searched around and established they are just really bad
SSDs. They also don't support SMART self-tests, which seems like a
really cheap thing to do.
I'm further confident that the problem lies with these SSDs because
there is a pair of much better SSDs in there and they perform as I would
expect. However, the use case for this storage is for lower cost so it's
not an option to just buy more of those.
So, could anyone suggest a decent low end (consumer/prosumer market) SSD
model that is known to work well without terrible firmware bugs under
Linux, preferably with power loss protection? Low write endurance is
fine. Capacity of 4TB ideally.
Thanks,
Andy
^ 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