* Re: LBS setup
From: Piergiorgio Sartor @ 2026-07-10 17:54 UTC (permalink / raw)
To: Wol; +Cc: Piergiorgio Sartor, linux-raid
In-Reply-To: <811c24a5-0c72-4b83-af19-609e42e12613@youngman.org.uk>
Hi Wol
thanks for the answer.
My question is about "what to do?".
Not much what happened.
The log says we should copy some size
from somewhere to somewhere else.
The docs says this should be done
with array at rest.
Is there any documentation about this?
Should we just ignore the log?
What should be done, if any?
The log:
md1: echo current LBS to md/logical_block_size to prevent data loss issues from LBS changes.
Note: After setting, array will not be assembled in old kernels (<= 6.18)
The docs:
logical_block_size
Configure the array’s logical block size in bytes.
This attribute is only supported for 1.x meta.
*Write the value before starting array.* (emphasis mine)
The final array LBS uses the maximum between this
configuration and LBS of all combined devices.
Note that LBS cannot exceed PAGE_SIZE before RAID
supports folio.
WARNING: Arrays created on new kernel cannot be
assembled at old kernel due to padding check,
Set module parameter ‘check_new_feature’ to
false to bypass, but data loss may occur.
The kernel is pretty new, actually 7.1.3,
from Fedora.
Old kernels is for me no problem, it would
be a problem losing the array.
There is no procedure explained, documentation
is, for me, quite dry.
It would be good to have some comments from
whom wrote it (and the code as well).
Thanks,
bye,
pg
On Fri, Jul 10, 2026 at 06:10:47PM +0100, Wol wrote:
> On 10/07/2026 17:20, Piergiorgio Sartor wrote:
> > Hi again,
> >
> > not sure, but has anybody comments
> > on the topic?
>
> I don't understand what you're asking, but I do think I remember the events
> you're talking about.
>
> Basically, some change accidentally slipped into the wild with a kernel,
> probably a 6.x, and people started creating kernels with the new kernel.
>
> Then, as people started upgrading kernels, they suddenly realised that this
> change affected the raid format, such that new kernels couldn't read old
> arrays, and vice versa. More importantly, and more dangerously, it led to
> arrays being trashed!
>
> What to do? They couldn't pull old kernels obviously, there were too many
> arrays out there. They couldn't pull new kernels, there were already some
> arrays out there.
>
> So they rushed out a fix, such that if a new kernel created an array, it
> flagged it as being a new array. If you booted a new kernel with an array
> that didn't have the flag IT WOULDN'T LOAD THE ARRAY. And they trusted to
> luck that people wouldn't downgrade an array and try to load a new array on
> an old kernel.
>
> So this now meant that all of a sudden new kernels wouldn't boot on old
> systems, until the user explicitly set the flag on the array to say "this is
> an old-style array". If they happened to be upgrading from one of the few
> early new-style kernels, they could set the flag to "this is a new-style
> array".
>
> Point is, this code was meant to make it very hard to upgrade your kernel
> without being prompted what sort of array you had, so that the kernel devs
> could try to ensure you didn't lose your array.
>
> A pretty awful way of doing it, but nobody could think of anything better.
>
> Hopefully that explains what's going on there, and you'll be able to work
> out your own answer from this information.
>
> Cheers,
> Wol
--
piergiorgio
^ permalink raw reply
* Re: LBS setup
From: Wol @ 2026-07-10 17:10 UTC (permalink / raw)
To: Piergiorgio Sartor; +Cc: linux-raid
In-Reply-To: <alEb50Nmqt8IW2Nu@lazy.lzy>
On 10/07/2026 17:20, Piergiorgio Sartor wrote:
> Hi again,
>
> not sure, but has anybody comments
> on the topic?
I don't understand what you're asking, but I do think I remember the
events you're talking about.
Basically, some change accidentally slipped into the wild with a kernel,
probably a 6.x, and people started creating kernels with the new kernel.
Then, as people started upgrading kernels, they suddenly realised that
this change affected the raid format, such that new kernels couldn't
read old arrays, and vice versa. More importantly, and more dangerously,
it led to arrays being trashed!
What to do? They couldn't pull old kernels obviously, there were too
many arrays out there. They couldn't pull new kernels, there were
already some arrays out there.
So they rushed out a fix, such that if a new kernel created an array, it
flagged it as being a new array. If you booted a new kernel with an
array that didn't have the flag IT WOULDN'T LOAD THE ARRAY. And they
trusted to luck that people wouldn't downgrade an array and try to load
a new array on an old kernel.
So this now meant that all of a sudden new kernels wouldn't boot on old
systems, until the user explicitly set the flag on the array to say
"this is an old-style array". If they happened to be upgrading from one
of the few early new-style kernels, they could set the flag to "this is
a new-style array".
Point is, this code was meant to make it very hard to upgrade your
kernel without being prompted what sort of array you had, so that the
kernel devs could try to ensure you didn't lose your array.
A pretty awful way of doing it, but nobody could think of anything better.
Hopefully that explains what's going on there, and you'll be able to
work out your own answer from this information.
Cheers,
Wol
^ permalink raw reply
* Re: LBS setup
From: Wol @ 2026-07-10 20:43 UTC (permalink / raw)
To: Piergiorgio Sartor; +Cc: linux-raid
In-Reply-To: <alExwDILVwd3nVvK@lazy.lzy>
On 10/07/2026 18:54, Piergiorgio Sartor wrote:
> Hi Wol
>
> thanks for the answer.
>
> My question is about "what to do?".
> Not much what happened.
>
> The log says we should copy some size
> from somewhere to somewhere else.
> The docs says this should be done
> with array at rest.
>
> Is there any documentation about this?
> Should we just ignore the log?
> What should be done, if any?
>
> The log:
>
> md1: echo current LBS to md/logical_block_size to prevent data loss issues from LBS changes.
> Note: After setting, array will not be assembled in old kernels (<= 6.18)
Ah. Personally I wouldn't do anything, then. Are you using any systems
with kernel 6.18 or earlier?
Are you likely to use any kernels 6.18 or earlier in the future?
If the answer to both questions is no, then you're not going to have any
trouble ...
Cheers,
Wol
^ permalink raw reply
* [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
From: Chen Cheng @ 2026-07-11 2:51 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
sb_flags is checked without a lock in md, raid1, raid5, and raid10.
KCSAN reports these reads as data races.
The write side uses atomic bit ops.
The read side still has plain loads in a few places.
Use READ_ONCE() for the lockless reads of sb_flags.
KCSAN reports #1:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start
write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
md_write_start+0x5dd/0x910
raid10_make_request+0x9b/0x1080
md_handle_request+0x4a2/0xa40
[........]
read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
md_check_recovery+0x574/0x900
raid10d+0xb7/0x2950
[........]
KCSAN reports #2:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start
write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
md_write_start+0x5dd/0x910
raid1_make_request+0x141/0x1990
[........]
read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
md_check_recovery+0x574/0x900
raid1d+0xcc/0x3840
[........]
value changed: 0x0000000000000002 -> 0x0000000000000006
KCSAN reports #3:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold
write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
md_do_sync.cold+0x3f6/0x1686
[........]
read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
md_check_recovery+0x16d/0x900
raid1d+0xcc/0x3840
[........]
value changed: 0x0000000000000000 -> 0x0000000000000002
KCSAN reports #4:
======================================
BUG: KCSAN: data-race in md_do_sync.cold / raid5d
write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
md_do_sync.cold+0x3f6/0x1686
md_thread+0x15a/0x2d0
[........]
read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
raid5d+0x7f9/0xba0
md_thread+0x15a/0x2d0
[........]
value changed: 0x0000000000000000 -> 0x0000000000000002
Fixes: 2953079c692d ("md: separate flags for superblock changes")
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
v1 -> v2
- pass checkpatch.pl check.
---
drivers/md/md.c | 8 ++++----
drivers/md/raid1.c | 2 +-
drivers/md/raid10.c | 4 ++--
drivers/md/raid5.c | 7 ++++---
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..e72bf2010287 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
* via sysfs - until a lack of spares is confirmed.
*/
set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
- if (mddev->sb_flags)
+ if (READ_ONCE(mddev->sb_flags))
md_update_sb(mddev, 0);
if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
!mddev->bitmap_info.offset)
md_bitmap_set_none(mddev);
@@ -7024,11 +7024,11 @@ static void __md_stop_writes(struct mddev *mddev)
mddev->bitmap_ops->flush(mddev);
}
if (md_is_rdwr(mddev) &&
((!mddev->in_sync && !mddev_is_clustered(mddev)) ||
- mddev->sb_flags)) {
+ READ_ONCE(mddev->sb_flags))) {
/* mark array as shutdown cleanly */
if (!mddev_is_clustered(mddev))
mddev->in_sync = 1;
md_update_sb(mddev, 1);
}
@@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
/*
* MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
* active, and no action is needed for now.
* All other MD_SB_* flags require to update the superblock.
*/
- if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1<<MD_SB_CHANGE_PENDING))
return true;
/*
* If the array is not using external metadata and there has been no data
* written for some time, then the array's status needs to be set to
@@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
spin_lock(&mddev->lock);
set_in_sync(mddev);
spin_unlock(&mddev->lock);
}
- if (mddev->sb_flags)
+ if (READ_ONCE(mddev->sb_flags))
md_update_sb(mddev, 0);
/*
* Never start a new sync thread if MD_RECOVERY_RUNNING is
* still set.
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 29b58583e381..bd6808656edb 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2738,11 +2738,11 @@ static void raid1d(struct md_thread *thread)
handle_read_error(conf, r1_bio);
else
WARN_ON_ONCE(1);
cond_resched();
- if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
md_check_recovery(mddev);
}
blk_finish_plug(&plug);
}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index adaf9e432e25..3ffa5a19964d 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3030,11 +3030,11 @@ static void raid10d(struct md_thread *thread)
handle_read_error(mddev, r10_bio);
else
WARN_ON_ONCE(1);
cond_resched();
- if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
md_check_recovery(mddev);
}
blk_finish_plug(&plug);
}
@@ -4698,11 +4698,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
else
mddev->curr_resync_completed = conf->reshape_progress;
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
- wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
+ wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
test_bit(MD_RECOVERY_INTR, &mddev->recovery));
if (test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
allow_barrier(conf);
return sectors_done;
}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ded6a69f7795..cb58b4353995 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -1219,11 +1219,11 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
break;
if (bad < 0) {
set_bit(BlockedBadBlocks, &rdev->flags);
if (!conf->mddev->external &&
- conf->mddev->sb_flags) {
+ READ_ONCE(conf->mddev->sb_flags)) {
/* It is very unlikely, but we might
* still need to write out the
* bad block log - better give it
* a chance*/
md_check_recovery(conf->mddev);
@@ -6469,11 +6469,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
rdev->recovery_offset = sector_nr;
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
- wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
+ wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
test_bit(MD_RECOVERY_INTR, &mddev->recovery));
if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
return 0;
spin_lock_irq(&conf->device_lock);
conf->reshape_safe = mddev->reshape_position;
@@ -6913,11 +6913,12 @@ static void raid5d(struct md_thread *thread)
conf->temp_inactive_list);
if (!batch_size && !released)
break;
handled += batch_size;
- if (mddev->sb_flags & ~(1 << MD_SB_CHANGE_PENDING)) {
+ if (READ_ONCE(mddev->sb_flags) &
+ ~(1 << MD_SB_CHANGE_PENDING)) {
spin_unlock_irq(&conf->device_lock);
md_check_recovery(mddev);
spin_lock_irq(&conf->device_lock);
}
}
--
2.54.0
^ permalink raw reply related
* Re: [RFC PATCH] md,dm: caller-owned memalloc_noio token in mddev_suspend()/resume()
From: Chen Cheng @ 2026-07-11 3:53 UTC (permalink / raw)
To: yukuai, linux-raid, dm-devel; +Cc: linux-kernel
In-Reply-To: <da94b991-fe62-4a8b-b448-c4ee226b6b4f@fygo.io>
在 2026/7/5 21:10, yu kuai 写道:
> Hi,
>
> 在 2026/6/24 18:13, Chen Cheng 写道:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> Save token as mddev-scoped in mddev->noio_flags cause PF_MEMALLOC_NOIO
>> leak into task A, while task B restores a token that it never saved.
>>
>> scenario:
>>
>> task A mddev task B
>> ====== ======= ============
>> write suspend_lo
>> mddev_suspend()
>> suspended == 0
>> drain active_io
>> suspended = 1
>> A: noio_A = memalloc_noio_save()
>> A returns with PF_MEMALLOC_NOIO set
>>
>> write suspend_hi
>> mddev_suspend()
>> suspended == 1
>> suspended = 2
>> B returns
>> (no save)
>>
>> mddev_resume()
>> suspended = 1
>> not last resume
>> A returns
>> A still has PF_MEMALLOC_NOIO <-- leaked
>>
>> mddev_resume()
>> suspended = 0
>> memalloc_noio_restore(noio_A)
>> (restores A's token in B)
>>
>> Fixed by:
>> - return each caller's noio_flags from mddev_suspend()
>> - pass that token back into mddev_resume()
>> - update the suspend-and-lock helpers to carry the token
>> - store the token in struct raid_set for dm-raid paths where suspend
>> and resume are paired across callbacks
>>
>> Validation:
>> repeatedly updates the array's suspend_lo and suspend_hi sysfs from many
>> concurrent userspace workers. That makes multiple tasks to call
>> mddev_suspend()/mddev_resume() concurrently.
>>
>> Each worker:
>> - reads its initial /proc/self/stat flags and verifies that PF_MEMALLOC_NOIO is not already
>> set
>> - writes 0 to either suspend_lo or suspend_hi
>> - immediately reads its own task flags again
>> - reports success if flags & PF_MEMALLOC_NOIO is true after the write returns
>>
>> Link: https://github.com/chencheng-fnnas/reproducer/blob/main/repro-md-noio-token-leak.sh
>>
>> Fixes: 78f57ef9d50a ("md: use memalloc scope APIs in mddev_suspend()/mddev_resume()")
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/dm-raid.c | 7 ++--
>> drivers/md/md-autodetect.c | 5 ++-
>> drivers/md/md-bitmap.c | 12 +++---
>> drivers/md/md.c | 85 ++++++++++++++++++++++----------------
>> drivers/md/md.h | 23 ++++++-----
>> drivers/md/raid5-cache.c | 11 +++--
>> drivers/md/raid5.c | 25 ++++++-----
>> 7 files changed, 97 insertions(+), 71 deletions(-)
>>
>> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
>> index 8f5a5e1342a9..d89207e3722a 100644
>> --- a/drivers/md/dm-raid.c
>> +++ b/drivers/md/dm-raid.c
>> @@ -239,10 +239,11 @@ struct raid_set {
>> int raid_disks;
>> int delta_disks;
>> int data_offset;
>> int raid10_copies;
>> int requested_bitmap_chunk_sectors;
>> + unsigned int suspend_noio_flags;
>>
>> struct mddev md;
>> struct raid_type *raid_type;
>>
>> sector_t array_sectors;
>> @@ -3251,11 +3252,11 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>> /* Start raid set read-only and assumed clean to change in raid_resume() */
>> rs->md.ro = MD_RDONLY;
>> rs->md.in_sync = 1;
>>
>> /* Has to be held on running the array */
>> - mddev_suspend_and_lock_nointr(&rs->md);
>> + mddev_suspend_and_lock_nointr(&rs->md, &rs->suspend_noio_flags);
>>
>> /* Keep array frozen until resume. */
>> md_frozen_sync_thread(&rs->md);
>>
>> r = md_run(&rs->md);
>> @@ -3863,11 +3864,11 @@ static void raid_postsuspend(struct dm_target *ti)
>> /*
>> * sync_thread must be stopped during suspend, and writes have
>> * to be stopped before suspending to avoid deadlocks.
>> */
>> md_stop_writes(&rs->md);
>> - mddev_suspend(&rs->md, false);
>> + mddev_suspend(&rs->md, false, &rs->suspend_noio_flags);
>> rs->md.ro = MD_RDONLY;
>> }
>> clear_bit(MD_DM_SUSPENDING, &mddev->flags);
>>
>> }
>> @@ -4141,11 +4142,11 @@ static void raid_resume(struct dm_target *ti)
>> lockdep_is_held(&mddev->reconfig_mutex)));
>> clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
>> mddev->ro = MD_RDWR;
>> mddev->in_sync = 0;
>> md_unfrozen_sync_thread(mddev);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, rs->suspend_noio_flags);
>> }
>> }
>
> For mdraid, changes looks fine. However, for dm-raid, for example:
>
> dmsetup suspend ...
>
> In this case, array will be suspended while task returned to user. Take a look at
> the commit to introduce memalloc_noio_save(), it's supposed to be called for mdraid
> arrays to allocate memory while array is suspended, I don't see why dm-raid need it.
> So I'll suggest just bypass the memalloc_noio_save() for dm-raid where mddev_suspend()
> is not paired with mddev_resume() in the same task context.
>
Hi Kuai.
I check the commit you mentioned., and I truelly misused
memalloc_noio_save() in dmraid side, really thanks for point-out.
thanks,
cheng.
>>
>> static struct target_type raid_target = {
>> .name = "raid",
>> diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
>> index 4b80165afd23..58e062cd0580 100644
>> --- a/drivers/md/md-autodetect.c
>> +++ b/drivers/md/md-autodetect.c
>> @@ -126,10 +126,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
>> dev_t devices[MD_SB_DISKS + 1], mdev;
>> struct mdu_array_info_s ainfo = { };
>> struct mddev *mddev;
>> int err = 0, i;
>> char name[16];
>> + unsigned int noio_flags;
>>
>> if (args->partitioned) {
>> mdev = MKDEV(mdp_major, args->minor << MdpMinorShift);
>> sprintf(name, "md_d%d", args->minor);
>> } else {
>> @@ -173,11 +174,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
>> if (IS_ERR(mddev)) {
>> pr_err("md: md_alloc failed - cannot start array %s\n", name);
>> return;
>> }
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err) {
>> pr_err("md: failed to lock array %s\n", name);
>> goto out_mddev_put;
>> }
>>
>> @@ -219,11 +220,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
>> if (!err)
>> err = do_md_run(mddev);
>> if (err)
>> pr_warn("md: starting %s failed\n", name);
>> out_unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> out_mddev_put:
>> mddev_put(mddev);
>> }
>>
>> static int __init raid_setup(char *str)
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 028b9ca8ce52..74b7f569a3f4 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -2620,13 +2620,14 @@ location_show(struct mddev *mddev, char *page)
>> }
>>
>> static ssize_t
>> location_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> + unsigned int noio_flags;
>> int rv;
>>
>> - rv = mddev_suspend_and_lock(mddev);
>> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (rv)
>> return rv;
>>
>> if (mddev->pers) {
>> if (mddev->recovery || mddev->sync_thread) {
>> @@ -2711,11 +2712,11 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
>> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
>> md_wakeup_thread(mddev->thread);
>> }
>> rv = 0;
>> out:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> if (rv)
>> return rv;
>> return len;
>>
>> merge_err:
>> @@ -2831,17 +2832,18 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> unsigned long backlog;
>> unsigned long old_mwb = mddev->bitmap_info.max_write_behind;
>> struct md_rdev *rdev;
>> bool has_write_mostly = false;
>> + unsigned int noio_flags;
>> int rv = kstrtoul(buf, 10, &backlog);
>> if (rv)
>> return rv;
>> if (backlog > COUNTER_MAX)
>> return -EINVAL;
>>
>> - rv = mddev_suspend_and_lock(mddev);
>> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (rv)
>> return rv;
>>
>> /*
>> * Without write mostly device, it doesn't make sense to set
>> @@ -2854,11 +2856,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
>> }
>> }
>> if (!has_write_mostly) {
>> pr_warn_ratelimited("%s: can't set backlog, no write mostly device available\n",
>> mdname(mddev));
>> - mddev_unlock(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return -EINVAL;
>> }
>>
>> mddev->bitmap_info.max_write_behind = backlog;
>> if (!backlog && mddev->serial_info_pool) {
>> @@ -2871,11 +2873,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
>> mddev_create_serial_pool(mddev, rdev);
>> }
>> if (old_mwb != backlog)
>> bitmap_update_sb(mddev->bitmap);
>>
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return len;
>> }
>>
>> static struct md_sysfs_entry bitmap_backlog =
>> __ATTR(backlog, S_IRUGO|S_IWUSR, backlog_show, backlog_store);
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 1377c407614c..86d938dee50a 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -459,11 +459,12 @@ static void md_submit_bio(struct bio *bio)
>>
>> /*
>> * Make sure no new requests are submitted to the device, and any requests that
>> * have been submitted are completely handled.
>> */
>> -int mddev_suspend(struct mddev *mddev, bool interruptible)
>> +int mddev_suspend(struct mddev *mddev, bool interruptible,
>> + unsigned int *noio_flags)
>> {
>> int err = 0;
>>
>> /*
>> * hold reconfig_mutex to wait for normal io will deadlock, because
>> @@ -478,10 +479,11 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
>> mutex_lock(&mddev->suspend_mutex);
>> if (err)
>> return err;
>>
>> if (mddev->suspended) {
>> + *noio_flags = memalloc_noio_save();
>> WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
>> mutex_unlock(&mddev->suspend_mutex);
>> return 0;
>> }
>>
>> @@ -515,31 +517,30 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
>> * prevent deadlock.
>> */
>> WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
>>
>> /* restrict memory reclaim I/O during raid array is suspend */
>> - mddev->noio_flag = memalloc_noio_save();
>> + *noio_flags = memalloc_noio_save();
>>
>> mutex_unlock(&mddev->suspend_mutex);
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(mddev_suspend);
>>
>> -static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
>> +static void __mddev_resume(struct mddev *mddev, bool recovery_needed,
>> + unsigned int noio_flags)
>> {
>> lockdep_assert_not_held(&mddev->reconfig_mutex);
>>
>> mutex_lock(&mddev->suspend_mutex);
>> + memalloc_noio_restore(noio_flags);
>> WRITE_ONCE(mddev->suspended, mddev->suspended - 1);
>> if (mddev->suspended) {
>> mutex_unlock(&mddev->suspend_mutex);
>> return;
>> }
>>
>> - /* entred the memalloc scope from mddev_suspend() */
>> - memalloc_noio_restore(mddev->noio_flag);
>> -
>> percpu_ref_resurrect(&mddev->active_io);
>> wake_up(&mddev->sb_wait);
>>
>> if (recovery_needed)
>> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
>> @@ -547,13 +548,13 @@ static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
>> md_wakeup_thread(mddev->sync_thread); /* possibly kick off a reshape */
>>
>> mutex_unlock(&mddev->suspend_mutex);
>> }
>>
>> -void mddev_resume(struct mddev *mddev)
>> +void mddev_resume(struct mddev *mddev, unsigned int noio_flags)
>> {
>> - return __mddev_resume(mddev, true);
>> + return __mddev_resume(mddev, true, noio_flags);
>> }
>> EXPORT_SYMBOL_GPL(mddev_resume);
>>
>> /* sync bdev before setting device to readonly or stopping raid*/
>> static int mddev_set_closing_and_sync_blockdev(struct mddev *mddev, int opener_num)
>> @@ -3737,10 +3738,11 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
>> {
>> struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr);
>> struct md_rdev *rdev = container_of(kobj, struct md_rdev, kobj);
>> struct kernfs_node *kn = NULL;
>> bool suspend = false;
>> + unsigned int noio_flags = 0;
>> ssize_t rv;
>> struct mddev *mddev = READ_ONCE(rdev->mddev);
>>
>> if (!entry->store)
>> return -EIO;
>> @@ -3756,17 +3758,17 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
>> cmd_match(page, "writemostly") ||
>> cmd_match(page, "-writemostly"))
>> suspend = true;
>> }
>>
>> - rv = suspend ? mddev_suspend_and_lock(mddev) : mddev_lock(mddev);
>> + rv = suspend ? mddev_suspend_and_lock(mddev, &noio_flags) : mddev_lock(mddev);
>> if (!rv) {
>> if (rdev->mddev == NULL)
>> rv = -ENODEV;
>> else
>> rv = entry->store(rdev, page, length);
>> - suspend ? mddev_unlock_and_resume(mddev) : mddev_unlock(mddev);
>> + suspend ? mddev_unlock_and_resume(mddev, noio_flags) : mddev_unlock(mddev);
>> }
>>
>> if (kn)
>> sysfs_unbreak_active_protection(kn);
>>
>> @@ -4049,15 +4051,16 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>> size_t slen = len;
>> struct md_personality *pers, *oldpers;
>> long level;
>> void *priv, *oldpriv;
>> struct md_rdev *rdev;
>> + unsigned int noio_flags;
>>
>> if (slen == 0 || slen >= sizeof(clevel))
>> return -EINVAL;
>>
>> - rv = mddev_suspend_and_lock(mddev);
>> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (rv)
>> return rv;
>>
>> if (mddev->pers == NULL) {
>> memcpy(mddev->clevel, buf, slen);
>> @@ -4231,11 +4234,11 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>> md_update_sb(mddev, 1);
>> sysfs_notify_dirent_safe(mddev->sysfs_level);
>> md_new_event();
>> rv = len;
>> out_unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return rv;
>> }
>>
>> static struct md_sysfs_entry md_level =
>> __ATTR(level, S_IRUGO|S_IWUSR, level_show, level_store);
>> @@ -4410,17 +4413,18 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks);
>>
>> static ssize_t
>> raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> unsigned int n;
>> + unsigned int noio_flags;
>> int err;
>>
>> err = kstrtouint(buf, 10, &n);
>> if (err < 0)
>> return err;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> if (mddev->pers) {
>> if (n != mddev->raid_disks)
>> err = update_raid_disks(mddev, n);
>> @@ -4442,11 +4446,11 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
>> mddev->raid_disks = n;
>> mddev->reshape_backwards = (mddev->delta_disks < 0);
>> } else
>> mddev->raid_disks = n;
>> out_unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return err ? err : len;
>> }
>> static struct md_sysfs_entry md_raid_disks =
>> __ATTR(raid_disks, S_IRUGO|S_IWUSR, raid_disks_show, raid_disks_store);
>>
>> @@ -4822,10 +4826,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
>> char *e;
>> int major = simple_strtoul(buf, &e, 10);
>> int minor;
>> dev_t dev;
>> struct md_rdev *rdev;
>> + unsigned int noio_flags;
>> int err;
>>
>> if (!*buf || *e != ':' || !e[1] || e[1] == '\n')
>> return -EINVAL;
>> minor = simple_strtoul(e+1, &e, 10);
>> @@ -4834,11 +4839,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
>> dev = MKDEV(major, minor);
>> if (major != MAJOR(dev) ||
>> minor != MINOR(dev))
>> return -EOVERFLOW;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> if (mddev->persistent) {
>> rdev = md_import_device(dev, mddev->major_version,
>> mddev->minor_version);
>> @@ -4855,18 +4860,18 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
>> rdev = md_import_device(dev, -2, -1);
>> else
>> rdev = md_import_device(dev, -1, -1);
>>
>> if (IS_ERR(rdev)) {
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return PTR_ERR(rdev);
>> }
>> err = bind_rdev_to_array(rdev, mddev);
>> out:
>> if (err)
>> export_rdev(rdev);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> if (!err)
>> md_new_event();
>> return err ? err : len;
>> }
>>
>> @@ -5257,28 +5262,29 @@ static int mddev_start_reshape(struct mddev *mddev)
>> static ssize_t
>> action_store(struct mddev *mddev, const char *page, size_t len)
>> {
>> int ret;
>> enum sync_action action;
>> + unsigned int noio_flags = 0;
>>
>> if (!mddev->pers || !mddev->pers->sync_request)
>> return -EINVAL;
>>
>> action = md_sync_action_by_name(page);
>> retry:
>> if (work_busy(&mddev->sync_work))
>> flush_work(&mddev->sync_work);
>>
>> ret = (action == ACTION_RESHAPE) ?
>> - mddev_suspend_and_lock(mddev) :
>> + mddev_suspend_and_lock(mddev, &noio_flags) :
>> mddev_lock(mddev);
>> if (ret)
>> return ret;
>>
>> if (work_busy(&mddev->sync_work)) {
>> if (action == ACTION_RESHAPE)
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> else
>> mddev_unlock(mddev);
>> goto retry;
>> }
>>
>> @@ -5349,11 +5355,11 @@ action_store(struct mddev *mddev, const char *page, size_t len)
>> sysfs_notify_dirent_safe(mddev->sysfs_action);
>> ret = len;
>>
>> out:
>> if (action == ACTION_RESHAPE)
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> else
>> mddev_unlock(mddev);
>> return ret;
>> }
>>
>> @@ -5640,24 +5646,25 @@ suspend_lo_show(struct mddev *mddev, char *page)
>>
>> static ssize_t
>> suspend_lo_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> unsigned long long new;
>> + unsigned int noio_flags;
>> int err;
>>
>> err = kstrtoull(buf, 10, &new);
>> if (err < 0)
>> return err;
>> if (new != (sector_t)new)
>> return -EINVAL;
>>
>> - err = mddev_suspend(mddev, true);
>> + err = mddev_suspend(mddev, true, &noio_flags);
>> if (err)
>> return err;
>>
>> WRITE_ONCE(mddev->suspend_lo, new);
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>>
>> return len;
>> }
>> static struct md_sysfs_entry md_suspend_lo =
>> __ATTR(suspend_lo, S_IRUGO|S_IWUSR, suspend_lo_show, suspend_lo_store);
>> @@ -5671,24 +5678,25 @@ suspend_hi_show(struct mddev *mddev, char *page)
>>
>> static ssize_t
>> suspend_hi_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> unsigned long long new;
>> + unsigned int noio_flags;
>> int err;
>>
>> err = kstrtoull(buf, 10, &new);
>> if (err < 0)
>> return err;
>> if (new != (sector_t)new)
>> return -EINVAL;
>>
>> - err = mddev_suspend(mddev, true);
>> + err = mddev_suspend(mddev, true, &noio_flags);
>> if (err)
>> return err;
>>
>> WRITE_ONCE(mddev->suspend_hi, new);
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>>
>> return len;
>> }
>> static struct md_sysfs_entry md_suspend_hi =
>> __ATTR(suspend_hi, S_IRUGO|S_IWUSR, suspend_hi_show, suspend_hi_store);
>> @@ -5928,19 +5936,20 @@ static ssize_t serialize_policy_show(struct mddev *mddev, char *page)
>> static ssize_t
>> serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> int err;
>> bool value;
>> + unsigned int noio_flags;
>>
>> err = kstrtobool(buf, &value);
>> if (err)
>> return err;
>>
>> if (value == test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
>> return len;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> if (mddev->pers == NULL || (mddev->pers->head.id != ID_RAID1)) {
>> pr_err("md: serialize_policy is only effective for raid1\n");
>> err = -EINVAL;
>> @@ -5953,11 +5962,11 @@ serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
>> } else {
>> mddev_destroy_serial_pool(mddev, NULL);
>> clear_bit(MD_SERIALIZE_POLICY, &mddev->flags);
>> }
>> unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return err ?: len;
>> }
>>
>> static struct md_sysfs_entry md_serialize_policy =
>> __ATTR(serialize_policy, S_IRUGO | S_IWUSR, serialize_policy_show,
>> @@ -6263,21 +6272,22 @@ EXPORT_SYMBOL_GPL(mddev_stack_new_rdev);
>>
>> /* update the optimal I/O size after a reshape */
>> void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes)
>> {
>> struct queue_limits lim;
>> + unsigned int noio_flags;
>>
>> if (mddev_is_dm(mddev))
>> return;
>>
>> /* don't bother updating io_opt if we can't suspend the array */
>> - if (mddev_suspend(mddev, false) < 0)
>> + if (mddev_suspend(mddev, false, &noio_flags) < 0)
>> return;
>> lim = queue_limits_start_update(mddev->gendisk->queue);
>> lim.io_opt = lim.io_min * nr_stripes;
>> queue_limits_commit_update(mddev->gendisk->queue, &lim);
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>> }
>> EXPORT_SYMBOL_GPL(mddev_update_io_opt);
>>
>> static void mddev_delayed_delete(struct work_struct *ws)
>> {
>> @@ -7255,10 +7265,11 @@ static void autorun_array(struct mddev *mddev)
>> */
>> static void autorun_devices(int part)
>> {
>> struct md_rdev *rdev0, *rdev, *tmp;
>> struct mddev *mddev;
>> + unsigned int noio_flags;
>>
>> pr_info("md: autorun ...\n");
>> while (!list_empty(&pending_raid_disks)) {
>> int unit;
>> dev_t dev;
>> @@ -7295,27 +7306,27 @@ static void autorun_devices(int part)
>>
>> mddev = md_alloc(dev, NULL);
>> if (IS_ERR(mddev))
>> break;
>>
>> - if (mddev_suspend_and_lock(mddev))
>> + if (mddev_suspend_and_lock(mddev, &noio_flags))
>> pr_warn("md: %s locked, cannot run\n", mdname(mddev));
>> else if (mddev->raid_disks || mddev->major_version
>> || !list_empty(&mddev->disks)) {
>> pr_warn("md: %s already running, cannot run %pg\n",
>> mdname(mddev), rdev0->bdev);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> } else {
>> pr_debug("md: created %s\n", mdname(mddev));
>> mddev->persistent = 1;
>> rdev_for_each_list(rdev, tmp, &candidates) {
>> list_del_init(&rdev->same_set);
>> if (bind_rdev_to_array(rdev, mddev))
>> export_rdev(rdev);
>> }
>> autorun_array(mddev);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> }
>> /* on success, candidates will be empty, on error
>> * it won't...
>> */
>> rdev_for_each_list(rdev, tmp, &candidates) {
>> @@ -8329,10 +8340,11 @@ static int __md_set_array_info(struct mddev *mddev, void __user *argp)
>>
>> static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
>> unsigned int cmd, unsigned long arg)
>> {
>> int err = 0;
>> + unsigned int noio_flags = 0;
>> void __user *argp = (void __user *)arg;
>> struct mddev *mddev = NULL;
>>
>> err = md_ioctl_valid(cmd);
>> if (err)
>> @@ -8380,11 +8392,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
>> }
>>
>> if (!md_is_rdwr(mddev))
>> flush_work(&mddev->sync_work);
>>
>> - err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev) :
>> + err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev, &noio_flags) :
>> mddev_lock(mddev);
>> if (err) {
>> pr_debug("md: ioctl lock interrupted, reason %d, cmd %d\n",
>> err, cmd);
>> goto out;
>> @@ -8511,11 +8523,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
>> unlock:
>> if (mddev->hold_active == UNTIL_IOCTL &&
>> err != -EINVAL)
>> mddev->hold_active = 0;
>>
>> - md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev) :
>> + md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev, noio_flags) :
>> mddev_unlock(mddev);
>>
>> out:
>> if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
>> clear_bit(MD_CLOSING, &mddev->flags);
>> @@ -10180,20 +10192,21 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>> static void md_start_sync(struct work_struct *ws)
>> {
>> struct mddev *mddev = container_of(ws, struct mddev, sync_work);
>> int spares = 0;
>> bool suspend = false;
>> + unsigned int noio_flags = 0;
>> char *name;
>>
>> /*
>> * If reshape is still in progress, spares won't be added or removed
>> * from conf until reshape is done.
>> */
>> if (mddev->reshape_position == MaxSector &&
>> md_spares_need_change(mddev)) {
>> suspend = true;
>> - mddev_suspend(mddev, false);
>> + mddev_suspend(mddev, false, &noio_flags);
>> }
>>
>> mddev_lock_nointr(mddev);
>> if (!md_is_rdwr(mddev)) {
>> /*
>> @@ -10237,11 +10250,11 @@ static void md_start_sync(struct work_struct *ws)
>> * not set it again. Otherwise, we may cause issue like this one:
>> * https://bugzilla.kernel.org/show_bug.cgi?id=218200
>> * Therefore, use __mddev_resume(mddev, false).
>> */
>> if (suspend)
>> - __mddev_resume(mddev, false);
>> + __mddev_resume(mddev, false, noio_flags);
>> md_wakeup_thread(mddev->sync_thread);
>> sysfs_notify_dirent_safe(mddev->sysfs_action);
>> md_new_event();
>> return;
>>
>> @@ -10257,11 +10270,11 @@ static void md_start_sync(struct work_struct *ws)
>> * not set it again. Otherwise, we may cause issue like this one:
>> * https://bugzilla.kernel.org/show_bug.cgi?id=218200
>> * Therefore, use __mddev_resume(mddev, false).
>> */
>> if (suspend)
>> - __mddev_resume(mddev, false);
>> + __mddev_resume(mddev, false, noio_flags);
>>
>> wake_up(&resync_wait);
>> if (test_and_clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery) &&
>> mddev->sysfs_action)
>> sysfs_notify_dirent_safe(mddev->sysfs_action);
>> diff --git a/drivers/md/md.h b/drivers/md/md.h
>> index d8daf0f75cbb..3337cd21eb30 100644
>> --- a/drivers/md/md.h
>> +++ b/drivers/md/md.h
>> @@ -619,11 +619,10 @@ struct mddev {
>> mempool_t *serial_info_pool;
>> void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
>> struct md_cluster_info *cluster_info;
>> struct md_cluster_operations *cluster_ops;
>> unsigned int good_device_nr; /* good device num within cluster raid */
>> - unsigned int noio_flag; /* for memalloc scope API */
>>
>> /*
>> * Temporarily store rdev that will be finally removed when
>> * reconfig_mutex is unlocked, protected by reconfig_mutex.
>> */
>> @@ -953,12 +952,13 @@ extern void md_stop(struct mddev *mddev);
>> extern void md_stop_writes(struct mddev *mddev);
>> extern int md_rdev_init(struct md_rdev *rdev);
>> extern void md_rdev_clear(struct md_rdev *rdev);
>>
>> extern bool md_handle_request(struct mddev *mddev, struct bio *bio);
>> -extern int mddev_suspend(struct mddev *mddev, bool interruptible);
>> -extern void mddev_resume(struct mddev *mddev);
>> +extern int mddev_suspend(struct mddev *mddev, bool interruptible,
>> + unsigned int *noio_flags);
>> +extern void mddev_resume(struct mddev *mddev, unsigned int noio_flags);
>> extern void md_idle_sync_thread(struct mddev *mddev);
>> extern void md_frozen_sync_thread(struct mddev *mddev);
>> extern void md_unfrozen_sync_thread(struct mddev *mddev);
>>
>> extern void md_update_sb(struct mddev *mddev, int force);
>> @@ -999,35 +999,38 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
>> if (bio_op(bio) == REQ_OP_WRITE_ZEROES &&
>> !bio->bi_bdev->bd_disk->queue->limits.max_write_zeroes_sectors)
>> mddev->gendisk->queue->limits.max_write_zeroes_sectors = 0;
>> }
>>
>> -static inline int mddev_suspend_and_lock(struct mddev *mddev)
>> +static inline int mddev_suspend_and_lock(struct mddev *mddev,
>> + unsigned int *noio_flags)
>> {
>> int ret;
>>
>> - ret = mddev_suspend(mddev, true);
>> + ret = mddev_suspend(mddev, true, noio_flags);
>> if (ret)
>> return ret;
>>
>> ret = mddev_lock(mddev);
>> if (ret)
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, *noio_flags);
>>
>> return ret;
>> }
>>
>> -static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev)
>> +static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev,
>> + unsigned int *noio_flags)
>> {
>> - mddev_suspend(mddev, false);
>> + mddev_suspend(mddev, false, noio_flags);
>> mddev_lock_nointr(mddev);
>> }
>>
>> -static inline void mddev_unlock_and_resume(struct mddev *mddev)
>> +static inline void mddev_unlock_and_resume(struct mddev *mddev,
>> + unsigned int noio_flags)
>> {
>> mddev_unlock(mddev);
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>> }
>>
>> struct mdu_array_info_s;
>> struct mdu_disk_info_s;
>>
>> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
>> index 7b7546bfa21f..6f8e3a624456 100644
>> --- a/drivers/md/raid5-cache.c
>> +++ b/drivers/md/raid5-cache.c
>> @@ -693,13 +693,15 @@ static void r5c_disable_writeback_async(struct work_struct *work)
>> !READ_ONCE(conf->log) ||
>> !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
>>
>> log = READ_ONCE(conf->log);
>> if (log) {
>> - mddev_suspend(mddev, false);
>> + unsigned int noio_flags;
>> +
>> + mddev_suspend(mddev, false, &noio_flags);
>> log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>> }
>> }
>>
>> static void r5l_submit_current_io(struct r5l_log *log)
>> {
>> @@ -2603,10 +2605,11 @@ EXPORT_SYMBOL(r5c_journal_mode_set);
>> static ssize_t r5c_journal_mode_store(struct mddev *mddev,
>> const char *page, size_t length)
>> {
>> int mode = ARRAY_SIZE(r5c_journal_mode_str);
>> size_t len = length;
>> + unsigned int noio_flags;
>> int ret;
>>
>> if (len < 2)
>> return -EINVAL;
>>
>> @@ -2615,15 +2618,15 @@ static ssize_t r5c_journal_mode_store(struct mddev *mddev,
>>
>> while (mode--)
>> if (strlen(r5c_journal_mode_str[mode]) == len &&
>> !strncmp(page, r5c_journal_mode_str[mode], len))
>> break;
>> - ret = mddev_suspend_and_lock(mddev);
>> + ret = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (ret)
>> return ret;
>> ret = r5c_journal_mode_set(mddev, mode);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return ret ?: length;
>> }
>>
>> struct md_sysfs_entry
>> r5c_journal_mode = __ATTR(journal_mode, 0644,
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index 65ae7d8930fc..6062c4b62cc8 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -6992,11 +6992,11 @@ raid5_show_stripe_size(struct mddev *mddev, char *page)
>> #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
>> static ssize_t
>> raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
>> {
>> struct r5conf *conf;
>> - unsigned long new;
>> + unsigned long new, noio_flags;
>> int err;
>> int size;
>>
>> if (len >= PAGE_SIZE)
>> return -EINVAL;
>> @@ -7011,11 +7011,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
>> if (new % DEFAULT_STRIPE_SIZE != 0 ||
>> new > PAGE_SIZE || new == 0 ||
>> new != roundup_pow_of_two(new))
>> return -EINVAL;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>>
>> conf = mddev->private;
>> if (!conf) {
>> @@ -7049,11 +7049,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
>> err = -ENOMEM;
>> }
>> mutex_unlock(&conf->cache_size_mutex);
>>
>> out_unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return err ?: len;
>> }
>>
>> static struct md_sysfs_entry
>> raid5_stripe_size = __ATTR(stripe_size, 0644,
>> @@ -7127,19 +7127,20 @@ raid5_show_skip_copy(struct mddev *mddev, char *page)
>> static ssize_t
>> raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
>> {
>> struct r5conf *conf;
>> unsigned long new;
>> + unsigned int noio_flags;
>> int err;
>>
>> if (len >= PAGE_SIZE)
>> return -EINVAL;
>> if (kstrtoul(page, 10, &new))
>> return -EINVAL;
>> new = !!new;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> conf = mddev->private;
>> if (!conf)
>> err = -ENODEV;
>> @@ -7152,11 +7153,11 @@ raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
>> lim.features |= BLK_FEAT_STABLE_WRITES;
>> else
>> lim.features &= ~BLK_FEAT_STABLE_WRITES;
>> err = queue_limits_commit_update(q, &lim);
>> }
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return err ?: len;
>> }
>>
>> static struct md_sysfs_entry
>> raid5_skip_copy = __ATTR(skip_copy, S_IRUGO | S_IWUSR,
>> @@ -7195,10 +7196,11 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt,
>> static ssize_t
>> raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
>> {
>> struct r5conf *conf;
>> unsigned int new;
>> + unsigned int noio_flags;
>> int err;
>> struct r5worker_group *new_groups, *old_groups;
>> int group_cnt;
>>
>> if (len >= PAGE_SIZE)
>> @@ -7207,16 +7209,16 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
>> return -EINVAL;
>> /* 8192 should be big enough */
>> if (new > 8192)
>> return -EINVAL;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> conf = mddev->private;
>> if (!conf) {
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return -ENODEV;
>> }
>> raid5_quiesce(mddev, true);
>>
>> if (new != conf->worker_cnt_per_group) {
>> @@ -7237,11 +7239,11 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
>> kfree(old_groups);
>> }
>> }
>>
>> raid5_quiesce(mddev, false);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>>
>> return err ?: len;
>> }
>>
>> static struct md_sysfs_entry
>> @@ -8940,18 +8942,19 @@ static void *raid6_takeover(struct mddev *mddev)
>> }
>>
>> static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
>> {
>> struct r5conf *conf;
>> + unsigned int noio_flags;
>> int err;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> conf = mddev->private;
>> if (!conf) {
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return -ENODEV;
>> }
>>
>> if (strncmp(buf, "ppl", 3) == 0) {
>> /* ppl only works with RAID 5 */
>> @@ -8990,11 +8993,11 @@ static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
>> }
>>
>> if (!err)
>> md_update_sb(mddev, 1);
>>
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>>
>> return err;
>> }
>>
>> static int raid5_start(struct mddev *mddev)
>
^ permalink raw reply
* [PATCH v1] md,dm: caller-owned memalloc_noio token in mddev_suspend()/resume()
From: Chen Cheng @ 2026-07-11 6:48 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
Save token as mddev-scoped in mddev->noio_flags cause PF_MEMALLOC_NOIO
leak into task A, while task B restores a token that it never saved.
scenario:
task A mddev task B
====== ======= ============
write suspend_lo
mddev_suspend()
suspended == 0
drain active_io
suspended = 1
A: noio_A = memalloc_noio_save()
A returns with PF_MEMALLOC_NOIO set
write suspend_hi
mddev_suspend()
suspended == 1
suspended = 2
B returns
(no save)
mddev_resume()
suspended = 1
not last resume
A returns
A still has PF_MEMALLOC_NOIO <-- leaked
mddev_resume()
suspended = 0
memalloc_noio_restore(noio_A)
(restores A's token in B)
Fixed by:
for md-raid side:
- return each caller's noio_flags from mddev_suspend()
- pass that token back into mddev_resume()
- update the suspend-and-lock helpers to carry the token
for dm-raid side:
- just bypass because: dm-raid call suspend() or resume() will return to
userland, so is not paired in the same task context.
Validation:
repeatedly updates the array's suspend_lo and suspend_hi sysfs from many
concurrent userspace workers. That makes multiple tasks to call
mddev_suspend()/mddev_resume() concurrently.
Each worker:
- reads its initial /proc/self/stat flags and verifies that PF_MEMALLOC_NOIO is not already
set
- writes 0 to either suspend_lo or suspend_hi
- immediately reads its own task flags again
- reports success if flags & PF_MEMALLOC_NOIO is true after the write returns
Link: https://github.com/chencheng-fnnas/reproducer/blob/main/repro-md-noio-token-leak.sh
Fixes: 78f57ef9d50a ("md: use memalloc scope APIs in mddev_suspend()/mddev_resume()")
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
rfc -> v1:
- dm-raid side just bypass because suspend and resume not in the
same task context..
---
drivers/md/dm-raid.c | 6 +--
drivers/md/md-autodetect.c | 5 ++-
drivers/md/md-bitmap.c | 12 ++---
drivers/md/md.c | 90 +++++++++++++++++++++++---------------
drivers/md/md.h | 23 +++++-----
drivers/md/raid5-cache.c | 11 +++--
drivers/md/raid5.c | 24 +++++-----
7 files changed, 101 insertions(+), 70 deletions(-)
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 8f5a5e1342a9..ce0f562c5da3 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3251,11 +3251,11 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
/* Start raid set read-only and assumed clean to change in raid_resume() */
rs->md.ro = MD_RDONLY;
rs->md.in_sync = 1;
/* Has to be held on running the array */
- mddev_suspend_and_lock_nointr(&rs->md);
+ mddev_suspend_and_lock_nointr(&rs->md, NULL);
/* Keep array frozen until resume. */
md_frozen_sync_thread(&rs->md);
r = md_run(&rs->md);
@@ -3863,11 +3863,11 @@ static void raid_postsuspend(struct dm_target *ti)
/*
* sync_thread must be stopped during suspend, and writes have
* to be stopped before suspending to avoid deadlocks.
*/
md_stop_writes(&rs->md);
- mddev_suspend(&rs->md, false);
+ mddev_suspend(&rs->md, false, NULL);
rs->md.ro = MD_RDONLY;
}
clear_bit(MD_DM_SUSPENDING, &mddev->flags);
}
@@ -4141,11 +4141,11 @@ static void raid_resume(struct dm_target *ti)
lockdep_is_held(&mddev->reconfig_mutex)));
clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
mddev->ro = MD_RDWR;
mddev->in_sync = 0;
md_unfrozen_sync_thread(mddev);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, 0);
}
}
static struct target_type raid_target = {
.name = "raid",
diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
index 4b80165afd23..58e062cd0580 100644
--- a/drivers/md/md-autodetect.c
+++ b/drivers/md/md-autodetect.c
@@ -126,10 +126,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
dev_t devices[MD_SB_DISKS + 1], mdev;
struct mdu_array_info_s ainfo = { };
struct mddev *mddev;
int err = 0, i;
char name[16];
+ unsigned int noio_flags;
if (args->partitioned) {
mdev = MKDEV(mdp_major, args->minor << MdpMinorShift);
sprintf(name, "md_d%d", args->minor);
} else {
@@ -173,11 +174,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
if (IS_ERR(mddev)) {
pr_err("md: md_alloc failed - cannot start array %s\n", name);
return;
}
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err) {
pr_err("md: failed to lock array %s\n", name);
goto out_mddev_put;
}
@@ -219,11 +220,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
if (!err)
err = do_md_run(mddev);
if (err)
pr_warn("md: starting %s failed\n", name);
out_unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
out_mddev_put:
mddev_put(mddev);
}
static int __init raid_setup(char *str)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 028b9ca8ce52..74b7f569a3f4 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2620,13 +2620,14 @@ location_show(struct mddev *mddev, char *page)
}
static ssize_t
location_store(struct mddev *mddev, const char *buf, size_t len)
{
+ unsigned int noio_flags;
int rv;
- rv = mddev_suspend_and_lock(mddev);
+ rv = mddev_suspend_and_lock(mddev, &noio_flags);
if (rv)
return rv;
if (mddev->pers) {
if (mddev->recovery || mddev->sync_thread) {
@@ -2711,11 +2712,11 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
}
rv = 0;
out:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
if (rv)
return rv;
return len;
merge_err:
@@ -2831,17 +2832,18 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
{
unsigned long backlog;
unsigned long old_mwb = mddev->bitmap_info.max_write_behind;
struct md_rdev *rdev;
bool has_write_mostly = false;
+ unsigned int noio_flags;
int rv = kstrtoul(buf, 10, &backlog);
if (rv)
return rv;
if (backlog > COUNTER_MAX)
return -EINVAL;
- rv = mddev_suspend_and_lock(mddev);
+ rv = mddev_suspend_and_lock(mddev, &noio_flags);
if (rv)
return rv;
/*
* Without write mostly device, it doesn't make sense to set
@@ -2854,11 +2856,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
}
}
if (!has_write_mostly) {
pr_warn_ratelimited("%s: can't set backlog, no write mostly device available\n",
mdname(mddev));
- mddev_unlock(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return -EINVAL;
}
mddev->bitmap_info.max_write_behind = backlog;
if (!backlog && mddev->serial_info_pool) {
@@ -2871,11 +2873,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
mddev_create_serial_pool(mddev, rdev);
}
if (old_mwb != backlog)
bitmap_update_sb(mddev->bitmap);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return len;
}
static struct md_sysfs_entry bitmap_backlog =
__ATTR(backlog, S_IRUGO|S_IWUSR, backlog_show, backlog_store);
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 1377c407614c..04e10769d5c6 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -459,11 +459,12 @@ static void md_submit_bio(struct bio *bio)
/*
* Make sure no new requests are submitted to the device, and any requests that
* have been submitted are completely handled.
*/
-int mddev_suspend(struct mddev *mddev, bool interruptible)
+int mddev_suspend(struct mddev *mddev, bool interruptible,
+ unsigned int *noio_flags)
{
int err = 0;
/*
* hold reconfig_mutex to wait for normal io will deadlock, because
@@ -477,11 +478,16 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
else
mutex_lock(&mddev->suspend_mutex);
if (err)
return err;
+ if (noio_flags)
+ *noio_flags = 0;
+
if (mddev->suspended) {
+ if (noio_flags && !mddev_is_dm(mddev))
+ *noio_flags = memalloc_noio_save();
WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
mutex_unlock(&mddev->suspend_mutex);
return 0;
}
@@ -515,31 +521,31 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
* prevent deadlock.
*/
WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
/* restrict memory reclaim I/O during raid array is suspend */
- mddev->noio_flag = memalloc_noio_save();
+ if (noio_flags && !mddev_is_dm(mddev))
+ *noio_flags = memalloc_noio_save();
mutex_unlock(&mddev->suspend_mutex);
return 0;
}
EXPORT_SYMBOL_GPL(mddev_suspend);
-static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
+static void __mddev_resume(struct mddev *mddev, bool recovery_needed,
+ unsigned int noio_flags)
{
lockdep_assert_not_held(&mddev->reconfig_mutex);
mutex_lock(&mddev->suspend_mutex);
+ memalloc_noio_restore(noio_flags);
WRITE_ONCE(mddev->suspended, mddev->suspended - 1);
if (mddev->suspended) {
mutex_unlock(&mddev->suspend_mutex);
return;
}
- /* entred the memalloc scope from mddev_suspend() */
- memalloc_noio_restore(mddev->noio_flag);
-
percpu_ref_resurrect(&mddev->active_io);
wake_up(&mddev->sb_wait);
if (recovery_needed)
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
@@ -547,13 +553,13 @@ static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
md_wakeup_thread(mddev->sync_thread); /* possibly kick off a reshape */
mutex_unlock(&mddev->suspend_mutex);
}
-void mddev_resume(struct mddev *mddev)
+void mddev_resume(struct mddev *mddev, unsigned int noio_flags)
{
- return __mddev_resume(mddev, true);
+ return __mddev_resume(mddev, true, noio_flags);
}
EXPORT_SYMBOL_GPL(mddev_resume);
/* sync bdev before setting device to readonly or stopping raid*/
static int mddev_set_closing_and_sync_blockdev(struct mddev *mddev, int opener_num)
@@ -3737,10 +3743,11 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
{
struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr);
struct md_rdev *rdev = container_of(kobj, struct md_rdev, kobj);
struct kernfs_node *kn = NULL;
bool suspend = false;
+ unsigned int noio_flags = 0;
ssize_t rv;
struct mddev *mddev = READ_ONCE(rdev->mddev);
if (!entry->store)
return -EIO;
@@ -3756,17 +3763,17 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
cmd_match(page, "writemostly") ||
cmd_match(page, "-writemostly"))
suspend = true;
}
- rv = suspend ? mddev_suspend_and_lock(mddev) : mddev_lock(mddev);
+ rv = suspend ? mddev_suspend_and_lock(mddev, &noio_flags) : mddev_lock(mddev);
if (!rv) {
if (rdev->mddev == NULL)
rv = -ENODEV;
else
rv = entry->store(rdev, page, length);
- suspend ? mddev_unlock_and_resume(mddev) : mddev_unlock(mddev);
+ suspend ? mddev_unlock_and_resume(mddev, noio_flags) : mddev_unlock(mddev);
}
if (kn)
sysfs_unbreak_active_protection(kn);
@@ -4049,15 +4056,16 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
size_t slen = len;
struct md_personality *pers, *oldpers;
long level;
void *priv, *oldpriv;
struct md_rdev *rdev;
+ unsigned int noio_flags;
if (slen == 0 || slen >= sizeof(clevel))
return -EINVAL;
- rv = mddev_suspend_and_lock(mddev);
+ rv = mddev_suspend_and_lock(mddev, &noio_flags);
if (rv)
return rv;
if (mddev->pers == NULL) {
memcpy(mddev->clevel, buf, slen);
@@ -4231,11 +4239,11 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
md_update_sb(mddev, 1);
sysfs_notify_dirent_safe(mddev->sysfs_level);
md_new_event();
rv = len;
out_unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return rv;
}
static struct md_sysfs_entry md_level =
__ATTR(level, S_IRUGO|S_IWUSR, level_show, level_store);
@@ -4410,17 +4418,18 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks);
static ssize_t
raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
{
unsigned int n;
+ unsigned int noio_flags;
int err;
err = kstrtouint(buf, 10, &n);
if (err < 0)
return err;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
if (mddev->pers) {
if (n != mddev->raid_disks)
err = update_raid_disks(mddev, n);
@@ -4442,11 +4451,11 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
mddev->raid_disks = n;
mddev->reshape_backwards = (mddev->delta_disks < 0);
} else
mddev->raid_disks = n;
out_unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ? err : len;
}
static struct md_sysfs_entry md_raid_disks =
__ATTR(raid_disks, S_IRUGO|S_IWUSR, raid_disks_show, raid_disks_store);
@@ -4822,10 +4831,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
char *e;
int major = simple_strtoul(buf, &e, 10);
int minor;
dev_t dev;
struct md_rdev *rdev;
+ unsigned int noio_flags;
int err;
if (!*buf || *e != ':' || !e[1] || e[1] == '\n')
return -EINVAL;
minor = simple_strtoul(e+1, &e, 10);
@@ -4834,11 +4844,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
dev = MKDEV(major, minor);
if (major != MAJOR(dev) ||
minor != MINOR(dev))
return -EOVERFLOW;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
if (mddev->persistent) {
rdev = md_import_device(dev, mddev->major_version,
mddev->minor_version);
@@ -4855,18 +4865,18 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
rdev = md_import_device(dev, -2, -1);
else
rdev = md_import_device(dev, -1, -1);
if (IS_ERR(rdev)) {
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return PTR_ERR(rdev);
}
err = bind_rdev_to_array(rdev, mddev);
out:
if (err)
export_rdev(rdev);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
if (!err)
md_new_event();
return err ? err : len;
}
@@ -5257,28 +5267,29 @@ static int mddev_start_reshape(struct mddev *mddev)
static ssize_t
action_store(struct mddev *mddev, const char *page, size_t len)
{
int ret;
enum sync_action action;
+ unsigned int noio_flags = 0;
if (!mddev->pers || !mddev->pers->sync_request)
return -EINVAL;
action = md_sync_action_by_name(page);
retry:
if (work_busy(&mddev->sync_work))
flush_work(&mddev->sync_work);
ret = (action == ACTION_RESHAPE) ?
- mddev_suspend_and_lock(mddev) :
+ mddev_suspend_and_lock(mddev, &noio_flags) :
mddev_lock(mddev);
if (ret)
return ret;
if (work_busy(&mddev->sync_work)) {
if (action == ACTION_RESHAPE)
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
else
mddev_unlock(mddev);
goto retry;
}
@@ -5349,11 +5360,11 @@ action_store(struct mddev *mddev, const char *page, size_t len)
sysfs_notify_dirent_safe(mddev->sysfs_action);
ret = len;
out:
if (action == ACTION_RESHAPE)
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
else
mddev_unlock(mddev);
return ret;
}
@@ -5640,24 +5651,25 @@ suspend_lo_show(struct mddev *mddev, char *page)
static ssize_t
suspend_lo_store(struct mddev *mddev, const char *buf, size_t len)
{
unsigned long long new;
+ unsigned int noio_flags;
int err;
err = kstrtoull(buf, 10, &new);
if (err < 0)
return err;
if (new != (sector_t)new)
return -EINVAL;
- err = mddev_suspend(mddev, true);
+ err = mddev_suspend(mddev, true, &noio_flags);
if (err)
return err;
WRITE_ONCE(mddev->suspend_lo, new);
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
return len;
}
static struct md_sysfs_entry md_suspend_lo =
__ATTR(suspend_lo, S_IRUGO|S_IWUSR, suspend_lo_show, suspend_lo_store);
@@ -5671,24 +5683,25 @@ suspend_hi_show(struct mddev *mddev, char *page)
static ssize_t
suspend_hi_store(struct mddev *mddev, const char *buf, size_t len)
{
unsigned long long new;
+ unsigned int noio_flags;
int err;
err = kstrtoull(buf, 10, &new);
if (err < 0)
return err;
if (new != (sector_t)new)
return -EINVAL;
- err = mddev_suspend(mddev, true);
+ err = mddev_suspend(mddev, true, &noio_flags);
if (err)
return err;
WRITE_ONCE(mddev->suspend_hi, new);
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
return len;
}
static struct md_sysfs_entry md_suspend_hi =
__ATTR(suspend_hi, S_IRUGO|S_IWUSR, suspend_hi_show, suspend_hi_store);
@@ -5928,19 +5941,20 @@ static ssize_t serialize_policy_show(struct mddev *mddev, char *page)
static ssize_t
serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
{
int err;
bool value;
+ unsigned int noio_flags;
err = kstrtobool(buf, &value);
if (err)
return err;
if (value == test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
return len;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
if (mddev->pers == NULL || (mddev->pers->head.id != ID_RAID1)) {
pr_err("md: serialize_policy is only effective for raid1\n");
err = -EINVAL;
@@ -5953,11 +5967,11 @@ serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
} else {
mddev_destroy_serial_pool(mddev, NULL);
clear_bit(MD_SERIALIZE_POLICY, &mddev->flags);
}
unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ?: len;
}
static struct md_sysfs_entry md_serialize_policy =
__ATTR(serialize_policy, S_IRUGO | S_IWUSR, serialize_policy_show,
@@ -6263,21 +6277,22 @@ EXPORT_SYMBOL_GPL(mddev_stack_new_rdev);
/* update the optimal I/O size after a reshape */
void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes)
{
struct queue_limits lim;
+ unsigned int noio_flags;
if (mddev_is_dm(mddev))
return;
/* don't bother updating io_opt if we can't suspend the array */
- if (mddev_suspend(mddev, false) < 0)
+ if (mddev_suspend(mddev, false, &noio_flags) < 0)
return;
lim = queue_limits_start_update(mddev->gendisk->queue);
lim.io_opt = lim.io_min * nr_stripes;
queue_limits_commit_update(mddev->gendisk->queue, &lim);
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
}
EXPORT_SYMBOL_GPL(mddev_update_io_opt);
static void mddev_delayed_delete(struct work_struct *ws)
{
@@ -7255,10 +7270,11 @@ static void autorun_array(struct mddev *mddev)
*/
static void autorun_devices(int part)
{
struct md_rdev *rdev0, *rdev, *tmp;
struct mddev *mddev;
+ unsigned int noio_flags;
pr_info("md: autorun ...\n");
while (!list_empty(&pending_raid_disks)) {
int unit;
dev_t dev;
@@ -7295,27 +7311,27 @@ static void autorun_devices(int part)
mddev = md_alloc(dev, NULL);
if (IS_ERR(mddev))
break;
- if (mddev_suspend_and_lock(mddev))
+ if (mddev_suspend_and_lock(mddev, &noio_flags))
pr_warn("md: %s locked, cannot run\n", mdname(mddev));
else if (mddev->raid_disks || mddev->major_version
|| !list_empty(&mddev->disks)) {
pr_warn("md: %s already running, cannot run %pg\n",
mdname(mddev), rdev0->bdev);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
} else {
pr_debug("md: created %s\n", mdname(mddev));
mddev->persistent = 1;
rdev_for_each_list(rdev, tmp, &candidates) {
list_del_init(&rdev->same_set);
if (bind_rdev_to_array(rdev, mddev))
export_rdev(rdev);
}
autorun_array(mddev);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
}
/* on success, candidates will be empty, on error
* it won't...
*/
rdev_for_each_list(rdev, tmp, &candidates) {
@@ -8329,10 +8345,11 @@ static int __md_set_array_info(struct mddev *mddev, void __user *argp)
static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
unsigned int cmd, unsigned long arg)
{
int err = 0;
+ unsigned int noio_flags = 0;
void __user *argp = (void __user *)arg;
struct mddev *mddev = NULL;
err = md_ioctl_valid(cmd);
if (err)
@@ -8380,11 +8397,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
}
if (!md_is_rdwr(mddev))
flush_work(&mddev->sync_work);
- err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev) :
+ err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev, &noio_flags) :
mddev_lock(mddev);
if (err) {
pr_debug("md: ioctl lock interrupted, reason %d, cmd %d\n",
err, cmd);
goto out;
@@ -8511,11 +8528,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
unlock:
if (mddev->hold_active == UNTIL_IOCTL &&
err != -EINVAL)
mddev->hold_active = 0;
- md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev) :
+ md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev, noio_flags) :
mddev_unlock(mddev);
out:
if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
clear_bit(MD_CLOSING, &mddev->flags);
@@ -10180,20 +10197,21 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
static void md_start_sync(struct work_struct *ws)
{
struct mddev *mddev = container_of(ws, struct mddev, sync_work);
int spares = 0;
bool suspend = false;
+ unsigned int noio_flags = 0;
char *name;
/*
* If reshape is still in progress, spares won't be added or removed
* from conf until reshape is done.
*/
if (mddev->reshape_position == MaxSector &&
md_spares_need_change(mddev)) {
suspend = true;
- mddev_suspend(mddev, false);
+ mddev_suspend(mddev, false, &noio_flags);
}
mddev_lock_nointr(mddev);
if (!md_is_rdwr(mddev)) {
/*
@@ -10237,11 +10255,11 @@ static void md_start_sync(struct work_struct *ws)
* not set it again. Otherwise, we may cause issue like this one:
* https://bugzilla.kernel.org/show_bug.cgi?id=218200
* Therefore, use __mddev_resume(mddev, false).
*/
if (suspend)
- __mddev_resume(mddev, false);
+ __mddev_resume(mddev, false, noio_flags);
md_wakeup_thread(mddev->sync_thread);
sysfs_notify_dirent_safe(mddev->sysfs_action);
md_new_event();
return;
@@ -10257,11 +10275,11 @@ static void md_start_sync(struct work_struct *ws)
* not set it again. Otherwise, we may cause issue like this one:
* https://bugzilla.kernel.org/show_bug.cgi?id=218200
* Therefore, use __mddev_resume(mddev, false).
*/
if (suspend)
- __mddev_resume(mddev, false);
+ __mddev_resume(mddev, false, noio_flags);
wake_up(&resync_wait);
if (test_and_clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery) &&
mddev->sysfs_action)
sysfs_notify_dirent_safe(mddev->sysfs_action);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d8daf0f75cbb..7d92deea34f2 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -619,11 +619,10 @@ struct mddev {
mempool_t *serial_info_pool;
void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
struct md_cluster_info *cluster_info;
struct md_cluster_operations *cluster_ops;
unsigned int good_device_nr; /* good device num within cluster raid */
- unsigned int noio_flag; /* for memalloc scope API */
/*
* Temporarily store rdev that will be finally removed when
* reconfig_mutex is unlocked, protected by reconfig_mutex.
*/
@@ -953,12 +952,13 @@ extern void md_stop(struct mddev *mddev);
extern void md_stop_writes(struct mddev *mddev);
extern int md_rdev_init(struct md_rdev *rdev);
extern void md_rdev_clear(struct md_rdev *rdev);
extern bool md_handle_request(struct mddev *mddev, struct bio *bio);
-extern int mddev_suspend(struct mddev *mddev, bool interruptible);
-extern void mddev_resume(struct mddev *mddev);
+extern int mddev_suspend(struct mddev *mddev, bool interruptible,
+ unsigned int *noio_flags);
+extern void mddev_resume(struct mddev *mddev, unsigned int noio_flags);
extern void md_idle_sync_thread(struct mddev *mddev);
extern void md_frozen_sync_thread(struct mddev *mddev);
extern void md_unfrozen_sync_thread(struct mddev *mddev);
extern void md_update_sb(struct mddev *mddev, int force);
@@ -999,35 +999,38 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
if (bio_op(bio) == REQ_OP_WRITE_ZEROES &&
!bio->bi_bdev->bd_disk->queue->limits.max_write_zeroes_sectors)
mddev->gendisk->queue->limits.max_write_zeroes_sectors = 0;
}
-static inline int mddev_suspend_and_lock(struct mddev *mddev)
+static inline int mddev_suspend_and_lock(struct mddev *mddev,
+ unsigned int *noio_flags)
{
int ret;
- ret = mddev_suspend(mddev, true);
+ ret = mddev_suspend(mddev, true, noio_flags);
if (ret)
return ret;
ret = mddev_lock(mddev);
if (ret)
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags ? *noio_flags : 0);
return ret;
}
-static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev)
+static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev,
+ unsigned int *noio_flags)
{
- mddev_suspend(mddev, false);
+ mddev_suspend(mddev, false, noio_flags);
mddev_lock_nointr(mddev);
}
-static inline void mddev_unlock_and_resume(struct mddev *mddev)
+static inline void mddev_unlock_and_resume(struct mddev *mddev,
+ unsigned int noio_flags)
{
mddev_unlock(mddev);
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
}
struct mdu_array_info_s;
struct mdu_disk_info_s;
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 7b7546bfa21f..6f8e3a624456 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -693,13 +693,15 @@ static void r5c_disable_writeback_async(struct work_struct *work)
!READ_ONCE(conf->log) ||
!test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
log = READ_ONCE(conf->log);
if (log) {
- mddev_suspend(mddev, false);
+ unsigned int noio_flags;
+
+ mddev_suspend(mddev, false, &noio_flags);
log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
}
}
static void r5l_submit_current_io(struct r5l_log *log)
{
@@ -2603,10 +2605,11 @@ EXPORT_SYMBOL(r5c_journal_mode_set);
static ssize_t r5c_journal_mode_store(struct mddev *mddev,
const char *page, size_t length)
{
int mode = ARRAY_SIZE(r5c_journal_mode_str);
size_t len = length;
+ unsigned int noio_flags;
int ret;
if (len < 2)
return -EINVAL;
@@ -2615,15 +2618,15 @@ static ssize_t r5c_journal_mode_store(struct mddev *mddev,
while (mode--)
if (strlen(r5c_journal_mode_str[mode]) == len &&
!strncmp(page, r5c_journal_mode_str[mode], len))
break;
- ret = mddev_suspend_and_lock(mddev);
+ ret = mddev_suspend_and_lock(mddev, &noio_flags);
if (ret)
return ret;
ret = r5c_journal_mode_set(mddev, mode);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return ret ?: length;
}
struct md_sysfs_entry
r5c_journal_mode = __ATTR(journal_mode, 0644,
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 65ae7d8930fc..482872ae7198 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6993,10 +6993,11 @@ raid5_show_stripe_size(struct mddev *mddev, char *page)
static ssize_t
raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
{
struct r5conf *conf;
unsigned long new;
+ unsigned int noio_flags;
int err;
int size;
if (len >= PAGE_SIZE)
return -EINVAL;
@@ -7011,11 +7012,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
if (new % DEFAULT_STRIPE_SIZE != 0 ||
new > PAGE_SIZE || new == 0 ||
new != roundup_pow_of_two(new))
return -EINVAL;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
conf = mddev->private;
if (!conf) {
@@ -7049,11 +7050,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
err = -ENOMEM;
}
mutex_unlock(&conf->cache_size_mutex);
out_unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ?: len;
}
static struct md_sysfs_entry
raid5_stripe_size = __ATTR(stripe_size, 0644,
@@ -7127,19 +7128,20 @@ raid5_show_skip_copy(struct mddev *mddev, char *page)
static ssize_t
raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
{
struct r5conf *conf;
unsigned long new;
+ unsigned int noio_flags;
int err;
if (len >= PAGE_SIZE)
return -EINVAL;
if (kstrtoul(page, 10, &new))
return -EINVAL;
new = !!new;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
conf = mddev->private;
if (!conf)
err = -ENODEV;
@@ -7152,11 +7154,11 @@ raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
lim.features |= BLK_FEAT_STABLE_WRITES;
else
lim.features &= ~BLK_FEAT_STABLE_WRITES;
err = queue_limits_commit_update(q, &lim);
}
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ?: len;
}
static struct md_sysfs_entry
raid5_skip_copy = __ATTR(skip_copy, S_IRUGO | S_IWUSR,
@@ -7195,10 +7197,11 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt,
static ssize_t
raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
{
struct r5conf *conf;
unsigned int new;
+ unsigned int noio_flags;
int err;
struct r5worker_group *new_groups, *old_groups;
int group_cnt;
if (len >= PAGE_SIZE)
@@ -7207,16 +7210,16 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
return -EINVAL;
/* 8192 should be big enough */
if (new > 8192)
return -EINVAL;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
conf = mddev->private;
if (!conf) {
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return -ENODEV;
}
raid5_quiesce(mddev, true);
if (new != conf->worker_cnt_per_group) {
@@ -7237,11 +7240,11 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
kfree(old_groups);
}
}
raid5_quiesce(mddev, false);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ?: len;
}
static struct md_sysfs_entry
@@ -8940,18 +8943,19 @@ static void *raid6_takeover(struct mddev *mddev)
}
static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
{
struct r5conf *conf;
+ unsigned int noio_flags;
int err;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
conf = mddev->private;
if (!conf) {
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return -ENODEV;
}
if (strncmp(buf, "ppl", 3) == 0) {
/* ppl only works with RAID 5 */
@@ -8990,11 +8994,11 @@ static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
}
if (!err)
md_update_sb(mddev, 1);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err;
}
static int raid5_start(struct mddev *mddev)
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v6 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
From: Chen Cheng @ 2026-07-11 8:38 UTC (permalink / raw)
To: yukuai, linux-raid; +Cc: linux-kernel
In-Reply-To: <fb8b048b-3912-44e8-9f56-a25989cd80ee@fygo.io>
在 2026/7/5 15:48, yu kuai 写道:
> Hi,
>
> 在 2026/6/23 20:38, Chen Cheng 写道:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> origin flow:
>>
>> bio_endio(master_bio); /* may drop active_io to zero */
>> allow_barrier(conf);
>> free_r10bio(r10_bio); /* reads conf->geo, returns to pool */
>>
>> one scenario is:
>>
>> CPU A (softirq, raid_end_bio_io) CPU B (action_store) --> reshape
>> ================================ ===============================
>> bio_endio(master_bio)
>> md_end_clone_io
>> percpu_ref_put -> 0
>> wait_event wakeup, and,
>> mddev_suspend return
>> raid10_start_reshape:
>> setup_geo(&conf->geo, new)
>> ...
>> mempool_destroy(old_pool)
>> conf->r10bio_pool = new_pool
>> allow_barrier(conf)
>> free_r10bio(r10_bio)
>> put_all_bios:
>> for (i=0; i<conf->geo.raid_disks; i++)
>> ==> old obj, new geo, OOB
>> mempool_free(r10_bio, conf->r10bio_pool)
>> ==> old-geometry obj freed into new pool
>>
>> so .. fix by reorder the flow:
>>
>> free_r10bio(r10_bio)
>> allow_barrier(conf)
>> bio_endio(master_io)
>>
>> raid_end_discard_bio() is exactly the same.
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/raid10.c | 17 ++++++++++-------
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index d740744a9746..e44a9b6e95c7 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -330,24 +330,25 @@ static void reschedule_retry(struct r10bio *r10_bio)
>> */
>> static void raid_end_bio_io(struct r10bio *r10_bio)
>> {
>> struct bio *bio = r10_bio->master_bio;
>> struct r10conf *conf = r10_bio->mddev->private;
>> + unsigned long state = r10_bio->state;
>>
>> - if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
>> - if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
>> - bio->bi_status = BLK_STS_IOERR;
>> - bio_endio(bio);
>> - }
>> + free_r10bio(r10_bio);
>>
>> /*
>> * Wake up any possible resync thread that waits for the device
>> * to go idle.
>> */
>> allow_barrier(conf);
>>
>> - free_r10bio(r10_bio);
>> + if (!test_and_set_bit(R10BIO_Returned, &state)) {
>> + if (!test_bit(R10BIO_Uptodate, &state))
>> + bio->bi_status = BLK_STS_IOERR;
>> + bio_endio(bio);
>> + }
>
> Why is this moved to the end? I feel it's not necessary.
>
yes, point is let endio after free r10bio.
free_r10bio(r10_bio) -> allow_barrier(conf) -> bio_endio(master_io) or
free_r10bio(r10_bio) -> bio_endio(master_io) -> allow_barrier(conf)
both order is correct to fix the bug.
>> }
>>
>> /*
>> * Update disk head position estimator based on IRQ completion info.
>> */
>> @@ -1580,13 +1581,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
>> if (!test_bit(R10BIO_Discard, &r10bio->state)) {
>> first_r10bio = (struct r10bio *)r10bio->master_bio;
>> free_r10bio(r10bio);
>> r10bio = first_r10bio;
>> } else {
>> + struct bio *master_bio = r10bio->master_bio;
>> +
>> md_write_end(r10bio->mddev);
>> - bio_endio(r10bio->master_bio);
>> free_r10bio(r10bio);
>> + bio_endio(master_bio);
>> break;
>> }
>> }
>> }
>>
>
^ permalink raw reply
* [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
Hi,
This series fixes slab out-of-bounds accesses in raid10 when reshape changes
the number of raid disks while regular I/O is still reusing r10bio objects
allocated under the previous geometry.
The bug is reproducible with a simple 4-disk to 5-disk reshape under write
load, for example:
mdadm -C /dev/md777 -l10 -n4 /dev/sda /dev/sdb /dev/sdc /dev/sdd
mkfs.ext4 /dev/md777
mount /dev/md777 /mnt/test
fsstress -d /mnt/test -n 24000 -p 8 -l 24 &
mdadm /dev/md777 --add /dev/sde
mdadm --grow /dev/md777 --raid-devices=5 \
--backup-file=/tmp/md-reshape-backup
KASAN report:
BUG: KASAN: slab-out-of-bounds in free_r10bio+0x1c4/0x260 [raid10]
Read of size 8 at addr ffff00008c2dfac8 by task ksoftirqd/0/15
free_r10bio
raid_end_bio_io
one_write_done
raid10_end_write_request
This series addresses the problem in three steps:
1. ensure the sync_action=reshape caller suspends and locks before start_reshape
2. resize r10bio_pool when reshape grows raid_disks
3. reorder the r10bio free flow before bio_endio in the regular and discard
completion paths
Changes in v7:
- patch #1, refactor out suspend from retry block
- patch #3, raid_end_bio_io() refactor to:
free(r10_bio) -> free(master_bio) -> allow_barrier
Changes in v6:
- suspend the array in action_store() after flush_work()
- free r10bio before ending the discard master bio
Changes in v5 (suggested by Yu Kuai):
- simplify patch 2
- switch patch 3 from bounding reused r10bio devs[] walks by used_nr_devs
to reordering the free/endio flow
Changes in v4:
- make the sync_action=reshape path invoke mddev_suspend_and_lock() before
calling start_reshape()
- leave the md-cluster and dm-raid paths unchanged; they still reach
start_reshape() with the mddev locked but without suspend
Changes in v3:
- replace freeze_array()/unfreeze_array() in raid10_start_reshape() with
mddev_suspend_and_lock_nointr()/mddev_unlock_and_resume(); freeze_array()
can return while retry-list items still hold pool objects, while
mddev_suspend() provides the correct upper-layer quiesce interface
Changes in v2:
- add this cover letter
- convert r10bio_pool to a fixed-size kmalloc mempool
- rebuild r10bio_pool inside the freeze window before switching live reshape
geometry
- switch raid10_quiesce() to freeze_array()/unfreeze_array()
Testing:
- reproduced the original KASAN slab-out-of-bounds on 4-disk -> 5-disk
raid10 reshape with fsstress
- verified that this series fixes that reproducer
- exercised the 5-disk -> 4-disk reshape direction as well
Thanks,
Chen Cheng
Chen Cheng (3):
md: suspend array when sync_action=reshape
md/raid10: resize r10bio_pool for reshape
md/raid10: free r10bio before ending master_bio in raid_end_bio_io()
and raid_end_discard_bio()
drivers/md/md.c | 17 +++++++++----
drivers/md/raid10.c | 61 ++++++++++++++++++++++++++++++++-------------
drivers/md/raid10.h | 2 +-
3 files changed, 56 insertions(+), 24 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH v7 1/3] md: suspend array when sync_action=reshape
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
In-Reply-To: <20260711100352.425177-1-chencheng@fnnas.com>
From: Chen Cheng <chencheng@fnnas.com>
raid10 needs to resize/swap r10bio_pool when reshape changes
raid_disks, and, don't let new requests keep allocating r10bio
objects from the old pool while that transition is in progress.
suspend and lock array before mddev_start_reshape(), and resume
it on exit.
Other sync_action ops are unchanged.
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/md.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..8ee2dcc116eb 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5261,25 +5261,32 @@ action_store(struct mddev *mddev, const char *page, size_t len)
enum sync_action action;
if (!mddev->pers || !mddev->pers->sync_request)
return -EINVAL;
+ action = md_sync_action_by_name(page);
+ if (action == ACTION_RESHAPE) {
+ ret = mddev_suspend(mddev, true);
+ if (ret)
+ return ret;
+ }
retry:
if (work_busy(&mddev->sync_work))
flush_work(&mddev->sync_work);
ret = mddev_lock(mddev);
- if (ret)
+ if (ret) {
+ if (action == ACTION_RESHAPE)
+ mddev_resume(mddev);
return ret;
+ }
if (work_busy(&mddev->sync_work)) {
mddev_unlock(mddev);
goto retry;
}
- action = md_sync_action_by_name(page);
-
/* TODO: mdadm rely on "idle" to start sync_thread. */
if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) {
switch (action) {
case ACTION_FROZEN:
md_frozen_sync_thread(mddev);
@@ -5345,10 +5352,12 @@ action_store(struct mddev *mddev, const char *page, size_t len)
sysfs_notify_dirent_safe(mddev->sysfs_action);
ret = len;
out:
mddev_unlock(mddev);
+ if (action == ACTION_RESHAPE)
+ mddev_resume(mddev);
return ret;
}
static struct md_sysfs_entry md_scan_mode =
__ATTR_PREALLOC(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
--
2.54.0
^ permalink raw reply related
* [PATCH v7 2/3] md/raid10: resize r10bio_pool for reshape
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
In-Reply-To: <20260711100352.425177-1-chencheng@fnnas.com>
From: Chen Cheng <chencheng@fnnas.com>
When reshape grows raid_disks, the pool must also switch to new geometry
object size , and allocate a new geometry size pool and replace the old.
But not for shrinking reshape, because regular I/O can still use the
prev geo for sectors that have not crossed reshape_progress yet.
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid10.c | 47 ++++++++++++++++++++++++++++++++-------------
drivers/md/raid10.h | 2 +-
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index cee5a253a281..db68fcc9e9be 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -101,17 +101,27 @@ static void end_reshape(struct r10conf *conf);
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 int calc_r10bio_size(unsigned int raid_disks)
{
- struct r10conf *conf = data;
- int size = offsetof(struct r10bio, devs[conf->geo.raid_disks]);
+ return offsetof(struct r10bio, devs[raid_disks]);
+}
+
+static mempool_t *create_r10bio_pool(unsigned int raid_disks)
+{
+ int size = calc_r10bio_size(raid_disks);
- /* allocate a r10bio with room for raid_disks entries in the
- * bios array */
+ return mempool_create_kmalloc_pool(NR_RAID_BIOS, size);
+}
+
+static struct r10bio *alloc_r10bio(unsigned int raid_disks, gfp_t gfp_flags)
+{
+ int size = calc_r10bio_size(raid_disks);
+
+ /* allocate a r10bio sized for current geometry */
return kzalloc(size, gfp_flags);
}
#define RESYNC_SECTORS (RESYNC_BLOCK_SIZE >> 9)
/* amount of memory to reserve for resync requests */
@@ -135,11 +145,11 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
struct bio *bio;
int j;
int nalloc, nalloc_rp;
struct resync_pages *rps;
- r10_bio = r10bio_pool_alloc(gfp_flags, conf);
+ r10_bio = alloc_r10bio(conf->geo.raid_disks, gfp_flags);
if (!r10_bio)
return NULL;
if (test_bit(MD_RECOVERY_SYNC, &conf->mddev->recovery) ||
test_bit(MD_RECOVERY_RESHAPE, &conf->mddev->recovery))
@@ -275,11 +285,11 @@ static void put_all_bios(struct r10conf *conf, struct r10bio *r10_bio)
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)
{
struct r10conf *conf = r10_bio->mddev->private;
@@ -1537,11 +1547,11 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
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;
r10_bio->mddev = mddev;
@@ -1729,11 +1739,11 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
last_stripe_index *= geo->far_copies;
end_disk_offset = (bio_end & geo->chunk_mask) +
(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;
r10_bio->read_slot = -1;
memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
@@ -3830,11 +3840,11 @@ static int setup_geo(struct geom *geo, struct mddev *mddev, enum geo_type new)
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);
safe_put_page(conf->tmppage);
bioset_exit(&conf->bio_split);
@@ -3877,13 +3887,12 @@ static struct r10conf *setup_conf(struct mddev *mddev)
if (!conf->tmppage)
goto out;
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(conf->geo.raid_disks);
+ if (!conf->r10bio_pool)
goto out;
err = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
if (err)
goto out;
@@ -4373,10 +4382,11 @@ static int raid10_start_reshape(struct mddev *mddev)
struct geom new;
struct r10conf *conf = mddev->private;
struct md_rdev *rdev;
int spares = 0;
int ret;
+ mempool_t *new_pool = NULL;
if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
return -EBUSY;
if (setup_geo(&new, mddev, geo_start) != conf->copies)
@@ -4409,10 +4419,15 @@ static int raid10_start_reshape(struct mddev *mddev)
if (spares < mddev->delta_disks)
return -EINVAL;
conf->offset_diff = min_offset_diff;
+ if (mddev->delta_disks > 0) {
+ new_pool = create_r10bio_pool(new.raid_disks);
+ if (!new_pool)
+ return -ENOMEM;
+ }
spin_lock_irq(&conf->device_lock);
if (conf->mirrors_new) {
memcpy(conf->mirrors_new, conf->mirrors,
sizeof(struct raid10_info)*conf->prev.raid_disks);
smp_mb();
@@ -4509,10 +4524,14 @@ static int raid10_start_reshape(struct mddev *mddev)
mddev->degraded = calc_degraded(conf);
spin_unlock_irq(&conf->device_lock);
mddev->raid_disks = conf->geo.raid_disks;
mddev->reshape_position = conf->reshape_progress;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
+ if (new_pool) {
+ mempool_destroy(conf->r10bio_pool);
+ conf->r10bio_pool = new_pool;
+ }
clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
clear_bit(MD_RECOVERY_DONE, &mddev->recovery);
set_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
@@ -4531,10 +4550,12 @@ static int raid10_start_reshape(struct mddev *mddev)
smp_wmb();
conf->reshape_progress = MaxSector;
conf->reshape_safe = MaxSector;
mddev->reshape_position = MaxSector;
spin_unlock_irq(&conf->device_lock);
+ if (new_pool)
+ mempool_destroy(new_pool);
return ret;
}
/* Calculate the last device-address that could contain
* any block from the chunk that includes the array-address 's'
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
@@ -85,11 +85,11 @@ struct r10conf {
int have_replacement; /* There is at least one
* replacement device.
*/
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;
/* When taking over an array from a different personality, we store
--
2.54.0
^ permalink raw reply related
* [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
In-Reply-To: <20260711100352.425177-1-chencheng@fnnas.com>
From: Chen Cheng <chencheng@fnnas.com>
origin flow:
bio_endio(master_bio); /* may drop active_io to zero */
allow_barrier(conf);
free_r10bio(r10_bio); /* reads conf->geo, returns to pool */
one scenario is:
CPU A (softirq, raid_end_bio_io) CPU B (action_store) --> reshape
================================ ===============================
bio_endio(master_bio)
md_end_clone_io
percpu_ref_put -> 0
wait_event wakeup, and,
mddev_suspend return
raid10_start_reshape:
setup_geo(&conf->geo, new)
...
mempool_destroy(old_pool)
conf->r10bio_pool = new_pool
allow_barrier(conf)
free_r10bio(r10_bio)
put_all_bios:
for (i=0; i<conf->geo.raid_disks; i++)
==> old obj, new geo, OOB
mempool_free(r10_bio, conf->r10bio_pool)
==> old-geometry obj freed into new pool
so .. fix by reorder the flow:
free_r10bio(r10_bio)
allow_barrier(conf)
bio_endio(master_bio)
raid_end_discard_bio() is exactly the same.
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid10.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index db68fcc9e9be..d77f60db7660 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -329,24 +329,27 @@ static void reschedule_retry(struct r10bio *r10_bio)
*/
static void raid_end_bio_io(struct r10bio *r10_bio)
{
struct bio *bio = r10_bio->master_bio;
struct r10conf *conf = r10_bio->mddev->private;
+ unsigned long state = r10_bio->state;
+ bool returned;
- if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
- if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
- bio->bi_status = BLK_STS_IOERR;
+ returned = test_and_set_bit(R10BIO_Returned, &state);
+ if (!returned && !test_bit(R10BIO_Uptodate, &state))
+ bio->bi_status = BLK_STS_IOERR;
+
+ free_r10bio(r10_bio);
+
+ if (!returned)
bio_endio(bio);
- }
/*
* Wake up any possible resync thread that waits for the device
* to go idle.
*/
allow_barrier(conf);
-
- free_r10bio(r10_bio);
}
/*
* Update disk head position estimator based on IRQ completion info.
*/
@@ -1579,13 +1582,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
if (!test_bit(R10BIO_Discard, &r10bio->state)) {
first_r10bio = (struct r10bio *)r10bio->master_bio;
free_r10bio(r10bio);
r10bio = first_r10bio;
} else {
+ struct bio *master_bio = r10bio->master_bio;
+
md_write_end(r10bio->mddev);
- bio_endio(r10bio->master_bio);
free_r10bio(r10bio);
+ bio_endio(master_bio);
break;
}
}
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Chen Cheng @ 2026-07-11 10:56 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <m2y0g0z0ll.fsf@gmail.com>
在 2026/6/27 19:25, Abd-Alrhman Masalkhi 写道:
>
> Hi Chen,
>
> On Sat, Jun 27, 2026 at 18:52 +0800, Chen Cheng wrote:
>> 在 2026/6/27 18:46, Abd-Alrhman Masalkhi 写道:
>>> On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
>>>> From: Chen Cheng <chencheng@fnnas.com>
>>>>
>>>> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
>>>> instead of u64 plain read.
>>>>
>>> unisgned long is not always u64...
>>> I can not see what it fixes, is this just to silence KCSAN? If so,
>>> please say so in the commit message.
>>
>> I need to find KCSAN report , I lost this one, but I think it's easy to
>> find locklessly path to call md_sync_action(), so. READ_ONCE is need.
>>
> Thanks. Adding the KCSAN report would be helpful.
>
hi Abd-Alrhman
Uh... a apologize for my mislead . it's truelly report by review-bot ,
and after double check , I think it wouldn't be reproduce in my x86-64
machine , because 8B in x64-arch load is atomic +align..
thanks,
cheng.
>>>
>>>> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>>>>
>>>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>>>> ---
>>>> drivers/md/md.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>> index c5c50640b684..f4415c1a79d9 100644
>>>> --- a/drivers/md/md.c
>>>> +++ b/drivers/md/md.c
>>>> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
>>>> return is_recover ? ACTION_RECOVER : ACTION_IDLE;
>>>> }
>>>>
>>>> enum sync_action md_sync_action(struct mddev *mddev)
>>>> {
>>>> - unsigned long recovery = mddev->recovery;
>>>> + unsigned long recovery = READ_ONCE(mddev->recovery);
>>>> enum sync_action active_action;
>>>>
>>>> /*
>>>> * frozen has the highest priority, means running sync_thread will be
>>>> * stopped immediately, and no new sync_thread can start.
>>>> --
>>>> 2.54.0
>>>>
>>>
>>
>
^ permalink raw reply
* Re: LBS setup
From: Piergiorgio Sartor @ 2026-07-11 13:22 UTC (permalink / raw)
To: Wol; +Cc: Piergiorgio Sartor, linux-raid
In-Reply-To: <a2a4e6f6-5521-44f7-b045-7432e3e8e757@youngman.org.uk>
On Fri, Jul 10, 2026 at 09:43:56PM +0100, Wol wrote:
> On 10/07/2026 18:54, Piergiorgio Sartor wrote:
> > Hi Wol
> >
> > thanks for the answer.
> >
> > My question is about "what to do?".
> > Not much what happened.
> >
> > The log says we should copy some size
> > from somewhere to somewhere else.
> > The docs says this should be done
> > with array at rest.
> >
> > Is there any documentation about this?
> > Should we just ignore the log?
> > What should be done, if any?
> >
> > The log:
> >
> > md1: echo current LBS to md/logical_block_size to prevent data loss issues from LBS changes.
> > Note: After setting, array will not be assembled in old kernels (<= 6.18)
>
> Ah. Personally I wouldn't do anything, then. Are you using any systems with
> kernel 6.18 or earlier?
>
> Are you likely to use any kernels 6.18 or earlier in the future?
>
> If the answer to both questions is no, then you're not going to have any
> trouble ...
Thanks again for the answer.
Nevertheless, sorry if I insist.
What I understand from the log message is
that if I do *not* copy the LBS I can get
data loss. If I do copy the LBS, I will
not be anymore backward compatible.
This backward compatibility does not seem
interesting for me, hence I should copy
this LBS.
Which then begs the question: how?
I mean, this seems the logical conclusion
from what I understood from the log.
Also, why to have this log in the first place?
The kernel is 7.1.3, so if everything would
be fine with it, no need to log anything.
I do not know...
Anyway, sorry again if I bother...
Thanks,
bye,
--
piergiorgio
^ permalink raw reply
* Re: [PATCH] md: remove REQ_NOWAIT support
From: yu kuai @ 2026-07-11 15:21 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, vverma,
axboe, linux-raid, linux-kernel
In-Reply-To: <20260628142737.1051059-1-abd.masalkhi@gmail.com>
在 2026/6/28 22:27, Abd-Alrhman Masalkhi 写道:
> REQ_NOWAIT support in md is fundamentally incomplete. While reads can
> avoid some blocking paths, write requests can still encounter cases
> where one mirror succeeds while another returns -EAGAIN. At that point
> md cannot distinguish queue pressure from a real device failure, so it
> can neither record a bad block nor safely retry the write without
> REQ_NOWAIT, leaving mirrors with divergent data.
>
> Rather than continue advertising REQ_NOWAIT support that cannot be
> implemented correctly, remove it from md and its raid personalities.
> This simplifies the waiting paths and avoids exposing incorrect
> non-blocking semantics.
>
> Fixes: bf2c411bb1cf ("md: raid456 add nowait support")
> Fixes: c9aa889b035f ("md: raid10 add nowait support")
> Fixes: 5aa705039c4f ("md: raid1 add nowait support")
> Fixes: f51d46d0e7cb ("md: add support for REQ_NOWAIT")
> Suggested-by: Yu Kuai<yukuai@fygo.io>
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
> ---
> drivers/md/md-bitmap.c | 9 +---
> drivers/md/md-bitmap.h | 2 +-
> drivers/md/md-llbitmap.c | 10 +----
> drivers/md/md.c | 15 ++-----
> drivers/md/raid1-10.c | 8 ++--
> drivers/md/raid1.c | 91 +++++++++-------------------------------
> drivers/md/raid10.c | 80 +++++++++--------------------------
> drivers/md/raid5.c | 13 ------
> 8 files changed, 51 insertions(+), 177 deletions(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: protect lockless reshape_progress accesses
From: yu kuai @ 2026-07-11 15:37 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260627105524.144022-1-chencheng@fnnas.com>
Hi,
在 2026/6/27 18:55, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> During reshape:
> - reshape_request() advances conf->reshape_progress.
> - reshape_request() itself also reads conf->reshape_progress several times
> while calculating sector_nr, writepos, and readpos for the current
> reshape step.
> - use_new_offset() reads conf->reshape_progress locklessly to decide
> whether the current stripe should use data_offset or new_data_offset.
>
> one possible scenario is:
>
> CPU1 CPU2
> reshape_request()
> -> conf->reshape_progress += delta
> ops_run_io()
> -> use_new_offset(conf, sh)
> -> progress = conf->reshape_progress
> -> decide old layout or new layout
> -> use data_offset or new_data_offset
>
> reshape_progress is the boundary between old and new layout mapping.
>
> If CPU2 observes an unprotected reshape_progress value, it can choose
> the wrong layout offset for the current stripe.
>
> if reshape_request() reads reshape_progress multiple times without
> a stable snapshot, one iteration of reshape can calculate its range from
> inconsistent progress values.
>
> Fixes: 7a6613810785 ("md/raid5: reshape using largest of old and new chunk size")
>
> The race report:
> ==================================================================
> BUG: KCSAN: data-race in ops_run_io / reshape_request
>
> write to 0xffff89a8d3ef2270 of 8 bytes by task 1299 on cpu 7:
> reshape_request+0x1292/0x17b0
> raid5_sync_request+0x815/0xa00
> md_do_sync.cold+0xf8d/0x1516
> md_thread+0x15a/0x2d0
> [....]
>
> read to 0xffff89a8d3ef2270 of 8 bytes by task 1292 on cpu 9:
> ops_run_io+0xc25/0x1960
> handle_stripe+0x2273/0x4570
> handle_active_stripes.isra.0+0x6e0/0xa50
> raid5d+0x7d5/0xb90
> [....]
>
> value changed: 0x0000000000173700 -> 0x0000000000173800
> ==================================================================
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 34 +++++++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index eaee7f206ab8..b6809f1a3ab4 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -1011,11 +1011,11 @@ static void stripe_add_to_batch_list(struct r5conf *conf,
> /* Determine if 'data_offset' or 'new_data_offset' should be used
> * in this stripe_head.
> */
> static int use_new_offset(struct r5conf *conf, struct stripe_head *sh)
> {
> - sector_t progress = conf->reshape_progress;
> + sector_t progress = READ_ONCE(conf->reshape_progress);
> /* Need a memory barrier to make sure we see the value
> * of conf->generation, or ->data_offset that was set before
> * reshape_progress was updated.
> */
> smp_rmb();
> @@ -6340,24 +6340,25 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
> sector_t writepos, readpos, safepos;
> sector_t stripe_addr;
> int reshape_sectors;
> struct list_head stripes;
> sector_t retn;
> + sector_t reshape_progress = READ_ONCE(conf->reshape_progress);
>
> if (sector_nr == 0) {
> /* If restarting in the middle, skip the initial sectors */
> if (mddev->reshape_backwards &&
> - conf->reshape_progress < raid5_size(mddev, 0, 0)) {
> + reshape_progress < raid5_size(mddev, 0, 0)) {
> sector_nr = raid5_size(mddev, 0, 0)
> - - conf->reshape_progress;
> + - reshape_progress;
> } else if (mddev->reshape_backwards &&
> - conf->reshape_progress == MaxSector) {
> + reshape_progress == MaxSector) {
> /* shouldn't happen, but just in case, finish up.*/
> sector_nr = MaxSector;
> } else if (!mddev->reshape_backwards &&
> - conf->reshape_progress > 0)
> - sector_nr = conf->reshape_progress;
> + reshape_progress > 0)
> + sector_nr = reshape_progress;
> sector_div(sector_nr, new_data_disks);
> if (sector_nr) {
> mddev->curr_resync_completed = sector_nr;
> sysfs_notify_dirent_safe(mddev->sysfs_completed);
> *skipped = 1;
> @@ -6377,13 +6378,13 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
> * the data about to be copied would over-write the source of
> * the data at the front of the range. i.e. one new_stripe
> * along from reshape_progress new_maps to after where
> * reshape_safe old_maps to
> */
> - writepos = conf->reshape_progress;
> + writepos = reshape_progress;
> sector_div(writepos, new_data_disks);
> - readpos = conf->reshape_progress;
> + readpos = reshape_progress;
> sector_div(readpos, data_disks);
> safepos = conf->reshape_safe;
> sector_div(safepos, data_disks);
> if (mddev->reshape_backwards) {
> if (WARN_ON(writepos < reshape_sectors))
> @@ -6404,11 +6405,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
>
> /* Having calculated the 'writepos' possibly use it
> * to set 'stripe_addr' which is where we will write to.
> */
> if (mddev->reshape_backwards) {
> - if (WARN_ON(conf->reshape_progress == 0))
> + if (WARN_ON(reshape_progress == 0))
> return MaxSector;
>
> stripe_addr = writepos;
> if (WARN_ON((mddev->dev_sectors &
> ~((sector_t)reshape_sectors - 1)) -
> @@ -6514,14 +6515,21 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
> set_bit(STRIPE_HANDLE, &sh->state);
> }
> list_add(&sh->lru, &stripes);
> }
> spin_lock_irq(&conf->device_lock);
> - if (mddev->reshape_backwards)
> - conf->reshape_progress -= reshape_sectors * new_data_disks;
> - else
> - conf->reshape_progress += reshape_sectors * new_data_disks;
> + if (mddev->reshape_backwards) {
> + sector_t progress = conf->reshape_progress;
> +
> + progress -= reshape_sectors * new_data_disks;
> + WRITE_ONCE(conf->reshape_progress, progress);
> + } else {
> + sector_t progress = conf->reshape_progress;
> +
> + progress += reshape_sectors * new_data_disks;
> + WRITE_ONCE(conf->reshape_progress, progress);
> + }
> spin_unlock_irq(&conf->device_lock);
> /* Ok, those stripe are ready. We can start scheduling
> * reads on the source stripes.
> * The source stripes are determined by mapping the first and last
> * block on the destination stripes.
AI report there are still other places, please check:
1. get_reshape_loc(), the fast path before taking device_lock:
if (likely(conf->reshape_progress == MaxSector))
return LOC_NO_RESHAPE;
The comment right below it already mentions the 64bit-on-32bit tearing
concern, so it makes sense to mark it:
if (likely(READ_ONCE(conf->reshape_progress) == MaxSector))
2. raid5_make_request():
if (likely(conf->reshape_progress == MaxSector)) {
Same - called on every IO, lockless.
For consistency the `== MaxSector` check in raid5_remove_disk() could use
READ_ONCE() too, though it's a reconfig path.
--
Thanks,
Kuai
^ permalink raw reply
* [PATCH] md/raid5: complete discard bios while reshape is active
From: Genjian @ 2026-07-11 16:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao
Cc: linux-raid, linux-kernel, zhanggenjian, Genjian Zhang
From: Genjian Zhang <zhanggenjian@kylinos.cn>
make_discard_request() returns without completing the bio when reshape
is in progress. Discard callers block in submit_bio_wait()
waiting for a completion that never arrives. The caller hangs in
uninterruptible sleep, and this does not resolve when reshape finishes.
Complete the bio with BLK_STS_AGAIN so userspace can retry after reshape,
consistent with the existing policy of not processing discard during
reshape.
Tested on a loop-backed RAID5 array during mdadm --grow: without this
patch, blkdiscard hangs in bio_await() and remains in uninterruptible
sleep after md reports "reshape done"; with this patch it returns
-EAGAIN instead.
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/md/raid5.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ffb5fcde54a9..4c3b42b2af85 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5722,9 +5722,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
return;
- if (mddev->reshape_position != MaxSector)
+ if (mddev->reshape_position != MaxSector) {
/* Skip discard while reshape is happening */
+ bio_endio_status(bi, BLK_STS_AGAIN);
return;
+ }
if (!raid5_discard_limits(mddev, bi))
return;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] md/raid5: protect lockless recovery_offset accesses during reshape
From: yu kuai @ 2026-07-11 16:24 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260627102519.136940-1-chencheng@fnnas.com>
在 2026/6/27 18:25, Chen Cheng 写道:
> From: Chen Cheng<chencheng@fnnas.com>
>
> During reshape:
> - reshape_request() advances rdev->recovery_offset for non-In_sync
> devices locklessly.
> - analyse_stripe() reads rdev->recovery_offset locklessly to decide:
> a. use a replacement device to read ?
> b. a device can already be treated as in-sync for the current
> stripe ?
>
> one possible scenario is:
>
> CPU1 CPU2
> reshape_request()
> -> mddev->curr_resync_completed = sector_nr
> -> if (!mddev->reshape_backwards)
> -> rdev->recovery_offset = sector_nr
> analyse_stripe(sh)
> -> rdev = conf->disks[i].replacement
> -> if (rdev->recovery_offset >=
> sh->sector + stripe_sectors)
> set_bit(R5_ReadRepl)
> -> or
> -> if (sh->sector + stripe_sectors <=
> rdev->recovery_offset)
> set_bit(R5_Insync)
>
> And it could be:
>
> - reading from a replacement before it is recovered far enough; or
> - treating a not-yet-recovered device as in-sync for the current stripe.
>
> Fixes: db0505d32066 ("md: be cautious about using ->curr_resync_completed for ->recovery_offset")
>
> The race report:
> ==================================================================
> BUG: KCSAN: data-race in ops_run_io / reshape_request
>
> write to 0xffff8bdee168b270 of 8 bytes by task 1704 on cpu 10:
> reshape_request+0x1292/0x17b0
> raid5_sync_request+0x815/0xa00
> md_do_sync.cold+0xf8d/0x1516
> [......]
>
> read to 0xffff8bdee168b270 of 8 bytes by task 1696 on cpu 9:
> ops_run_io+0xc25/0x1960
> handle_stripe+0x2273/0x4570
> handle_active_stripes.isra.0+0x6e0/0xa50
> raid5d+0x7d5/0xb90
> [......]
>
> value changed: 0x0000000000091a00 -> 0x0000000000091b00
> ==================================================================
>
> Signed-off-by: Chen Cheng<chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: complete discard bios while reshape is active
From: sashiko-bot @ 2026-07-11 16:27 UTC (permalink / raw)
To: Genjian; +Cc: linux-raid, yukuai
In-Reply-To: <20260711161326.962336-1-zhanggenjian@126.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Completing blocking bios with BLK_STS_AGAIN violates block layer semantics and causes unexpected errors in synchronous paths.
Pre-existing issues:
- [High] make_discard_request() leaks the bio and causes an uninterruptible hang if REQ_NOWAIT is set.
--
--- Patch [1]: [PATCH] md/raid5: complete discard bios while reshape is active ---
commit f117d558ba13364f82645f866dea16210cadce51
Author: Genjian Zhang <zhanggenjian@kylinos.cn>
md/raid5: complete discard bios while reshape is active
This patch modifies the discard request handling to complete bios with
BLK_STS_AGAIN when an mdraid reshape is in progress. This prevents callers
from hanging in uninterruptible sleep waiting for a completion that never
arrives.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5722,9 +5722,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
> if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
> return;
[Severity: High]
This isn't a bug introduced by this patch, but does this early return leak
the bio and cause an uninterruptible hang if REQ_NOWAIT is set?
If a caller submits a discard bio with REQ_NOWAIT, the driver encounters this
warning and returns immediately without calling bio_endio(). Because the
caller assumes the bio was successfully handled, could this lead to a
resource leak and task hang?
>
> - if (mddev->reshape_position != MaxSector)
> + if (mddev->reshape_position != MaxSector) {
> /* Skip discard while reshape is happening */
> + bio_endio_status(bi, BLK_STS_AGAIN);
[Severity: High]
Does completing a blocking bio with BLK_STS_AGAIN violate block layer
semantics?
Since the REQ_NOWAIT bios are filtered out just above this check, this path
is only reached by synchronous, blocking discard bios. Completing them with
BLK_STS_AGAIN will propagate an unexpected -EAGAIN error to synchronous IO
paths and userspace applications.
Block APIs typically handle BLK_STS_NOTSUPP gracefully, but passing -EAGAIN
back through synchronous paths might cause filesystem transaction aborts or
userspace failures. Should this return BLK_STS_NOTSUPP instead?
> return;
> + }
>
> if (!raid5_discard_limits(mddev, bi))
> return;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260711161326.962336-1-zhanggenjian@126.com?part=1
^ permalink raw reply
* Re: [PATCH v3] md/raid5: fix reshape deadlock while failed devices more than max degraded
From: yu kuai @ 2026-07-11 16:39 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260624075824.2601110-1-chencheng@fnnas.com>
在 2026/6/24 15:58, Chen Cheng 写道:
> From: Chen Cheng<chencheng@fnnas.com>
>
> reshape stripe lifetime:
> - start reshape ==> reshape_request():
> * get destination stripe,
> - if need to copy source data chunks, set STRIPE_EXPANDING;
> - or, if new regions past the old end of the array, zero-filled,
> no need source data, set STRIPE_EXPANDING | STRIPE_READY
> * get source stripe,
> - set STRIPE_EXPAND_SOURCE
>
> - handle expand stripe ==> handle_stripe():
> reshape use reconstruct-write to construct stripe,
> four stages:
> 1. prepare source data chunks for old geometry stripe
> - fill source stripe data by read or compute
> 2. move data from old geometry source stripe to new geometry
> destination stripe
> - source stripe clear STRIPE_EXPAND_SOURCE
> - drain data from source to destination stripe
> - mark stripe chunk as R5_Expanded|R5_UPTODATE when the
> drain from source chunk to destionation chunk is completed
> - all stripe chunks drain are completed, then mark
> STRIPE_EXPAND_READY
> 3. calculate p/q chunks for destination stripe
> - if destination stripe does't depends on source dstripe,
> then we can clear STRIPE_EXPANDING
> 4. write-out to disks and release
> - set R5_Wantwrite|R5_Locked, writeout to disk
> - if write-out successed, clear STRIPE_EXPAND_READY, and
> decrement reshape_stripe, call md_done_sync() to report
> reshape progress.
>
> 1. cleanup the following kinds of **destination stripe**
> when failed device more than max degraded:
> - new regions past the old end of the array, zero-filled in place,
> requires no source data.
> (STRIPE_EXPANDING | STRIPE_EXPAND_READY)
> - prepare source data chunks already done, and writeout failed
> (STRIPE_EXPAND_READY)
>
> 2. destination stripes that need source data
> (STRIPE_EXPANDING, no STRIPE_HANDLE)
> - these kind of stripes sit idle in the stripe cache and are never seen
> by handle_stripe(). So clean up indirectly when thier source stripe
> (type 3) is processed.
>
> 3. source stripes (STRIPE_EXPAND_SOURCE)
> - hit handle_stripe() after thier member disks are markded Faulty.
> - clear STRIPE_EXPAND_SOURCE, finds and cleanup all dependent destination
> stripes that were waiting for data.
> - walks the source's data disks, compute the corresponding destination
> sector, looks up the destination stripe, and do cleanup(clear flags,
> dec counters, call md_done_sync())
>
> Reproducer:
> - Create a 4-disk RAID5 with mdadm on top of 5 disposable test disks
> wrapped by dm targets.
> - Add the 5th device as a spare and start a 4 -> 5 reshape.
> - Wait until /sys/block/mdX/md/sync_action reports "reshape".
> - Inject failures on two members so reshape exceeds max_degraded.
> - After a few seconds, write "frozen" to /sys/block/mdX/md/sync_action.
> Before this fix, the write blocks indefinitely.
>
> Read-error variant:
> - Use dm-dust on /dev/sd[b-f].
> - Preload bad blocks on two source members, e.g. dust0 and dust1:
> dmsetup message dust0 0 addbadblock <range>
> dmsetup message dust1 0 addbadblock <range>
> - Start reshape:
> mdadm -C /dev/mdX -e 1.2 -l 5 -n 4 -c 64 --assume-clean /dev/mapper/dust{0..3}
> mdadm --manage /dev/mdX --add /dev/mapper/dust4
> mdadm --grow /dev/mdX -n 5 --backup-file=/tmp/grow.backup &
> - Once reshape starts, enable the injected read failures:
> dmsetup message dust0 0 enable
> dmsetup message dust1 0 enable
> - Then:
> echo frozen > /sys/block/mdX/md/sync_action
> hangs forever before the fix.
>
> Write-error variant:
> - Use dm-flakey on /dev/sd[b-f].
> - Start the same 4 -> 5 reshape on flakey0..flakey4.
> - Once reshape starts, switch two members, e.g. flakey3 and flakey4,
> to error_writes.
> - Then:
> echo frozen > /sys/block/mdX/md/sync_action
> hangs forever before the fix.
>
> 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));
>
> After the fix recovery_active drains to zero, md_do_sync() prints
>
> md/raid:md0: Cannot continue operation (2/5 failed).
> md: md0: reshape interrupted.
>
> v2 -> v3:
> - just kick sashiko-bot to review my patch..
>
> changes v1 -> v2:
> - handle reshape write deadlock while failed devices more than max degraded
>
> Signed-off-by: Chen Cheng<chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 74 insertions(+)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
From: yu kuai @ 2026-07-11 17:43 UTC (permalink / raw)
To: Chen Cheng, linux-raid; +Cc: linux-kernel
In-Reply-To: <20260711100352.425177-4-chencheng@fnnas.com>
Hi,
在 2026/7/11 18:03, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> origin flow:
>
> bio_endio(master_bio); /* may drop active_io to zero */
> allow_barrier(conf);
> free_r10bio(r10_bio); /* reads conf->geo, returns to pool */
>
> one scenario is:
>
> CPU A (softirq, raid_end_bio_io) CPU B (action_store) --> reshape
> ================================ ===============================
> bio_endio(master_bio)
> md_end_clone_io
> percpu_ref_put -> 0
> wait_event wakeup, and,
> mddev_suspend return
> raid10_start_reshape:
> setup_geo(&conf->geo, new)
> ...
> mempool_destroy(old_pool)
> conf->r10bio_pool = new_pool
> allow_barrier(conf)
> free_r10bio(r10_bio)
> put_all_bios:
> for (i=0; i<conf->geo.raid_disks; i++)
> ==> old obj, new geo, OOB
> mempool_free(r10_bio, conf->r10bio_pool)
> ==> old-geometry obj freed into new pool
>
> so .. fix by reorder the flow:
>
> free_r10bio(r10_bio)
> allow_barrier(conf)
> bio_endio(master_bio)
>
> raid_end_discard_bio() is exactly the same.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid10.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index db68fcc9e9be..d77f60db7660 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -329,24 +329,27 @@ static void reschedule_retry(struct r10bio *r10_bio)
> */
> static void raid_end_bio_io(struct r10bio *r10_bio)
> {
> struct bio *bio = r10_bio->master_bio;
> struct r10conf *conf = r10_bio->mddev->private;
> + unsigned long state = r10_bio->state;
> + bool returned;
>
> - if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
> - if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
> - bio->bi_status = BLK_STS_IOERR;
> + returned = test_and_set_bit(R10BIO_Returned, &state);
> + if (!returned && !test_bit(R10BIO_Uptodate, &state))
> + bio->bi_status = BLK_STS_IOERR;
> +
> + free_r10bio(r10_bio);
> +
> + if (!returned)
> bio_endio(bio);
> - }
Apparently this change is not enough, the R10BIO_Returned case still return
the master_bio before freeing r10_bio.
I think you should get an active_io reference while allocating r10_bio to
guarantee that no r10_bio is active while array is suspended.
>
> /*
> * Wake up any possible resync thread that waits for the device
> * to go idle.
> */
> allow_barrier(conf);
> -
> - free_r10bio(r10_bio);
> }
>
> /*
> * Update disk head position estimator based on IRQ completion info.
> */
> @@ -1579,13 +1582,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
> if (!test_bit(R10BIO_Discard, &r10bio->state)) {
> first_r10bio = (struct r10bio *)r10bio->master_bio;
> free_r10bio(r10bio);
> r10bio = first_r10bio;
> } else {
> + struct bio *master_bio = r10bio->master_bio;
> +
> md_write_end(r10bio->mddev);
> - bio_endio(r10bio->master_bio);
> free_r10bio(r10bio);
> + bio_endio(master_bio);
> break;
> }
> }
> }
>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v1] md,dm: caller-owned memalloc_noio token in mddev_suspend()/resume()
From: yu kuai @ 2026-07-11 18:08 UTC (permalink / raw)
To: Chen Cheng, linux-raid; +Cc: linux-kernel, yu kuai
In-Reply-To: <20260711064845.155682-1-chencheng@fnnas.com>
Hi,
在 2026/7/11 14:48, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> Save token as mddev-scoped in mddev->noio_flags cause PF_MEMALLOC_NOIO
> leak into task A, while task B restores a token that it never saved.
>
> scenario:
>
> task A mddev task B
> ====== ======= ============
> write suspend_lo
> mddev_suspend()
> suspended == 0
> drain active_io
> suspended = 1
> A: noio_A = memalloc_noio_save()
> A returns with PF_MEMALLOC_NOIO set
>
> write suspend_hi
> mddev_suspend()
> suspended == 1
> suspended = 2
> B returns
> (no save)
>
> mddev_resume()
> suspended = 1
> not last resume
> A returns
> A still has PF_MEMALLOC_NOIO <-- leaked
>
> mddev_resume()
> suspended = 0
> memalloc_noio_restore(noio_A)
> (restores A's token in B)
>
> Fixed by:
> for md-raid side:
> - return each caller's noio_flags from mddev_suspend()
> - pass that token back into mddev_resume()
> - update the suspend-and-lock helpers to carry the token
> for dm-raid side:
> - just bypass because: dm-raid call suspend() or resume() will return to
> userland, so is not paired in the same task context.
>
> Validation:
> repeatedly updates the array's suspend_lo and suspend_hi sysfs from many
> concurrent userspace workers. That makes multiple tasks to call
> mddev_suspend()/mddev_resume() concurrently.
>
> Each worker:
> - reads its initial /proc/self/stat flags and verifies that PF_MEMALLOC_NOIO is not already
> set
> - writes 0 to either suspend_lo or suspend_hi
> - immediately reads its own task flags again
> - reports success if flags & PF_MEMALLOC_NOIO is true after the write returns
>
> Link: https://github.com/chencheng-fnnas/reproducer/blob/main/repro-md-noio-token-leak.sh
>
> Fixes: 78f57ef9d50a ("md: use memalloc scope APIs in mddev_suspend()/mddev_resume()")
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
>
> rfc -> v1:
> - dm-raid side just bypass because suspend and resume not in the
> same task context..
>
> ---
> drivers/md/dm-raid.c | 6 +--
> drivers/md/md-autodetect.c | 5 ++-
> drivers/md/md-bitmap.c | 12 ++---
> drivers/md/md.c | 90 +++++++++++++++++++++++---------------
> drivers/md/md.h | 23 +++++-----
> drivers/md/raid5-cache.c | 11 +++--
> drivers/md/raid5.c | 24 +++++-----
> 7 files changed, 101 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index 8f5a5e1342a9..ce0f562c5da3 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -3251,11 +3251,11 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
> /* Start raid set read-only and assumed clean to change in raid_resume() */
> rs->md.ro = MD_RDONLY;
> rs->md.in_sync = 1;
>
> /* Has to be held on running the array */
> - mddev_suspend_and_lock_nointr(&rs->md);
> + mddev_suspend_and_lock_nointr(&rs->md, NULL);
>
> /* Keep array frozen until resume. */
> md_frozen_sync_thread(&rs->md);
>
> r = md_run(&rs->md);
> @@ -3863,11 +3863,11 @@ static void raid_postsuspend(struct dm_target *ti)
> /*
> * sync_thread must be stopped during suspend, and writes have
> * to be stopped before suspending to avoid deadlocks.
> */
> md_stop_writes(&rs->md);
> - mddev_suspend(&rs->md, false);
> + mddev_suspend(&rs->md, false, NULL);
> rs->md.ro = MD_RDONLY;
> }
> clear_bit(MD_DM_SUSPENDING, &mddev->flags);
>
> }
> @@ -4141,11 +4141,11 @@ static void raid_resume(struct dm_target *ti)
> lockdep_is_held(&mddev->reconfig_mutex)));
> clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
> mddev->ro = MD_RDWR;
> mddev->in_sync = 0;
> md_unfrozen_sync_thread(mddev);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, 0);
Personally I don't like the coding style here.
Perhaps you can just move memalloc_noio_save() out of mddev_suspend(), so dm raid doesn't
need to change at all. And for mdraid, you can just call flags = memalloc_noio_save()
directly in mdraid, where: 1) after array suspend succeed; 2) before allocating memory,
means you don't have to do this for all the places to suspend array;
> }
> }
>
> static struct target_type raid_target = {
> .name = "raid",
> diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
> index 4b80165afd23..58e062cd0580 100644
> --- a/drivers/md/md-autodetect.c
> +++ b/drivers/md/md-autodetect.c
> @@ -126,10 +126,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
> dev_t devices[MD_SB_DISKS + 1], mdev;
> struct mdu_array_info_s ainfo = { };
> struct mddev *mddev;
> int err = 0, i;
> char name[16];
> + unsigned int noio_flags;
>
> if (args->partitioned) {
> mdev = MKDEV(mdp_major, args->minor << MdpMinorShift);
> sprintf(name, "md_d%d", args->minor);
> } else {
> @@ -173,11 +174,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
> if (IS_ERR(mddev)) {
> pr_err("md: md_alloc failed - cannot start array %s\n", name);
> return;
> }
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err) {
> pr_err("md: failed to lock array %s\n", name);
> goto out_mddev_put;
> }
>
> @@ -219,11 +220,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
> if (!err)
> err = do_md_run(mddev);
> if (err)
> pr_warn("md: starting %s failed\n", name);
> out_unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> out_mddev_put:
> mddev_put(mddev);
> }
>
> static int __init raid_setup(char *str)
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 028b9ca8ce52..74b7f569a3f4 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2620,13 +2620,14 @@ location_show(struct mddev *mddev, char *page)
> }
>
> static ssize_t
> location_store(struct mddev *mddev, const char *buf, size_t len)
> {
> + unsigned int noio_flags;
> int rv;
>
> - rv = mddev_suspend_and_lock(mddev);
> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
> if (rv)
> return rv;
>
> if (mddev->pers) {
> if (mddev->recovery || mddev->sync_thread) {
> @@ -2711,11 +2712,11 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> md_wakeup_thread(mddev->thread);
> }
> rv = 0;
> out:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> if (rv)
> return rv;
> return len;
>
> merge_err:
> @@ -2831,17 +2832,18 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned long backlog;
> unsigned long old_mwb = mddev->bitmap_info.max_write_behind;
> struct md_rdev *rdev;
> bool has_write_mostly = false;
> + unsigned int noio_flags;
> int rv = kstrtoul(buf, 10, &backlog);
> if (rv)
> return rv;
> if (backlog > COUNTER_MAX)
> return -EINVAL;
>
> - rv = mddev_suspend_and_lock(mddev);
> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
> if (rv)
> return rv;
>
> /*
> * Without write mostly device, it doesn't make sense to set
> @@ -2854,11 +2856,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
> }
> }
> if (!has_write_mostly) {
> pr_warn_ratelimited("%s: can't set backlog, no write mostly device available\n",
> mdname(mddev));
> - mddev_unlock(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return -EINVAL;
> }
>
> mddev->bitmap_info.max_write_behind = backlog;
> if (!backlog && mddev->serial_info_pool) {
> @@ -2871,11 +2873,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
> mddev_create_serial_pool(mddev, rdev);
> }
> if (old_mwb != backlog)
> bitmap_update_sb(mddev->bitmap);
>
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return len;
> }
>
> static struct md_sysfs_entry bitmap_backlog =
> __ATTR(backlog, S_IRUGO|S_IWUSR, backlog_show, backlog_store);
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 1377c407614c..04e10769d5c6 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -459,11 +459,12 @@ static void md_submit_bio(struct bio *bio)
>
> /*
> * Make sure no new requests are submitted to the device, and any requests that
> * have been submitted are completely handled.
> */
> -int mddev_suspend(struct mddev *mddev, bool interruptible)
> +int mddev_suspend(struct mddev *mddev, bool interruptible,
> + unsigned int *noio_flags)
> {
> int err = 0;
>
> /*
> * hold reconfig_mutex to wait for normal io will deadlock, because
> @@ -477,11 +478,16 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
> else
> mutex_lock(&mddev->suspend_mutex);
> if (err)
> return err;
>
> + if (noio_flags)
> + *noio_flags = 0;
> +
> if (mddev->suspended) {
> + if (noio_flags && !mddev_is_dm(mddev))
> + *noio_flags = memalloc_noio_save();
> WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
> mutex_unlock(&mddev->suspend_mutex);
> return 0;
> }
>
> @@ -515,31 +521,31 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
> * prevent deadlock.
> */
> WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
>
> /* restrict memory reclaim I/O during raid array is suspend */
> - mddev->noio_flag = memalloc_noio_save();
> + if (noio_flags && !mddev_is_dm(mddev))
> + *noio_flags = memalloc_noio_save();
>
> mutex_unlock(&mddev->suspend_mutex);
> return 0;
> }
> EXPORT_SYMBOL_GPL(mddev_suspend);
>
> -static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
> +static void __mddev_resume(struct mddev *mddev, bool recovery_needed,
> + unsigned int noio_flags)
> {
> lockdep_assert_not_held(&mddev->reconfig_mutex);
>
> mutex_lock(&mddev->suspend_mutex);
> + memalloc_noio_restore(noio_flags);
> WRITE_ONCE(mddev->suspended, mddev->suspended - 1);
> if (mddev->suspended) {
> mutex_unlock(&mddev->suspend_mutex);
> return;
> }
>
> - /* entred the memalloc scope from mddev_suspend() */
> - memalloc_noio_restore(mddev->noio_flag);
> -
> percpu_ref_resurrect(&mddev->active_io);
> wake_up(&mddev->sb_wait);
>
> if (recovery_needed)
> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
> @@ -547,13 +553,13 @@ static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
> md_wakeup_thread(mddev->sync_thread); /* possibly kick off a reshape */
>
> mutex_unlock(&mddev->suspend_mutex);
> }
>
> -void mddev_resume(struct mddev *mddev)
> +void mddev_resume(struct mddev *mddev, unsigned int noio_flags)
> {
> - return __mddev_resume(mddev, true);
> + return __mddev_resume(mddev, true, noio_flags);
> }
> EXPORT_SYMBOL_GPL(mddev_resume);
>
> /* sync bdev before setting device to readonly or stopping raid*/
> static int mddev_set_closing_and_sync_blockdev(struct mddev *mddev, int opener_num)
> @@ -3737,10 +3743,11 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
> {
> struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr);
> struct md_rdev *rdev = container_of(kobj, struct md_rdev, kobj);
> struct kernfs_node *kn = NULL;
> bool suspend = false;
> + unsigned int noio_flags = 0;
> ssize_t rv;
> struct mddev *mddev = READ_ONCE(rdev->mddev);
>
> if (!entry->store)
> return -EIO;
> @@ -3756,17 +3763,17 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
> cmd_match(page, "writemostly") ||
> cmd_match(page, "-writemostly"))
> suspend = true;
> }
>
> - rv = suspend ? mddev_suspend_and_lock(mddev) : mddev_lock(mddev);
> + rv = suspend ? mddev_suspend_and_lock(mddev, &noio_flags) : mddev_lock(mddev);
> if (!rv) {
> if (rdev->mddev == NULL)
> rv = -ENODEV;
> else
> rv = entry->store(rdev, page, length);
> - suspend ? mddev_unlock_and_resume(mddev) : mddev_unlock(mddev);
> + suspend ? mddev_unlock_and_resume(mddev, noio_flags) : mddev_unlock(mddev);
> }
>
> if (kn)
> sysfs_unbreak_active_protection(kn);
>
> @@ -4049,15 +4056,16 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> size_t slen = len;
> struct md_personality *pers, *oldpers;
> long level;
> void *priv, *oldpriv;
> struct md_rdev *rdev;
> + unsigned int noio_flags;
>
> if (slen == 0 || slen >= sizeof(clevel))
> return -EINVAL;
>
> - rv = mddev_suspend_and_lock(mddev);
> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
> if (rv)
> return rv;
>
> if (mddev->pers == NULL) {
> memcpy(mddev->clevel, buf, slen);
> @@ -4231,11 +4239,11 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> md_update_sb(mddev, 1);
> sysfs_notify_dirent_safe(mddev->sysfs_level);
> md_new_event();
> rv = len;
> out_unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return rv;
> }
>
> static struct md_sysfs_entry md_level =
> __ATTR(level, S_IRUGO|S_IWUSR, level_show, level_store);
> @@ -4410,17 +4418,18 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks);
>
> static ssize_t
> raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned int n;
> + unsigned int noio_flags;
> int err;
>
> err = kstrtouint(buf, 10, &n);
> if (err < 0)
> return err;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> if (mddev->pers) {
> if (n != mddev->raid_disks)
> err = update_raid_disks(mddev, n);
> @@ -4442,11 +4451,11 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
> mddev->raid_disks = n;
> mddev->reshape_backwards = (mddev->delta_disks < 0);
> } else
> mddev->raid_disks = n;
> out_unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return err ? err : len;
> }
> static struct md_sysfs_entry md_raid_disks =
> __ATTR(raid_disks, S_IRUGO|S_IWUSR, raid_disks_show, raid_disks_store);
>
> @@ -4822,10 +4831,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
> char *e;
> int major = simple_strtoul(buf, &e, 10);
> int minor;
> dev_t dev;
> struct md_rdev *rdev;
> + unsigned int noio_flags;
> int err;
>
> if (!*buf || *e != ':' || !e[1] || e[1] == '\n')
> return -EINVAL;
> minor = simple_strtoul(e+1, &e, 10);
> @@ -4834,11 +4844,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
> dev = MKDEV(major, minor);
> if (major != MAJOR(dev) ||
> minor != MINOR(dev))
> return -EOVERFLOW;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> if (mddev->persistent) {
> rdev = md_import_device(dev, mddev->major_version,
> mddev->minor_version);
> @@ -4855,18 +4865,18 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
> rdev = md_import_device(dev, -2, -1);
> else
> rdev = md_import_device(dev, -1, -1);
>
> if (IS_ERR(rdev)) {
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return PTR_ERR(rdev);
> }
> err = bind_rdev_to_array(rdev, mddev);
> out:
> if (err)
> export_rdev(rdev);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> if (!err)
> md_new_event();
> return err ? err : len;
> }
>
> @@ -5257,28 +5267,29 @@ static int mddev_start_reshape(struct mddev *mddev)
> static ssize_t
> action_store(struct mddev *mddev, const char *page, size_t len)
> {
> int ret;
> enum sync_action action;
> + unsigned int noio_flags = 0;
>
> if (!mddev->pers || !mddev->pers->sync_request)
> return -EINVAL;
>
> action = md_sync_action_by_name(page);
> retry:
> if (work_busy(&mddev->sync_work))
> flush_work(&mddev->sync_work);
>
> ret = (action == ACTION_RESHAPE) ?
> - mddev_suspend_and_lock(mddev) :
> + mddev_suspend_and_lock(mddev, &noio_flags) :
> mddev_lock(mddev);
> if (ret)
> return ret;
>
> if (work_busy(&mddev->sync_work)) {
> if (action == ACTION_RESHAPE)
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> else
> mddev_unlock(mddev);
> goto retry;
> }
>
> @@ -5349,11 +5360,11 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> sysfs_notify_dirent_safe(mddev->sysfs_action);
> ret = len;
>
> out:
> if (action == ACTION_RESHAPE)
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> else
> mddev_unlock(mddev);
> return ret;
> }
>
> @@ -5640,24 +5651,25 @@ suspend_lo_show(struct mddev *mddev, char *page)
>
> static ssize_t
> suspend_lo_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned long long new;
> + unsigned int noio_flags;
> int err;
>
> err = kstrtoull(buf, 10, &new);
> if (err < 0)
> return err;
> if (new != (sector_t)new)
> return -EINVAL;
>
> - err = mddev_suspend(mddev, true);
> + err = mddev_suspend(mddev, true, &noio_flags);
> if (err)
> return err;
>
> WRITE_ONCE(mddev->suspend_lo, new);
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
>
> return len;
> }
> static struct md_sysfs_entry md_suspend_lo =
> __ATTR(suspend_lo, S_IRUGO|S_IWUSR, suspend_lo_show, suspend_lo_store);
> @@ -5671,24 +5683,25 @@ suspend_hi_show(struct mddev *mddev, char *page)
>
> static ssize_t
> suspend_hi_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned long long new;
> + unsigned int noio_flags;
> int err;
>
> err = kstrtoull(buf, 10, &new);
> if (err < 0)
> return err;
> if (new != (sector_t)new)
> return -EINVAL;
>
> - err = mddev_suspend(mddev, true);
> + err = mddev_suspend(mddev, true, &noio_flags);
> if (err)
> return err;
>
> WRITE_ONCE(mddev->suspend_hi, new);
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
>
> return len;
> }
> static struct md_sysfs_entry md_suspend_hi =
> __ATTR(suspend_hi, S_IRUGO|S_IWUSR, suspend_hi_show, suspend_hi_store);
> @@ -5928,19 +5941,20 @@ static ssize_t serialize_policy_show(struct mddev *mddev, char *page)
> static ssize_t
> serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
> {
> int err;
> bool value;
> + unsigned int noio_flags;
>
> err = kstrtobool(buf, &value);
> if (err)
> return err;
>
> if (value == test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
> return len;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> if (mddev->pers == NULL || (mddev->pers->head.id != ID_RAID1)) {
> pr_err("md: serialize_policy is only effective for raid1\n");
> err = -EINVAL;
> @@ -5953,11 +5967,11 @@ serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
> } else {
> mddev_destroy_serial_pool(mddev, NULL);
> clear_bit(MD_SERIALIZE_POLICY, &mddev->flags);
> }
> unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return err ?: len;
> }
>
> static struct md_sysfs_entry md_serialize_policy =
> __ATTR(serialize_policy, S_IRUGO | S_IWUSR, serialize_policy_show,
> @@ -6263,21 +6277,22 @@ EXPORT_SYMBOL_GPL(mddev_stack_new_rdev);
>
> /* update the optimal I/O size after a reshape */
> void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes)
> {
> struct queue_limits lim;
> + unsigned int noio_flags;
>
> if (mddev_is_dm(mddev))
> return;
>
> /* don't bother updating io_opt if we can't suspend the array */
> - if (mddev_suspend(mddev, false) < 0)
> + if (mddev_suspend(mddev, false, &noio_flags) < 0)
> return;
> lim = queue_limits_start_update(mddev->gendisk->queue);
> lim.io_opt = lim.io_min * nr_stripes;
> queue_limits_commit_update(mddev->gendisk->queue, &lim);
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
> }
> EXPORT_SYMBOL_GPL(mddev_update_io_opt);
>
> static void mddev_delayed_delete(struct work_struct *ws)
> {
> @@ -7255,10 +7270,11 @@ static void autorun_array(struct mddev *mddev)
> */
> static void autorun_devices(int part)
> {
> struct md_rdev *rdev0, *rdev, *tmp;
> struct mddev *mddev;
> + unsigned int noio_flags;
>
> pr_info("md: autorun ...\n");
> while (!list_empty(&pending_raid_disks)) {
> int unit;
> dev_t dev;
> @@ -7295,27 +7311,27 @@ static void autorun_devices(int part)
>
> mddev = md_alloc(dev, NULL);
> if (IS_ERR(mddev))
> break;
>
> - if (mddev_suspend_and_lock(mddev))
> + if (mddev_suspend_and_lock(mddev, &noio_flags))
> pr_warn("md: %s locked, cannot run\n", mdname(mddev));
> else if (mddev->raid_disks || mddev->major_version
> || !list_empty(&mddev->disks)) {
> pr_warn("md: %s already running, cannot run %pg\n",
> mdname(mddev), rdev0->bdev);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> } else {
> pr_debug("md: created %s\n", mdname(mddev));
> mddev->persistent = 1;
> rdev_for_each_list(rdev, tmp, &candidates) {
> list_del_init(&rdev->same_set);
> if (bind_rdev_to_array(rdev, mddev))
> export_rdev(rdev);
> }
> autorun_array(mddev);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> }
> /* on success, candidates will be empty, on error
> * it won't...
> */
> rdev_for_each_list(rdev, tmp, &candidates) {
> @@ -8329,10 +8345,11 @@ static int __md_set_array_info(struct mddev *mddev, void __user *argp)
>
> static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
> unsigned int cmd, unsigned long arg)
> {
> int err = 0;
> + unsigned int noio_flags = 0;
> void __user *argp = (void __user *)arg;
> struct mddev *mddev = NULL;
>
> err = md_ioctl_valid(cmd);
> if (err)
> @@ -8380,11 +8397,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
> }
>
> if (!md_is_rdwr(mddev))
> flush_work(&mddev->sync_work);
>
> - err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev) :
> + err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev, &noio_flags) :
> mddev_lock(mddev);
> if (err) {
> pr_debug("md: ioctl lock interrupted, reason %d, cmd %d\n",
> err, cmd);
> goto out;
> @@ -8511,11 +8528,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
> unlock:
> if (mddev->hold_active == UNTIL_IOCTL &&
> err != -EINVAL)
> mddev->hold_active = 0;
>
> - md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev) :
> + md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev, noio_flags) :
> mddev_unlock(mddev);
>
> out:
> if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
> clear_bit(MD_CLOSING, &mddev->flags);
> @@ -10180,20 +10197,21 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
> static void md_start_sync(struct work_struct *ws)
> {
> struct mddev *mddev = container_of(ws, struct mddev, sync_work);
> int spares = 0;
> bool suspend = false;
> + unsigned int noio_flags = 0;
> char *name;
>
> /*
> * If reshape is still in progress, spares won't be added or removed
> * from conf until reshape is done.
> */
> if (mddev->reshape_position == MaxSector &&
> md_spares_need_change(mddev)) {
> suspend = true;
> - mddev_suspend(mddev, false);
> + mddev_suspend(mddev, false, &noio_flags);
> }
>
> mddev_lock_nointr(mddev);
> if (!md_is_rdwr(mddev)) {
> /*
> @@ -10237,11 +10255,11 @@ static void md_start_sync(struct work_struct *ws)
> * not set it again. Otherwise, we may cause issue like this one:
> * https://bugzilla.kernel.org/show_bug.cgi?id=218200
> * Therefore, use __mddev_resume(mddev, false).
> */
> if (suspend)
> - __mddev_resume(mddev, false);
> + __mddev_resume(mddev, false, noio_flags);
> md_wakeup_thread(mddev->sync_thread);
> sysfs_notify_dirent_safe(mddev->sysfs_action);
> md_new_event();
> return;
>
> @@ -10257,11 +10275,11 @@ static void md_start_sync(struct work_struct *ws)
> * not set it again. Otherwise, we may cause issue like this one:
> * https://bugzilla.kernel.org/show_bug.cgi?id=218200
> * Therefore, use __mddev_resume(mddev, false).
> */
> if (suspend)
> - __mddev_resume(mddev, false);
> + __mddev_resume(mddev, false, noio_flags);
>
> wake_up(&resync_wait);
> if (test_and_clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery) &&
> mddev->sysfs_action)
> sysfs_notify_dirent_safe(mddev->sysfs_action);
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index d8daf0f75cbb..7d92deea34f2 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -619,11 +619,10 @@ struct mddev {
> mempool_t *serial_info_pool;
> void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
> struct md_cluster_info *cluster_info;
> struct md_cluster_operations *cluster_ops;
> unsigned int good_device_nr; /* good device num within cluster raid */
> - unsigned int noio_flag; /* for memalloc scope API */
>
> /*
> * Temporarily store rdev that will be finally removed when
> * reconfig_mutex is unlocked, protected by reconfig_mutex.
> */
> @@ -953,12 +952,13 @@ extern void md_stop(struct mddev *mddev);
> extern void md_stop_writes(struct mddev *mddev);
> extern int md_rdev_init(struct md_rdev *rdev);
> extern void md_rdev_clear(struct md_rdev *rdev);
>
> extern bool md_handle_request(struct mddev *mddev, struct bio *bio);
> -extern int mddev_suspend(struct mddev *mddev, bool interruptible);
> -extern void mddev_resume(struct mddev *mddev);
> +extern int mddev_suspend(struct mddev *mddev, bool interruptible,
> + unsigned int *noio_flags);
> +extern void mddev_resume(struct mddev *mddev, unsigned int noio_flags);
> extern void md_idle_sync_thread(struct mddev *mddev);
> extern void md_frozen_sync_thread(struct mddev *mddev);
> extern void md_unfrozen_sync_thread(struct mddev *mddev);
>
> extern void md_update_sb(struct mddev *mddev, int force);
> @@ -999,35 +999,38 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
> if (bio_op(bio) == REQ_OP_WRITE_ZEROES &&
> !bio->bi_bdev->bd_disk->queue->limits.max_write_zeroes_sectors)
> mddev->gendisk->queue->limits.max_write_zeroes_sectors = 0;
> }
>
> -static inline int mddev_suspend_and_lock(struct mddev *mddev)
> +static inline int mddev_suspend_and_lock(struct mddev *mddev,
> + unsigned int *noio_flags)
> {
> int ret;
>
> - ret = mddev_suspend(mddev, true);
> + ret = mddev_suspend(mddev, true, noio_flags);
> if (ret)
> return ret;
>
> ret = mddev_lock(mddev);
> if (ret)
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags ? *noio_flags : 0);
>
> return ret;
> }
>
> -static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev)
> +static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev,
> + unsigned int *noio_flags)
> {
> - mddev_suspend(mddev, false);
> + mddev_suspend(mddev, false, noio_flags);
> mddev_lock_nointr(mddev);
> }
>
> -static inline void mddev_unlock_and_resume(struct mddev *mddev)
> +static inline void mddev_unlock_and_resume(struct mddev *mddev,
> + unsigned int noio_flags)
> {
> mddev_unlock(mddev);
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
> }
>
> struct mdu_array_info_s;
> struct mdu_disk_info_s;
>
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index 7b7546bfa21f..6f8e3a624456 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -693,13 +693,15 @@ static void r5c_disable_writeback_async(struct work_struct *work)
> !READ_ONCE(conf->log) ||
> !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
>
> log = READ_ONCE(conf->log);
> if (log) {
> - mddev_suspend(mddev, false);
> + unsigned int noio_flags;
> +
> + mddev_suspend(mddev, false, &noio_flags);
> log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
> }
> }
>
> static void r5l_submit_current_io(struct r5l_log *log)
> {
> @@ -2603,10 +2605,11 @@ EXPORT_SYMBOL(r5c_journal_mode_set);
> static ssize_t r5c_journal_mode_store(struct mddev *mddev,
> const char *page, size_t length)
> {
> int mode = ARRAY_SIZE(r5c_journal_mode_str);
> size_t len = length;
> + unsigned int noio_flags;
> int ret;
>
> if (len < 2)
> return -EINVAL;
>
> @@ -2615,15 +2618,15 @@ static ssize_t r5c_journal_mode_store(struct mddev *mddev,
>
> while (mode--)
> if (strlen(r5c_journal_mode_str[mode]) == len &&
> !strncmp(page, r5c_journal_mode_str[mode], len))
> break;
> - ret = mddev_suspend_and_lock(mddev);
> + ret = mddev_suspend_and_lock(mddev, &noio_flags);
> if (ret)
> return ret;
> ret = r5c_journal_mode_set(mddev, mode);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return ret ?: length;
> }
>
> struct md_sysfs_entry
> r5c_journal_mode = __ATTR(journal_mode, 0644,
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 65ae7d8930fc..482872ae7198 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6993,10 +6993,11 @@ raid5_show_stripe_size(struct mddev *mddev, char *page)
> static ssize_t
> raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
> {
> struct r5conf *conf;
> unsigned long new;
> + unsigned int noio_flags;
> int err;
> int size;
>
> if (len >= PAGE_SIZE)
> return -EINVAL;
> @@ -7011,11 +7012,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
> if (new % DEFAULT_STRIPE_SIZE != 0 ||
> new > PAGE_SIZE || new == 0 ||
> new != roundup_pow_of_two(new))
> return -EINVAL;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
>
> conf = mddev->private;
> if (!conf) {
> @@ -7049,11 +7050,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
> err = -ENOMEM;
> }
> mutex_unlock(&conf->cache_size_mutex);
>
> out_unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return err ?: len;
> }
>
> static struct md_sysfs_entry
> raid5_stripe_size = __ATTR(stripe_size, 0644,
> @@ -7127,19 +7128,20 @@ raid5_show_skip_copy(struct mddev *mddev, char *page)
> static ssize_t
> raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
> {
> struct r5conf *conf;
> unsigned long new;
> + unsigned int noio_flags;
> int err;
>
> if (len >= PAGE_SIZE)
> return -EINVAL;
> if (kstrtoul(page, 10, &new))
> return -EINVAL;
> new = !!new;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> conf = mddev->private;
> if (!conf)
> err = -ENODEV;
> @@ -7152,11 +7154,11 @@ raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
> lim.features |= BLK_FEAT_STABLE_WRITES;
> else
> lim.features &= ~BLK_FEAT_STABLE_WRITES;
> err = queue_limits_commit_update(q, &lim);
> }
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return err ?: len;
> }
>
> static struct md_sysfs_entry
> raid5_skip_copy = __ATTR(skip_copy, S_IRUGO | S_IWUSR,
> @@ -7195,10 +7197,11 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt,
> static ssize_t
> raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
> {
> struct r5conf *conf;
> unsigned int new;
> + unsigned int noio_flags;
> int err;
> struct r5worker_group *new_groups, *old_groups;
> int group_cnt;
>
> if (len >= PAGE_SIZE)
> @@ -7207,16 +7210,16 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
> return -EINVAL;
> /* 8192 should be big enough */
> if (new > 8192)
> return -EINVAL;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> conf = mddev->private;
> if (!conf) {
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return -ENODEV;
> }
> raid5_quiesce(mddev, true);
>
> if (new != conf->worker_cnt_per_group) {
> @@ -7237,11 +7240,11 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
> kfree(old_groups);
> }
> }
>
> raid5_quiesce(mddev, false);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
>
> return err ?: len;
> }
>
> static struct md_sysfs_entry
> @@ -8940,18 +8943,19 @@ static void *raid6_takeover(struct mddev *mddev)
> }
>
> static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
> {
> struct r5conf *conf;
> + unsigned int noio_flags;
> int err;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> conf = mddev->private;
> if (!conf) {
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return -ENODEV;
> }
>
> if (strncmp(buf, "ppl", 3) == 0) {
> /* ppl only works with RAID 5 */
> @@ -8990,11 +8994,11 @@ static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
> }
>
> if (!err)
> md_update_sb(mddev, 1);
>
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
>
> return err;
> }
>
> static int raid5_start(struct mddev *mddev)
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
From: yu kuai @ 2026-07-11 18:16 UTC (permalink / raw)
To: Chen Cheng, linux-raid; +Cc: linux-kernel, yu kuai
In-Reply-To: <20260711025121.3991689-1-chencheng@fnnas.com>
Hi,
在 2026/7/11 10:51, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> sb_flags is checked without a lock in md, raid1, raid5, and raid10.
> KCSAN reports these reads as data races.
>
> The write side uses atomic bit ops.
> The read side still has plain loads in a few places.
>
> Use READ_ONCE() for the lockless reads of sb_flags.
>
> KCSAN reports #1:
> ======================================
>
> BUG: KCSAN: data-race in md_check_recovery / md_write_start
>
> write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
> md_write_start+0x5dd/0x910
> raid10_make_request+0x9b/0x1080
> md_handle_request+0x4a2/0xa40
> [........]
>
> read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
> md_check_recovery+0x574/0x900
> raid10d+0xb7/0x2950
> [........]
>
> KCSAN reports #2:
> ======================================
> BUG: KCSAN: data-race in md_check_recovery / md_write_start
>
> write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
> md_write_start+0x5dd/0x910
> raid1_make_request+0x141/0x1990
> [........]
>
> read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
> md_check_recovery+0x574/0x900
> raid1d+0xcc/0x3840
> [........]
>
> value changed: 0x0000000000000002 -> 0x0000000000000006
>
> KCSAN reports #3:
> ======================================
> BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold
>
> write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
> md_do_sync.cold+0x3f6/0x1686
> [........]
>
> read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
> md_check_recovery+0x16d/0x900
> raid1d+0xcc/0x3840
> [........]
>
> value changed: 0x0000000000000000 -> 0x0000000000000002
>
> KCSAN reports #4:
> ======================================
> BUG: KCSAN: data-race in md_do_sync.cold / raid5d
>
> write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
> md_do_sync.cold+0x3f6/0x1686
> md_thread+0x15a/0x2d0
> [........]
>
> read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
> raid5d+0x7f9/0xba0
> md_thread+0x15a/0x2d0
> [........]
>
> value changed: 0x0000000000000000 -> 0x0000000000000002
>
> Fixes: 2953079c692d ("md: separate flags for superblock changes")
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> v1 -> v2
> - pass checkpatch.pl check.
> ---
> drivers/md/md.c | 8 ++++----
> drivers/md/raid1.c | 2 +-
> drivers/md/raid10.c | 4 ++--
> drivers/md/raid5.c | 7 ++++---
> 4 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd..e72bf2010287 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
> * via sysfs - until a lack of spares is confirmed.
> */
> set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
>
> - if (mddev->sb_flags)
> + if (READ_ONCE(mddev->sb_flags))
> md_update_sb(mddev, 0);
>
> if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
> !mddev->bitmap_info.offset)
> md_bitmap_set_none(mddev);
> @@ -7024,11 +7024,11 @@ static void __md_stop_writes(struct mddev *mddev)
> mddev->bitmap_ops->flush(mddev);
> }
>
> if (md_is_rdwr(mddev) &&
> ((!mddev->in_sync && !mddev_is_clustered(mddev)) ||
> - mddev->sb_flags)) {
> + READ_ONCE(mddev->sb_flags))) {
> /* mark array as shutdown cleanly */
> if (!mddev_is_clustered(mddev))
> mddev->in_sync = 1;
> md_update_sb(mddev, 1);
> }
> @@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
> /*
> * MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
> * active, and no action is needed for now.
> * All other MD_SB_* flags require to update the superblock.
> */
> - if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~(1<<MD_SB_CHANGE_PENDING))
> return true;
So this should be a v3, and the above line still not fixed. Please check checkpatch
warnings.
>
> /*
> * If the array is not using external metadata and there has been no data
> * written for some time, then the array's status needs to be set to
> @@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
> spin_lock(&mddev->lock);
> set_in_sync(mddev);
> spin_unlock(&mddev->lock);
> }
>
> - if (mddev->sb_flags)
> + if (READ_ONCE(mddev->sb_flags))
> md_update_sb(mddev, 0);
>
> /*
> * Never start a new sync thread if MD_RECOVERY_RUNNING is
> * still set.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 29b58583e381..bd6808656edb 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2738,11 +2738,11 @@ static void raid1d(struct md_thread *thread)
> handle_read_error(conf, r1_bio);
> else
> WARN_ON_ONCE(1);
>
> cond_resched();
> - if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
> md_check_recovery(mddev);
> }
> blk_finish_plug(&plug);
> }
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index adaf9e432e25..3ffa5a19964d 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3030,11 +3030,11 @@ static void raid10d(struct md_thread *thread)
> handle_read_error(mddev, r10_bio);
> else
> WARN_ON_ONCE(1);
>
> cond_resched();
> - if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
> md_check_recovery(mddev);
> }
> blk_finish_plug(&plug);
> }
>
> @@ -4698,11 +4698,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> else
> mddev->curr_resync_completed = conf->reshape_progress;
> conf->reshape_checkpoint = jiffies;
> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> md_wakeup_thread(mddev->thread);
> - wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
> + wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
> test_bit(MD_RECOVERY_INTR, &mddev->recovery));
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
> allow_barrier(conf);
> return sectors_done;
> }
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ded6a69f7795..cb58b4353995 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -1219,11 +1219,11 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
> break;
>
> if (bad < 0) {
> set_bit(BlockedBadBlocks, &rdev->flags);
> if (!conf->mddev->external &&
> - conf->mddev->sb_flags) {
> + READ_ONCE(conf->mddev->sb_flags)) {
> /* It is very unlikely, but we might
> * still need to write out the
> * bad block log - better give it
> * a chance*/
> md_check_recovery(conf->mddev);
> @@ -6469,11 +6469,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
> rdev->recovery_offset = sector_nr;
>
> conf->reshape_checkpoint = jiffies;
> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> md_wakeup_thread(mddev->thread);
> - wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
> + wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
> test_bit(MD_RECOVERY_INTR, &mddev->recovery));
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
> return 0;
> spin_lock_irq(&conf->device_lock);
> conf->reshape_safe = mddev->reshape_position;
> @@ -6913,11 +6913,12 @@ static void raid5d(struct md_thread *thread)
> conf->temp_inactive_list);
> if (!batch_size && !released)
> break;
> handled += batch_size;
>
> - if (mddev->sb_flags & ~(1 << MD_SB_CHANGE_PENDING)) {
> + if (READ_ONCE(mddev->sb_flags) &
> + ~(1 << MD_SB_CHANGE_PENDING)) {
> spin_unlock_irq(&conf->device_lock);
> md_check_recovery(mddev);
> spin_lock_irq(&conf->device_lock);
> }
> }
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: complete discard bios while reshape is active
From: genjian zhang @ 2026-07-12 14:35 UTC (permalink / raw)
To: sashiko-reviews, sashiko-bot; +Cc: linux-raid, yukuai, zhanggenjian
In-Reply-To: <20260711162728.369B01F000E9@smtp.kernel.org>
At 2026-07-12 00:27:27, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
>New issues:
>- [High] Completing blocking bios with BLK_STS_AGAIN violates block layer semantics and causes unexpected errors in synchronous paths.
>
>Pre-existing issues:
>- [High] make_discard_request() leaks the bio and causes an uninterruptible hang if REQ_NOWAIT is set.
>--
>
>--- Patch [1]: [PATCH] md/raid5: complete discard bios while reshape is active ---
>commit f117d558ba13364f82645f866dea16210cadce51
>Author: Genjian Zhang <zhanggenjian@kylinos.cn>
>
>md/raid5: complete discard bios while reshape is active
>
>This patch modifies the discard request handling to complete bios with
>BLK_STS_AGAIN when an mdraid reshape is in progress. This prevents callers
>from hanging in uninterruptible sleep waiting for a completion that never
>arrives.
>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -5722,9 +5722,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
>> if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
>> return;
>
>[Severity: High]
>This isn't a bug introduced by this patch, but does this early return leak
>the bio and cause an uninterruptible hang if REQ_NOWAIT is set?
>
>If a caller submits a discard bio with REQ_NOWAIT, the driver encounters this
>warning and returns immediately without calling bio_endio(). Because the
>caller assumes the bio was successfully handled, could this lead to a
>resource leak and task hang?
Agreed — the early return without bio_endio() is a pre-existing issue in the same function. This patch intentionally focuses on the reshape case that we reproduced, where blkdiscard hangs in bio_await() even after reshape has finished. I will fix the REQ_NOWAIT path in v2 using bio_wouldblock_error(), consistent with raid5_make_request().
>
>>
>> - if (mddev->reshape_position != MaxSector)
>> + if (mddev->reshape_position != MaxSector) {
>> /* Skip discard while reshape is happening */
>> + bio_endio_status(bi, BLK_STS_AGAIN);
>
>[Severity: High]
>Does completing a blocking bio with BLK_STS_AGAIN violate block layer
>semantics?
>
>Since the REQ_NOWAIT bios are filtered out just above this check, this path
>is only reached by synchronous, blocking discard bios. Completing them with
>BLK_STS_AGAIN will propagate an unexpected -EAGAIN error to synchronous IO
>paths and userspace applications.
>
>Block APIs typically handle BLK_STS_NOTSUPP gracefully, but passing -EAGAIN
>back through synchronous paths might cause filesystem transaction aborts or
>userspace failures. Should this return BLK_STS_NOTSUPP instead?
>
I did not find a status that fits perfectly.
In my view, BLK_STS_NOTSUPP does not fit well here either: discard is supported on the array, just not while reshape is in progress. In addition, blk_ioctl_discard() treats -EOPNOTSUPP as success (err = 0), so returning BLK_STS_NOTSUPP would report success to userspace even though no discard was performed.
BLK_STS_DEV_RESOURCE (-EBUSY) may also be worth considering.
Although the details differ from raid10, I used BLK_STS_AGAIN for the same case ( discard during reshape）, consistent with raid10_handle_discard() returning -EAGAIN. The main goal remains completing the bio with a retryable error instead of hanging callers.
>> return;
>> + }
>>
>> if (!raid5_discard_limits(mddev, bi))
>> return;
>
>--
>Sashiko AI review · https://sashiko.dev/#/patchset/20260711161326.962336-1-zhanggenjian@126.com?part=1
Suggestions are welcome.
Thanks,
Genjian
^ permalink raw reply
* Re: [PATCH 0/3] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
From: Qu Wenruo @ 2026-07-13 9:19 UTC (permalink / raw)
To: linux-btrfs, David Sterba; +Cc: hch, linux-raid
In-Reply-To: <cover.1779412152.git.wqu@suse.com>
Hi David,
I know you hate this way to fix the problem, but considering it's
already 7.2-rc3, if you do not like this method I think it's time for
you to submit your alternative.
My existing patches still apply on the latest for-next and is known to
avoid the extra warning from RAID6 lib.
Thanks,
Qu
在 2026/5/22 18:53, Qu Wenruo 写道:
> [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(-)
>
^ permalink raw reply
* Subject: RFC: Read repair for md RAID1 after mirror read failures
From: G.W. Kant - Hunenet B.V. @ 2026-07-15 2:52 UTC (permalink / raw)
To: linux-raid@vger.kernel.org
Hello list,
Recently I encountered an interesting failure mode while migrating data
from an aging backup system.
The source Btrfs filesystem spanned several LVM logical volumes, each
backed by an md RAID1 array. One of these logical volumes resided on a
degraded RAID1 array, so I added a new logical volume on a new RAID1
array and started migrating the data using:
btrfs device remove
During the migration, many read requests encountered unrecoverable read
errors (UNC) on the remaining member of the degraded RAID1 array. The
migration continued, which is exactly what I had hoped for, since only a
small fraction of the media appeared to be affected.
This made me wonder whether md RAID1 has, or has ever considered, a read
repair mechanism.
Consider the following situation:
mirror A:
read -> UNC
mirror B:
read -> OK
During a RAID1 read request, if md can satisfy the read from the
alternate mirror, no data is lost. However, this also represents an
opportunity to repair the degraded copy. Once a read has failed on one
mirror, the array has effectively lost its ability to tolerate a second
read failure for that logical block. This window persists until the
affected block is rewritten by normal filesystem activity, which may
never happen on cold archival data. Repairing the degraded copy
immediately may restore full redundancy while a valid copy is still
available. Writing the recovered block back to the the corresponding
block on the failed mirror would give the drive an opportunity to
recover that location, for example by successfully rewriting the sector
or remapping it if the write cannot be verified.
In other words, the first successfully recovered read request could
automatically become a repair opportunity. The repair could even be
scheduled asynchronously, so the successful read is returned immediately
while the rewrite is performed in the background. Unlike a periodic
resync, this repair would be driven by an actual read failure, making it
targeted rather than rewriting the entire mirror.
With today's 18–24 TB HDDs and backup/archive workloads, where data may
remain unchanged for years, latent media degradation seems increasingly
relevant. A successful read from the alternate mirror may be one of the
last opportunities to refresh such a sector before it becomes
permanently unreadable.
One advantage of such an approach is that it does not require md to
decide between two conflicting copies. One mirror has already reported
an unrecoverable read error, while the other has successfully
reconstructed the requested block. The proposal only applies to this
specific case.
Has this idea been discussed before, or is there a reason why md
deliberately avoids this type of read repair?
Regards,
Dion Kant
^ permalink raw reply
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