* Re: [PATCH v2 18/20] md/raid5: reject llbitmap reshape when md chunk shrinks
From: sashiko-bot @ 2026-06-24 7:24 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
In-Reply-To: <9b06d3a655f709b35ab10995a872b30d2026f7c3.1782282042.git.yukuai@kernel.org>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The validation in `check_reshape` fails to reject reshapes that decrease the number of data disks (`mddev->delta_disks < 0`) for llbitmap, leaving an identical bitmap coverage truncation vulnerability to the one the patch attempts to fix.
Pre-existing issues:
- [High] Writing 0 or a very small/overflowing value to the `chunk_size` sysfs attribute bypasses chunk size validation, leading to a divide-by-zero kernel panic during reshape.
- [High] Pre-existing memory leak of `stripe_head` pages in `resize_stripes()` if array reshape initialization fails.
--
commit cd96767f06f730181f5e43d1d52815ff0535ad82
Author: Yu Kuai <yukuai@fygo.io>
md/raid5: reject llbitmap reshape when md chunk shrinks
llbitmap reshape keeps one live bitmap and cannot safely make an
existing bitmap bit cover a smaller data range.
The llbitmap chunksize itself will not shrink when mddev->chunk_sectors
stays the same or grows. However, shrinking mddev->chunk_sectors shrinks
sectors_per_chunk used by raid5_bitmap_sector_map(). That can shrink the
effective data range covered by each bit across the old and new RAID5
geometry. Reject that reshape while llbitmap is active.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 1613a42cc25dc..9de648f75dd04 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -8506,6 +8506,9 @@ static int check_reshape(struct mddev *mddev)
> if (!check_stripe_cache(mddev))
> return -ENOSPC;
>
> + if (mddev->bitmap_id == ID_LLBITMAP &&
> + mddev->new_chunk_sectors < mddev->chunk_sectors)
> + return -EOPNOTSUPP;
[Severity: High]
Does this validation fully address the logical coverage truncation?
The commit message correctly notes that shrinking mddev->chunk_sectors shrinks
sectors_per_chunk, which is unsafe for llbitmap because it shrinks the
effective data range covered by each bit.
However, sectors_per_chunk is also reduced if mddev->delta_disks < 0:
sectors_per_chunk = chunk_sectors * (raid_disks - max_degraded);
Since the validation here only checks if the new chunk sectors are smaller,
a reshape that reduces the number of data disks could bypass this check,
resulting in the exact same logical coverage truncation vulnerability.
Could this be updated to also check mddev->delta_disks < 0, or compute the
product to verify sectors_per_chunk does not shrink?
> if (mddev->new_chunk_sectors > mddev->chunk_sectors ||
> mddev->delta_disks > 0)
[Severity: High]
This is a pre-existing issue, but writing 0 or a very small value to the
chunk_size sysfs attribute appears to bypass chunk size validation entirely,
leading to a divide-by-zero panic during reshape.
In drivers/md/md.c:chunk_size_store(), user input is right-shifted by 9
to compute mddev->new_chunk_sectors. If the user writes 0, it becomes 0.
When raid5_check_reshape() is called:
drivers/md/raid5.c:raid5_check_reshape() {
...
if (new_chunk > 0) {
if (!is_power_of_2(new_chunk))
return -EINVAL;
...
}
Because the checks are inside if (new_chunk > 0), a value of 0 skips the
sanity bounds. check_reshape() then accepts it.
Later, in raid5_compute_sector(), this chunk size causes a kernel panic:
drivers/md/raid5.c:raid5_compute_sector() {
...
chunk_offset = sector_div(r_sector, sectors_per_chunk);
...
}
Should chunk_size_store() or raid5_check_reshape() explicitly prevent a
chunk size of 0?
> if (resize_chunks(conf,
> conf->previous_raid_disks
> + max(0, mddev->delta_disks),
> max(mddev->new_chunk_sectors,
> mddev->chunk_sectors)
> ) < 0)
> return -ENOMEM;
>
> if (conf->previous_raid_disks + mddev->delta_disks <= conf->pool_size)
> return 0; /* never bother to shrink */
> return resize_stripes(conf, (conf->previous_raid_disks
> + mddev->delta_disks));
> }
[Severity: High]
This isn't a bug introduced by this patch, but is it possible for
resize_stripes() to leak the stripe_head pages if the array reshape
initialization fails?
If the array reshape fails under memory pressure during step 3 of
resize_stripes() (for example, the ndisks allocation fails), the function
structurally proceeds to step 4 without aborting:
drivers/md/raid5.c:resize_stripes() {
...
for (i=conf->raid_disks; i < newsize; i++)
if (nsh->dev[i].page == NULL) {
struct page *p = alloc_page(GFP_NOIO);
nsh->dev[i].page = p;
...
}
It loops up to newsize and allocates physical pages for the expanded stripe
slots. At the end of the function, because err is set, it skips updating
conf->pool_size:
drivers/md/raid5.c:resize_stripes() {
...
if (!err)
conf->pool_size = newsize;
...
}
Since conf->pool_size is unchanged, any future teardown via shrink_buffers()
will intentionally ignore the newly allocated pages at indices >= pool_size.
When free_stripe() subsequently destroys the slab object without freeing these
pages, they appear to be permanently leaked.
Is there a mechanism that cleans up these orphaned pages if conf->pool_size
is not updated?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782282042.git.yukuai@kernel.org?part=18
^ permalink raw reply
* Re: [PATCH v2 20/20] md/raid5: split reshape bios before bitmap accounting
From: sashiko-bot @ 2026-06-24 7:29 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
In-Reply-To: <f4bcf3a6cdb16689c42f2f5b5bb6b5dca0c84768.1782282042.git.yukuai@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Splitting a bio in `raid5_make_request()` breaks the `STRIPE_WAIT_RESHAPE` retry logic, causing a double-submission of the remainder and a permanent leak of the split front.
Pre-existing issues:
- [High] `make_discard_request()` leaks the `REQ_OP_DISCARD` bio without completion if an array is reshaping or `REQ_NOWAIT` is set.
--
commit fd0cb884709b54fc9c1fe1f70aa2016108b2a998
Author: Yu Kuai <yukuai@fygo.io>
md/raid5: split reshape bios before bitmap accounting
The commit uses mddev_bio_split_at_reshape_offset() in
raid5_make_request() to split bios at the reshape offset. This ensures
that RAID5 submits only one-side bios to llbitmap during reshape.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ac7ea483502fd..fb346a3c4aa3b 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6181,6 +6181,14 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
[ ... ]
> return true;
> }
>
[Severity: High]
This is a pre-existing issue, but is it possible for the discard bio to be
leaked if the array is reshaping or REQ_NOWAIT is set?
In make_discard_request() which is called just above this context, it returns
early without completing the bio:
if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
return;
if (mddev->reshape_position != MaxSector)
return;
Since raid5_make_request() calls make_discard_request() and then unconditionally
returns true, does this strand the bio and cause indefinite hangs?
> + bi = mddev_bio_split_at_reshape_offset(mddev, bi, NULL,
> + &conf->bio_split);
[Severity: Critical]
Does splitting the bio here break the STRIPE_WAIT_RESHAPE retry logic and cause
a double-submission and memory leak?
When mddev_bio_split_at_reshape_offset() splits the bio, it submits the
remainder via bio_submit_split_bioset() and returns the front half as the
new bi.
Later in raid5_make_request(), if make_stripe_request() hits the reshape window
and returns STRIPE_WAIT_RESHAPE, it handles the retry:
if (res == STRIPE_WAIT_RESHAPE) {
DECLARE_COMPLETION_ONSTACK(done);
WRITE_ONCE(bi->bi_private, &done);
bio_endio(bi);
wait_for_completion(&done);
return false;
}
When false is returned, md_handle_request() loops and retries the original
bio pointer.
However, the original bio was already submitted as the remainder by the split
operation. Will this result in concurrently submitting the remainder twice?
Additionally, when completing the cloned front half (bi) with the completion
flag, md_end_clone_io() does not complete the original split bio, which
permanently leaks the front half.
> + if (!bi) {
> + if (rw == WRITE)
> + md_write_end(mddev);
> + return true;
> + }
> +
> logical_sector = bi->bi_iter.bi_sector & ~((sector_t)RAID5_STRIPE_SECTORS(conf)-1);
> bi->bi_next = NULL;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782282042.git.yukuai@kernel.org?part=20
^ permalink raw reply
* [PATCH v3] md/raid5: fix reshape deadlock while failed devices more than max degraded
From: Chen Cheng @ 2026-06-24 7:58 UTC (permalink / raw)
To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel
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(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 65ae7d8930fc..a320b71d7117 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3728,10 +3728,82 @@ handle_failed_sync(struct r5conf *conf, struct stripe_head *sh,
if (abort)
md_sync_error(conf->mddev);
}
+/*
+ * handle_failed_reshape - handl failed stripes when reshape failed and
+ * degraded devices >= max_degraded
+ *
+ * handle following kinds of stripe:
+ * 1. cleanup the following kinds of destination stripe:
+ * - 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. dest 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. src 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())
+ */
+static void handle_failed_reshape(struct r5conf *conf, struct stripe_head *sh,
+ struct stripe_head_state *s)
+{
+ int i;
+ bool was_expanding = test_and_clear_bit(STRIPE_EXPANDING, &sh->state);
+ bool was_ready = test_and_clear_bit(STRIPE_EXPAND_READY, &sh->state);
+
+ if (was_expanding || was_ready) {
+ atomic_dec(&conf->reshape_stripes);
+ wake_up(&conf->wait_for_reshape);
+ md_done_sync(conf->mddev, RAID5_STRIPE_SECTORS(conf));
+ }
+
+ s->expanded = 0;
+ s->expanding = 0;
+
+ /* release the destination stripes that are waiting to be filled */
+ if (test_and_clear_bit(STRIPE_EXPAND_SOURCE, &sh->state)) {
+ for (i = 0; i < sh->disks; i++) {
+ int dd_idx;
+ struct stripe_head *sh2;
+ sector_t bn, sec;
+
+ if (i == sh->pd_idx)
+ continue;
+ if (conf->level == 6 && i == sh->qd_idx)
+ continue;
+
+ bn = raid5_compute_blocknr(sh, i, 1);
+ sec = raid5_compute_sector(conf, bn, 0, &dd_idx, NULL);
+ sh2 = raid5_get_active_stripe(conf, NULL, sec,
+ R5_GAS_NOBLOCK | R5_GAS_NOQUIESCE);
+ if (!sh2)
+ continue;
+
+ if (test_and_clear_bit(STRIPE_EXPANDING, &sh2->state)) {
+ atomic_dec(&conf->reshape_stripes);
+ wake_up(&conf->wait_for_reshape);
+ md_done_sync(conf->mddev,
+ RAID5_STRIPE_SECTORS(conf));
+ }
+
+ clear_bit(STRIPE_EXPAND_READY, &sh2->state);
+
+ raid5_release_stripe(sh2);
+ }
+ }
+}
+
static int want_replace(struct stripe_head *sh, int disk_idx)
{
struct md_rdev *rdev;
int rv = 0;
@@ -5001,10 +5073,12 @@ static void handle_stripe(struct stripe_head *sh)
break_stripe_batch_list(sh, 0);
if (s.to_read+s.to_write+s.written)
handle_failed_stripe(conf, sh, &s, disks);
if (s.syncing + s.replacing)
handle_failed_sync(conf, sh, &s);
+ if (s.expanding + s.expanded)
+ handle_failed_reshape(conf, sh, &s);
}
/* Now we check to see if any write operations have recently
* completed
*/
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2] md/raid5-ppl: fix use-after-free in ppl_do_flush()
From: Dan Carpenter @ 2026-06-24 8:07 UTC (permalink / raw)
To: Brigham Campbell
Cc: Sajal Gupta, linux-raid, song, yukuai3, tomasz.majchrzak,
linux-kernel, skhan, linux-kernel-mentees
In-Reply-To: <DJH0687PNY2P.1WOW0ATJ2KZA9@brighamcampbell.com>
On Tue, Jun 23, 2026 at 10:42:47PM -0600, Brigham Campbell wrote:
> On Mon Jun 22, 2026 at 8:06 AM MDT, Sajal Gupta wrote:
> > Compile tested only.
>
> It looks like you're on the right track, but this could use some
> testing. My analysis here may be incorrect, but it looks like it should
> be pretty easy to test this patch by compiling and running on a system
> with a RAID5 array, PPL enabled, and no RAID journal. I expect the call
> stack would look something like the following (feel free to correct me,
> anyone...):
Heh... That doesn't sound easy at all. (0_0)
I just left this one because it's not really a big deal. It probably
isn't even a real bug. We call increment the refcount in a loop and
then decrement it in another loop. It's not the right way. Sajal's
first approach is the right direction this should go but *that* patch
would require testing.
Adding a break here doesn't require testing because it can't possibly
break anything which is not already broken.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] md/raid1: honor REQ_NOWAIT when waiting for behind writes
From: Abd-Alrhman Masalkhi @ 2026-06-24 8:08 UTC (permalink / raw)
To: yu kuai, song, magiclinan, xiao, vverma, axboe
Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <868207d0-d20f-42e8-8257-f57816987bf5@fygo.io>
On Wed, Jun 24, 2026 at 14:39 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/6/22 2:08, Abd-Alrhman Masalkhi 写道:
>> What if on a partial nowait failure we just end the master bio as
>> success, but set NEEDED on that chunk's bitmap counter and start_sync()
>> picks it up for resync? That way we don't have to decide why rdev2
>> failed at all, resync just copies from rdev1 to rdev2 without nowait,
>> so if it's a real bad block, end_sync_write() records it then.
>>
>> We kinda have the idea of ending the bio before the write lands on all
>> mirrors in write-behind already, though it's not quite the same, there
>> the deferred write still lands, here we ACK after it already failed and
>> lean on resync to redo it.
>>
>> My worry is the loaded case: AGAIN under queue pressure isn't that rare,
>> so we might end up triggering resync frequently. Do you think that's
>> acceptable, or is dropping nowait better? Happy to prototype either way.
>
> Yes, AGAIN under queue pressure isn't rare, and I don't think trigger resync
> for a nowait IO failure is acceptable. This can cause lots of offline IO
> pressure which will affect disk service life. Meanwhile, resync will cause
> performance degradation for user.
>
> I still feel dropping nowait is better.
>
That makes sense. I'll drop nowait for md and post a patch.
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH 4/7] md/raid10: raid10_write_request() drops the barrier before calling
From: Abd-Alrhman Masalkhi @ 2026-06-24 8:12 UTC (permalink / raw)
To: yu kuai, song, magiclinan, xiao, axboe, john.g.garry,
martin.petersen, yukuai
Cc: linux-raid, linux-kernel
In-Reply-To: <7239c50c-b257-4179-827a-9facd48e75e1@fygo.io>
Hi Kaui,
On Wed, Jun 24, 2026 at 15:23 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/6/23 15:24, Abd-Alrhman Masalkhi 写道:
>> bio_submit_split_bioset() and reacquires it afterwards. This is
>> unnecessary because bio_submit_split_bioset() does not require
>> releasing the barrier protection.
>
> Why is this safe?
>
> It's possible plug is not enabled in this context, and the allocated new
> aplit bio will be submitted and enter wait_barrier() again. wait_barrier()
> is not supposed to be called recursively. Otherwise deadlock can happend:
>
> t1: sync_thread grab barrier and waiting for nr_pending to be 0 from raise_barrier()
> t2: wait_barrier() first inc nr_pending, then recursive wait_barrier() waiting
> for barrier to be released.
>
raid10_write_request() is only called from the bio submission path and
not from raid10d thread, so we do not re-enter it while already holding
barrier. Therefore, the split bio submitted by bio_submit_split_bioset()
cannot recurse back into wait_barrier() from the same execution path.
The situation is different for reads, where raid10_read_request() will
be called both from the submission path and from raid10d. There,
dropping and reacquiring the barrier protection remains necessary.
>>
>> Remove the redundant allow_barrier()/wait_barrier() pair around
>> bio_submit_split_bioset().
>>
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> drivers/md/raid10.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 840f0446c231..4bc1d5553ec7 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -1493,10 +1493,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
>> if (atomic)
>> goto err_handle;
>>
>> - allow_barrier(conf);
>> bio = bio_submit_split_bioset(bio, r10_bio->sectors,
>> &conf->bio_split);
>> - wait_barrier(conf, false);
>> if (!bio) {
>> set_bit(R10BIO_Returned, &r10_bio->state);
>> goto err_handle;
>
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v2 08/20] md/md-llbitmap: finish reshape geometry
From: sashiko-bot @ 2026-06-24 9:06 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
In-Reply-To: <73a8a0409b741c57bbc9927abc7a9981cbafbf27.1782282042.git.yukuai@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The entire bitmap is unnecessarily flushed to disk twice during reshape finish.
--
commit ae4a6d71e5ee77405793f20d5d615e089187608f
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: finish reshape geometry
This commit introduces the final step for reshaping llbitmap arrays by
committing the staged geometry. It handles quiescing the device, updating
internal state, writing the superblock, and flushing the bitmap.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index cdced2126e975..ae2e87fd58874 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
> @@ -1680,6 +1680,30 @@ static void llbitmap_dirty_bits(struct mddev *mddev, unsigned long s,
> llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
> }
>
> +static void llbitmap_reshape_finish(struct mddev *mddev)
> +{
> + struct llbitmap *llbitmap = mddev->bitmap;
> +
> + if (mddev->pers->quiesce)
> + mddev->pers->quiesce(mddev, 1);
> +
> + mutex_lock(&mddev->bitmap_info.mutex);
> + llbitmap_flush(mddev);
> +
> + llbitmap->chunksize = llbitmap->reshape_chunksize;
> + llbitmap->chunkshift = ffz(~llbitmap->chunksize);
> + llbitmap->chunks = llbitmap->reshape_chunks;
> + llbitmap->sync_size = llbitmap->reshape_sync_size;
> + llbitmap_refresh_reshape(llbitmap);
> + mddev->bitmap_info.chunksize = llbitmap->chunksize;
> + llbitmap_update_sb(llbitmap);
> + __llbitmap_flush(mddev);
[Severity: Low]
Is this second flush of the entire bitmap redundant?
The code calls llbitmap_flush() at the beginning of the lock section.
After updating the geometry and writing out the superblock via
llbitmap_update_sb(), __llbitmap_flush() is called again. Since no bitmap
data bits are modified in the interim, does this cause an unnecessary
synchronous I/O spike that could be avoided?
> + mutex_unlock(&mddev->bitmap_info.mutex);
> +
> + if (mddev->pers->quiesce)
> + mddev->pers->quiesce(mddev, 0);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782282042.git.yukuai@kernel.org?part=8
^ permalink raw reply
* [RFC PATCH] md,dm: caller-owned memalloc_noio token in mddev_suspend()/resume()
From: Chen Cheng @ 2026-06-24 10:13 UTC (permalink / raw)
To: linux-raid, yukuai, dm-devel; +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:
- 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);
}
}
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)
--
2.54.0
^ permalink raw reply related
* Re: [GIT PULL] md-7.2-20260623
From: Jens Axboe @ 2026-06-24 12:33 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, linux-block, Abd-Alrhman Masalkhi, Chen Cheng
In-Reply-To: <20260623090327.1097428-1-yukuai@kernel.org>
On 6/23/26 3:03 AM, Yu Kuai wrote:
> Hi Jens,
>
> Please consider pulling the following changes into your block-7.2
> branch.
>
> This pull request contains:
>
> Bug Fixes:
> - Fix raid1 writes_pending and barrier reference leaks on write failures.
> (Abd-Alrhman Masalkhi)
> - Fix raid10 writes_pending leak on write request failures.
> (Abd-Alrhman Masalkhi)
> - Fix raid10 writes_pending and barrier reference leaks on discard failures.
> (Abd-Alrhman Masalkhi)
> - Fix raid1 REQ_NOWAIT handling while waiting for behind writes.
> (Abd-Alrhman Masalkhi)
> - Fix raid1 r1_bio leak when a REQ_NOWAIT retry would block.
> (Abd-Alrhman Masalkhi)
> - Fix raid1 read-balance head_position data race. (Chen Cheng)
> - Fix raid5 stripe batch bm_seq wraparound comparison. (Chen Cheng)
> - Fix raid5 stripe batch state snapshot KCSAN noise. (Chen Cheng)
> - Fix raid5 R5_Overlap races while breaking stripe batches. (Chen Cheng)
>
> Improvements:
> - Add raid5 discard IO accounting. (Yu Kuai)
> - Always convert raid5 llbitmap bits for discard. (Yu Kuai)
>
> Cleanups:
> - Simplify raid1_write_request() error handling. (Abd-Alrhman Masalkhi)
Pulled, thanks.
--
Jens Axboe
^ permalink raw reply
* [PATCH 0/5] md: minor cleanups in md core and raid5/raid1/raid10
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
A handful of small, no-functional-change cleanups noticed while reading
the md code:
- drop an unused parameter from check_decay_read_errors();
- use max() in raid5_calc_degraded();
- make is_mddev_idle() take a bool init flag, matching the helper it
forwards to;
- declare a sector count as sector_t rather than int in status_resync();
- rewrite a self-contradictory comment above the resync ETA math.
No behavioral change in any patch. Built and functionally sanity-tested
(RAID5/RAID6 create, rebuild, scrub) on x86_64, including a run with KASAN
and lockdep enabled -- with concurrent /proc/mdstat polling to exercise
status_resync() -- and no reports.
Hiroshi Nishida (5):
md/raid1,raid10: drop unused mddev arg from check_decay_read_errors()
md/raid5: use max() in raid5_calc_degraded()
md: make is_mddev_idle() take a bool init flag
md: use sector_t for recovery_active in status_resync()
md: clarify the resync ETA comment in status_resync()
drivers/md/md.c | 26 ++++++++++----------------
drivers/md/raid1-10.c | 4 ++--
drivers/md/raid5.c | 4 +---
3 files changed, 13 insertions(+), 21 deletions(-)
base-commit: 55b77337bdd088c77461588e5ec094421b89911b
--
2.43.0
^ permalink raw reply
* [PATCH 1/5] md/raid1,raid10: drop unused mddev arg from check_decay_read_errors()
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155421.211626-1-nishidafmly@gmail.com>
check_decay_read_errors() only ever touches the passed rdev; the mddev
parameter has been unused. Remove it and update the sole caller in
exceed_read_errors().
No functional change.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid1-10.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index 56a56a4da4f8..edba52eb3116 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -171,7 +171,7 @@ static inline void raid1_prepare_flush_writes(struct mddev *mddev)
* We halve the read error count for every hour that has elapsed
* since the last recorded read error.
*/
-static inline void check_decay_read_errors(struct mddev *mddev, struct md_rdev *rdev)
+static inline void check_decay_read_errors(struct md_rdev *rdev)
{
long cur_time_mon;
unsigned long hours_since_last;
@@ -206,7 +206,7 @@ static inline bool exceed_read_errors(struct mddev *mddev, struct md_rdev *rdev)
int max_read_errors = atomic_read(&mddev->max_corr_read_errors);
int read_errors;
- check_decay_read_errors(mddev, rdev);
+ check_decay_read_errors(rdev);
read_errors = atomic_inc_return(&rdev->read_errors);
if (read_errors > max_read_errors) {
pr_notice("md/"RAID_1_10_NAME":%s: %pg: Raid device exceeded read_error threshold [cur %d:max %d]\n",
--
2.43.0
^ permalink raw reply related
* [PATCH 2/5] md/raid5: use max() in raid5_calc_degraded()
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155421.211626-1-nishidafmly@gmail.com>
The tail of raid5_calc_degraded() open-codes a max of the two computed
degraded counts. Use max() for clarity.
No functional change.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0c5c9fb0606e..2b2aa4f2431c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -741,9 +741,7 @@ int raid5_calc_degraded(struct r5conf *conf)
if (conf->raid_disks <= conf->previous_raid_disks)
degraded2++;
}
- if (degraded2 > degraded)
- return degraded2;
- return degraded;
+ return max(degraded, degraded2);
}
static bool has_failed(struct r5conf *conf)
--
2.43.0
^ permalink raw reply related
* [PATCH 3/5] md: make is_mddev_idle() take a bool init flag
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155421.211626-1-nishidafmly@gmail.com>
is_mddev_idle() took an int init flag but passes it straight through to
is_rdev_holder_idle(), which already takes a bool. Make the types
consistent and update the two callers to pass true/false.
No functional change.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/md.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..829b4a3f545b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9219,7 +9219,7 @@ static bool is_rdev_holder_idle(struct md_rdev *rdev, bool init)
*
* Noted this checking rely on IO accounting is enabled.
*/
-static bool is_mddev_idle(struct mddev *mddev, int init)
+static bool is_mddev_idle(struct mddev *mddev, bool init)
{
unsigned long last_events = mddev->normal_io_events;
struct gendisk *disk;
@@ -9771,7 +9771,7 @@ void md_do_sync(struct md_thread *thread)
pr_debug("md: using maximum available idle IO bandwidth (but not more than %d KB/sec) for %s.\n",
speed_max(mddev), desc);
- is_mddev_idle(mddev, 1); /* this initializes IO event counters */
+ is_mddev_idle(mddev, true); /* this initializes IO event counters */
io_sectors = 0;
for (m = 0; m < SYNC_MARKS; m++) {
@@ -9920,7 +9920,7 @@ void md_do_sync(struct md_thread *thread)
goto repeat;
}
if (!sync_io_within_limit(mddev) &&
- !is_mddev_idle(mddev, 0)) {
+ !is_mddev_idle(mddev, false)) {
/*
* Give other IO more of a chance.
* The faster the devices, the less we wait.
--
2.43.0
^ permalink raw reply related
* [PATCH 4/5] md: use sector_t for recovery_active in status_resync()
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155421.211626-1-nishidafmly@gmail.com>
recovery_active holds a sector count read from the sector-typed atomic
mddev->recovery_active and is then combined with the sector_t values
curr_mark_cnt and resync_mark_cnt. Declaring it as a plain int needlessly
narrows it and mixes signedness into sector_t arithmetic; declare it
sector_t to match.
No functional change.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/md.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 829b4a3f545b..0cd85cc92ed3 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8816,8 +8816,8 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
{
sector_t max_sectors, resync, res;
unsigned long dt, db = 0;
- sector_t rt, curr_mark_cnt, resync_mark_cnt;
- int scale, recovery_active;
+ sector_t rt, curr_mark_cnt, resync_mark_cnt, recovery_active;
+ int scale;
unsigned int per_milli;
if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery) ||
--
2.43.0
^ permalink raw reply related
* [PATCH 5/5] md: clarify the resync ETA comment in status_resync()
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155421.211626-1-nishidafmly@gmail.com>
The comment above the remaining-time computation was self-contradictory:
it said the original algorithm was being kept but "is not really
necessary", then described that algorithm under an "Original algorithm:"
heading as if it had been replaced. The code still uses it. Rewrite the
comment to simply describe what the code does.
Comment only; no functional change.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/md.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0cd85cc92ed3..a5c0da0d1133 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8916,17 +8916,11 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
* db: blocks written from mark until now
* rt: remaining time
*
- * rt is a sector_t, which is always 64bit now. We are keeping
- * the original algorithm, but it is not really necessary.
- *
- * Original algorithm:
- * So we divide before multiply in case it is 32bit and close
- * to the limit.
- * We scale the divisor (db) by 32 to avoid losing precision
- * near the end of resync when the number of remaining sectors
- * is close to 'db'.
- * We then divide rt by 32 after multiplying by db to compensate.
- * The '+1' avoids division by zero if db is very small.
+ * rt is computed as (remaining sectors) * dt / db. To keep precision
+ * near the end of resync, when the remaining count is close to db, the
+ * divisor db is scaled up by 32 before the divide and rt is scaled back
+ * down by 32 afterwards. The '+1' avoids division by zero when db is
+ * very small.
*/
dt = ((jiffies - mddev->resync_mark) / HZ);
if (!dt) dt++;
--
2.43.0
^ permalink raw reply related
* [PATCH 0/8] md/raid5: scalability and rebuild-path improvements
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
This series collects small, individually low-risk md/raid5 changes for
large, many-core, many-disk arrays. Their common theme is reducing
per-stripe and stripe-cache contention, so the benefit appears mainly
when the raid5 stripe-handling worker threads are in use
(group_thread_cnt > 0); at the default group_thread_cnt = 0 (a single
handling thread) the series is essentially neutral.
- patches 1-3 remove signed arithmetic from a hot-path divisor, lift an
arbitrary stripe-cache size cap, and widen a badblock length argument
that currently truncates large ranges;
- patch 4 raises NR_STRIPE_HASH_LOCKS (8 -> 32) to spread stripe-hash
contention on high core-count systems;
- patches 5 and 8 reduce per-stripe overhead in the resync/recovery
path and bound the share of the stripe cache a rebuild may hold while
user I/O is competing;
- patch 6 allocates each worker group's array on its own NUMA node;
- patch 7 raises MAX_STRIPE_BATCH (8 -> 32).
Measured effect, treatment vs baseline, % change in mean IOPS (N=3),
swept over group_thread_cnt (RAID6 4+2, 22-core host, ramdisk members):
workload gtc=0 gtc=2 gtc=4 gtc=8
random 4K write (RMW) +4.2% +8.1% +17.4% +6.5%
DB mixed 75/25 8K +0.4% +4.2% +10.3% +4.7%
high-concurrency 70/30 4K +3.9% +1.2% +10.0% +0.2%
OLTP 70/30 16K -0.3% +4.7% +10.1% +9.3%
partial-stripe write 8K +1.1% +4.8% +11.2% +14.2%
At the default single handling thread (group_thread_cnt = 0) the series is
neutral (no regression). As worker threads are added the gain grows,
peaking broadly around group_thread_cnt = 4 at roughly +10-17% across the
whole mix; at gtc = 8 the write-heavy workloads keep gaining while the
read-heavy high-concurrency case has saturated. (Per-run cv was <1%
except the random-write test, ~5-9%, from a cold first run.)
These numbers are on a ramdisk, which removes device latency and so
overstates the CPU-side contention effect relative to a real device;
they show the direction and the group_thread_cnt dependence, not an
absolute speedup. The stripe-hash/batch patches (4, 7) and the cache cap
(2) drive this; patch 6 only matters on multi-socket systems (not
exercised above) and patches 5/8 act on the resync/recovery path rather
than this steady-state workload.
Reproduction (stock mdadm + fio):
mdadm --create /dev/md0 --level=6 --raid-devices=6 --chunk=512 \
--assume-clean <6 members>
echo 16384 > /sys/block/md0/md/stripe_cache_size
echo N > /sys/block/md0/md/group_thread_cnt # N = 0,2,4,8
fio --filename=/dev/md0 --direct=1 --ioengine=libaio --group_reporting \
--time_based --runtime=15 --name=w <per-workload opts>:
random write : --rw=randwrite --bs=4k --numjobs=4 --iodepth=32
DB mixed : --rw=randrw --rwmixread=75 --bs=8k --numjobs=8 --iodepth=16
high-concur. : --rw=randrw --rwmixread=70 --bs=4k --numjobs=16 --iodepth=8
OLTP : --rw=randrw --rwmixread=70 --bs=16k --numjobs=6 --iodepth=16
partial-stripe : --rw=randwrite --bs=8k --numjobs=4 --iodepth=32
Each patch stands on its own; I am happy to drop or defer any that is not
justified on its own merit.
Functional testing on RAID5 and RAID6: create, fail a member, rebuild
onto a spare / re-add, full data read-back verified, and scrub
("check") reporting mismatch_cnt == 0. The series was also exercised
with KASAN and lockdep enabled -- including heavy group_thread_cnt
churn on a multi-node setup to stress the per-NUMA-node worker
allocation and the raid5_quiesce hash-lock-all path -- with no reports.
Hiroshi Nishida (8):
md: change chunk_sectors and stripe cache counts to unsigned int
md/raid5: raise stripe cache limit from 32768 to 262144
md: widen badblock sectors param from int to sector_t
md/raid5: raise NR_STRIPE_HASH_LOCKS from 8 to 32
md/raid5: submit a window of stripes during resync/recovery
md/raid5: allocate worker groups per NUMA node
md/raid5: raise MAX_STRIPE_BATCH from 8 to 32
md/raid5: reserve stripe cache for user I/O during rebuild
drivers/md/md.c | 4 +-
drivers/md/md.h | 10 ++--
drivers/md/raid5.c | 129 ++++++++++++++++++++++++++++++++-------------
drivers/md/raid5.h | 33 ++++++++----
4 files changed, 121 insertions(+), 55 deletions(-)
base-commit: 55b77337bdd088c77461588e5ec094421b89911b
--
2.43.0
^ permalink raw reply
* [PATCH 1/8] md: change chunk_sectors and stripe cache counts to unsigned int
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
chunk_sectors, new_chunk_sectors, prev_chunk_sectors, max_nr_stripes,
and min_nr_stripes are never negative. Using signed int is semantically
wrong and prevents the compiler from optimizing division/modulo by
power-of-two chunk sizes to right shifts in the hot I/O path.
Change all struct fields and derived local variables to unsigned int:
mddev->chunk_sectors
mddev->new_chunk_sectors
r5conf->chunk_sectors
r5conf->prev_chunk_sectors
r5conf->max_nr_stripes
r5conf->min_nr_stripes
Local: sectors_per_chunk, new_chunk, chunk_sectors
The min() in r5c_check_cached_full_stripe() required both operands to
match signedness; this is now satisfied with max_nr_stripes unsigned.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/md.h | 4 ++--
drivers/md/raid5.c | 14 +++++++-------
drivers/md/raid5.h | 8 ++++----
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d8daf0f75cbb..b9ad26844799 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -437,7 +437,7 @@ struct mddev {
int external; /* metadata is
* managed externally */
char metadata_type[17]; /* externally set*/
- int chunk_sectors;
+ unsigned int chunk_sectors;
time64_t ctime, utime;
int level, layout;
char clevel[16];
@@ -466,7 +466,7 @@ struct mddev {
*/
sector_t reshape_position;
int delta_disks, new_level, new_layout;
- int new_chunk_sectors;
+ unsigned int new_chunk_sectors;
int reshape_backwards;
struct md_thread __rcu *thread; /* management thread */
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0c5c9fb0606e..28828e083c2b 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2970,7 +2970,7 @@ sector_t raid5_compute_sector(struct r5conf *conf, sector_t r_sector,
sector_t new_sector;
int algorithm = previous ? conf->prev_algo
: conf->algorithm;
- int sectors_per_chunk = previous ? conf->prev_chunk_sectors
+ unsigned int sectors_per_chunk = previous ? conf->prev_chunk_sectors
: conf->chunk_sectors;
int raid_disks = previous ? conf->previous_raid_disks
: conf->raid_disks;
@@ -3166,7 +3166,7 @@ sector_t raid5_compute_blocknr(struct stripe_head *sh, int i, int previous)
int raid_disks = sh->disks;
int data_disks = raid_disks - conf->max_degraded;
sector_t new_sector = sh->sector, check;
- int sectors_per_chunk = previous ? conf->prev_chunk_sectors
+ unsigned int sectors_per_chunk = previous ? conf->prev_chunk_sectors
: conf->chunk_sectors;
int algorithm = previous ? conf->prev_algo
: conf->algorithm;
@@ -3584,7 +3584,7 @@ static void end_reshape(struct r5conf *conf);
static void stripe_set_idx(sector_t stripe, struct r5conf *conf, int previous,
struct stripe_head *sh)
{
- int sectors_per_chunk =
+ unsigned int sectors_per_chunk =
previous ? conf->prev_chunk_sectors : conf->chunk_sectors;
int dd_idx;
int chunk_offset = sector_div(stripe, sectors_per_chunk);
@@ -6103,7 +6103,7 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
static sector_t raid5_bio_lowest_chunk_sector(struct r5conf *conf,
struct bio *bi)
{
- int sectors_per_chunk = conf->chunk_sectors;
+ unsigned int sectors_per_chunk = conf->chunk_sectors;
int raid_disks = conf->raid_disks;
int dd_idx;
struct stripe_head sh;
@@ -7930,7 +7930,7 @@ static int raid5_run(struct mddev *mddev)
sector_t here_new, here_old;
int old_disks;
int max_degraded = (mddev->level == 6 ? 2 : 1);
- int chunk_sectors;
+ unsigned int chunk_sectors;
int new_data_disks;
if (journal_dev) {
@@ -8832,7 +8832,7 @@ static int raid5_check_reshape(struct mddev *mddev)
* to be used by a reshape pass.
*/
struct r5conf *conf = mddev->private;
- int new_chunk = mddev->new_chunk_sectors;
+ unsigned int new_chunk = mddev->new_chunk_sectors;
if (mddev->new_layout >= 0 && !algorithm_valid_raid5(mddev->new_layout))
return -EINVAL;
@@ -8866,7 +8866,7 @@ static int raid5_check_reshape(struct mddev *mddev)
static int raid6_check_reshape(struct mddev *mddev)
{
- int new_chunk = mddev->new_chunk_sectors;
+ unsigned int new_chunk = mddev->new_chunk_sectors;
if (mddev->new_layout >= 0 && !algorithm_valid_raid6(mddev->new_layout))
return -EINVAL;
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index cb5feae04db2..5cd9d0f36b6e 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -572,12 +572,12 @@ struct r5conf {
/* only protect corresponding hash list and inactive_list */
spinlock_t hash_locks[NR_STRIPE_HASH_LOCKS];
struct mddev *mddev;
- int chunk_sectors;
+ unsigned int chunk_sectors;
int level, algorithm, rmw_level;
int max_degraded;
int raid_disks;
- int max_nr_stripes;
- int min_nr_stripes;
+ unsigned int max_nr_stripes;
+ unsigned int min_nr_stripes;
#if PAGE_SIZE != DEFAULT_STRIPE_SIZE
unsigned long stripe_size;
unsigned int stripe_shift;
@@ -595,7 +595,7 @@ struct r5conf {
*/
sector_t reshape_safe;
int previous_raid_disks;
- int prev_chunk_sectors;
+ unsigned int prev_chunk_sectors;
int prev_algo;
short generation; /* increments with every reshape */
seqcount_spinlock_t gen_lock; /* lock against generation changes */
--
2.43.0
^ permalink raw reply related
* [PATCH 2/8] md/raid5: raise stripe cache limit from 32768 to 262144
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
Stripe cache memory is approximately max_nr_stripes * num_disks *
stripe_size, where stripe_size is 4KB (PAGE_SIZE). The old hardcoded
ceiling of 32768 stripes therefore limits a 12-disk array to ~1.5GB of
stripe cache. On servers with 256GB+ RAM backing wide arrays, a larger
cache reduces stalls on stripe-cache misses under heavy write workloads.
Define RAID5_MAX_NR_STRIPES = 262144 (256K) in raid5.h and use it in
raid5_set_cache_size() instead of the magic 32768 literal. This allows
up to ~12GB of stripe cache on a 12-disk array. The default stripe count
(NR_STRIPES = 256) is unchanged, so there is no memory impact unless the
administrator raises stripe_cache_size explicitly.
Also fix two local variables in raid5_cache_count() that were declared
as int but read from unsigned int fields; change to unsigned int to
avoid implicit sign-extension in the subtraction.
Tested: echo 262144 > /sys/block/md0/md/stripe_cache_size accepted;
262145 correctly returns EINVAL.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 6 +++---
drivers/md/raid5.h | 6 ++++++
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 28828e083c2b..9cb4ed3bd85c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6923,7 +6923,7 @@ raid5_set_cache_size(struct mddev *mddev, int size)
int result = 0;
struct r5conf *conf = mddev->private;
- if (size <= 16 || size > 32768)
+ if (size <= 16 || size > RAID5_MAX_NR_STRIPES)
return -EINVAL;
WRITE_ONCE(conf->min_nr_stripes, size);
@@ -7505,8 +7505,8 @@ static unsigned long raid5_cache_count(struct shrinker *shrink,
struct shrink_control *sc)
{
struct r5conf *conf = shrink->private_data;
- int max_stripes = READ_ONCE(conf->max_nr_stripes);
- int min_stripes = READ_ONCE(conf->min_nr_stripes);
+ unsigned int max_stripes = READ_ONCE(conf->max_nr_stripes);
+ unsigned int min_stripes = READ_ONCE(conf->min_nr_stripes);
if (max_stripes < min_stripes)
/* unlikely, but not impossible */
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 5cd9d0f36b6e..57349737d393 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -479,6 +479,12 @@ struct disk_info {
*/
#define NR_STRIPES 256
+/* Maximum user-settable stripe cache size, in stripes. Stripe cache memory is
+ * roughly max_nr_stripes * num_disks * stripe_size (stripe_size = 4KB), so the
+ * old cap of 32768 limited a 12-disk array to ~1.5GB. Raise to 262144 to allow
+ * larger caches on big-RAM systems; the default (NR_STRIPES) is unchanged.
+ */
+#define RAID5_MAX_NR_STRIPES 262144U
#if PAGE_SIZE == DEFAULT_STRIPE_SIZE
#define STRIPE_SIZE PAGE_SIZE
--
2.43.0
^ permalink raw reply related
* [PATCH 3/8] md: widen badblock sectors param from int to sector_t
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
The badblocks core API -- badblocks_set(), badblocks_clear() and
badblocks_check() -- and the is_badblock() helper all take the range
length as sector_t. The md wrappers rdev_set_badblocks(),
rdev_clear_badblocks() and rdev_has_badblock(), however, declared the
same length as int, narrowing sector_t to int and back again in the
middle of an otherwise 64-bit clean path.
Change the sectors parameter to sector_t in these three wrappers so it
matches the core API and is_badblock(). No functional change: current
callers pass per-I/O or per-resync-chunk lengths well within int range.
This just removes a gratuitous truncation point and keeps the type
consistent end to end.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/md.c | 4 ++--
drivers/md/md.h | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..61f40fa41e78 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -10553,7 +10553,7 @@ EXPORT_SYMBOL(md_finish_reshape);
/* Bad block management */
/* Returns true on success, false on failure */
-bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
+bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
int is_new)
{
struct mddev *mddev = rdev->mddev;
@@ -10593,7 +10593,7 @@ bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
}
EXPORT_SYMBOL_GPL(rdev_set_badblocks);
-void rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
+void rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
int is_new)
{
if (is_new)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b9ad26844799..95835a3286aa 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -311,7 +311,7 @@ static inline int is_badblock(struct md_rdev *rdev, sector_t s, sector_t sectors
}
static inline int rdev_has_badblock(struct md_rdev *rdev, sector_t s,
- int sectors)
+ sector_t sectors)
{
sector_t first_bad;
sector_t bad_sectors;
@@ -319,9 +319,9 @@ static inline int rdev_has_badblock(struct md_rdev *rdev, sector_t s,
return is_badblock(rdev, s, sectors, &first_bad, &bad_sectors);
}
-extern bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
+extern bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
int is_new);
-extern void rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
+extern void rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
int is_new);
struct md_cluster_info;
struct md_cluster_operations;
--
2.43.0
^ permalink raw reply related
* [PATCH 4/8] md/raid5: raise NR_STRIPE_HASH_LOCKS from 8 to 32
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
The stripe cache hash is striped across NR_STRIPE_HASH_LOCKS spinlocks
(see stripe_hash_locks_hash()). The value has been 8 since the per-hash
locking was introduced, which is small for modern many-core servers:
with only 8 buckets, stripe cache lookup and allocation contend on the
same few locks once the CPU count greatly exceeds the bucket count.
Raise it to 32. Two constraints bound the choice:
- STRIPE_HASH_LOCKS_MASK is (NR_STRIPE_HASH_LOCKS - 1) and is used as
a bitmask in stripe_hash_locks_hash(), so the value must be a power
of two.
- raid5_quiesce() acquires every hash lock plus device_lock at once
via lock_all_device_hash_locks_irq(). That holds
NR_STRIPE_HASH_LOCKS + 1 locks simultaneously, which must stay below
MAX_LOCK_DEPTH (48) so the held-lock array does not overflow when
lockdep is enabled.
32 is the largest power of two that satisfies both: 32 + 1 leaves
headroom under 48, whereas 64 would exceed it. (The pre-existing
"must remain below 64" comment understates this; MAX_LOCK_DEPTH is the
real ceiling.) STRIPE_HASH_LOCKS_MASK and all NR_STRIPE_HASH_LOCKS-
sized arrays scale automatically.
This is purely a lock-striping change; it does not affect stripe cache
correctness. The benefit appears on many-core systems, while on small
systems the extra buckets are harmless.
Tested: loop-device RAID-5 create, write/verify, fail a disk, rebuild
onto a spare and scrub all complete cleanly.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 57349737d393..3efab71ebef7 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -498,12 +498,14 @@ struct disk_info {
#define HASH_MASK (NR_HASH - 1)
#define MAX_STRIPE_BATCH 8
-/* NOTE NR_STRIPE_HASH_LOCKS must remain below 64.
- * This is because we sometimes take all the spinlocks
- * and creating that much locking depth can cause
- * problems.
+/* NR_STRIPE_HASH_LOCKS must be a power of two, since
+ * STRIPE_HASH_LOCKS_MASK masks with (NR_STRIPE_HASH_LOCKS - 1).
+ * It must also be small enough that taking all of them at once in
+ * lock_all_device_hash_locks_irq(), plus device_lock, keeps the held
+ * lock count below MAX_LOCK_DEPTH (48) with lockdep enabled. 32 is the
+ * largest power of two that satisfies both constraints.
*/
-#define NR_STRIPE_HASH_LOCKS 8
+#define NR_STRIPE_HASH_LOCKS 32
#define STRIPE_HASH_LOCKS_MASK (NR_STRIPE_HASH_LOCKS - 1)
struct r5worker {
--
2.43.0
^ permalink raw reply related
* [PATCH 5/8] md/raid5: submit a window of stripes during resync/recovery
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
raid5_sync_request() dispatches one stripe per call: it fetches a single
stripe head, marks it for sync, and returns one stripe's worth of
sectors. When the stripe cache is full the NOBLOCK fetch fails and it
re-enters a one-jiffy throttle sleep (schedule_timeout_uninterruptible(1))
before retrying. Because that sleep is taken per stripe, sustained cache
pressure bounds sync progress to roughly HZ stripes/second regardless of
how fast the member devices are.
Dispatch up to RAID5_SYNC_WINDOW (32) stripes per call instead. Only the
first stripe of the window keeps the original behaviour (block, then the
one-jiffy throttle if the cache was full); the remaining stripes are
requested with R5_GAS_NOBLOCK and the loop stops as soon as the cache is
full. So at most one throttle sleep is taken per window rather than per
stripe, and when the cache has free slots a single call can queue a batch
instead of one stripe at a time.
With a warm cache the window stays near full: counting raid5_sync_request()
invocations across a rebuild showed it averaging ~30 of the 32 stripes per
call, i.e. roughly 30x fewer calls into the sync path for the same resync.
The return value reports the number of stripes actually submitted, so
md_do_sync()'s recovery_active accounting stays balanced, and the window
is bounded by both the end of the sync region (max_sector) and
mddev->resync_max, so a user- or cluster-imposed sync ceiling is not
overshot.
This does not change which data is read or written during resync or
recovery.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 47 +++++++++++++++++++++++++++++++++-------------
drivers/md/raid5.h | 1 +
2 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 9cb4ed3bd85c..8e9edaaca667 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6563,7 +6563,8 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
struct stripe_head *sh;
sector_t sync_blocks;
bool still_degraded = false;
- int i;
+ int i, submitted;
+ sector_t win_sector;
if (sector_nr >= max_sector) {
/* just being told to finish up .. nothing much to do */
@@ -6620,16 +6621,7 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
if (md_bitmap_enabled(mddev, false))
mddev->bitmap_ops->cond_end_sync(mddev, sector_nr, false);
- sh = raid5_get_active_stripe(conf, NULL, sector_nr,
- R5_GAS_NOBLOCK);
- if (sh == NULL) {
- sh = raid5_get_active_stripe(conf, NULL, sector_nr, 0);
- /* make sure we don't swamp the stripe cache if someone else
- * is trying to get access
- */
- schedule_timeout_uninterruptible(1);
- }
- /* Need to check if array will still be degraded after recovery/resync
+ /* Check once whether array will still be degraded after recovery/resync.
* Note in case of > 1 drive failures it's possible we're rebuilding
* one drive while leaving another faulty drive in array.
*/
@@ -6640,13 +6632,42 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
still_degraded = true;
}
+ /* First stripe: block if stripe cache is full, then throttle. */
+ sh = raid5_get_active_stripe(conf, NULL, sector_nr, R5_GAS_NOBLOCK);
+ if (sh == NULL) {
+ sh = raid5_get_active_stripe(conf, NULL, sector_nr, 0);
+ /* make sure we don't swamp the stripe cache if someone else
+ * is trying to get access
+ */
+ schedule_timeout_uninterruptible(1);
+ }
md_bitmap_start_sync(mddev, sector_nr, &sync_blocks, still_degraded);
set_bit(STRIPE_SYNC_REQUESTED, &sh->state);
set_bit(STRIPE_HANDLE, &sh->state);
-
raid5_release_stripe(sh);
- return RAID5_STRIPE_SECTORS(conf);
+ /* Submit remaining stripes in the window non-blocking. Stop early
+ * if the stripe cache is full: the disk queue is already saturated.
+ * Bound by resync_max so a user- or cluster-imposed sync ceiling is
+ * not overshot.
+ */
+ win_sector = sector_nr + RAID5_STRIPE_SECTORS(conf);
+ for (submitted = 1;
+ submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
+ win_sector < mddev->resync_max;
+ submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
+ sh = raid5_get_active_stripe(conf, NULL, win_sector,
+ R5_GAS_NOBLOCK);
+ if (!sh)
+ break;
+ md_bitmap_start_sync(mddev, win_sector, &sync_blocks,
+ still_degraded);
+ set_bit(STRIPE_SYNC_REQUESTED, &sh->state);
+ set_bit(STRIPE_HANDLE, &sh->state);
+ raid5_release_stripe(sh);
+ }
+
+ return submitted * RAID5_STRIPE_SECTORS(conf);
}
static int retry_aligned_read(struct r5conf *conf, struct bio *raid_bio,
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 3efab71ebef7..7aeba1fc7f09 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -497,6 +497,7 @@ struct disk_info {
#define NR_HASH (PAGE_SIZE / sizeof(struct hlist_head))
#define HASH_MASK (NR_HASH - 1)
#define MAX_STRIPE_BATCH 8
+#define RAID5_SYNC_WINDOW 32 /* stripes to pre-submit per sync_request call */
/* NR_STRIPE_HASH_LOCKS must be a power of two, since
* STRIPE_HASH_LOCKS_MASK masks with (NR_STRIPE_HASH_LOCKS - 1).
--
2.43.0
^ permalink raw reply related
* [PATCH 6/8] md/raid5: allocate worker groups per NUMA node
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
alloc_thread_groups() previously allocated all r5worker arrays in a
single kcalloc() block, assigning workers for NUMA node N from node 0
memory. On multi-socket systems this causes remote memory traffic on
every worker->work and worker->temp_inactive_list access.
Replace the single allocation with kzalloc_node(size, GFP_NOIO, i) per
group so each node's workers live in local memory. Because the workers
are now separate per-node allocations, both free sites --
free_thread_groups() and the reallocation path in
raid5_store_group_thread_cnt() -- are updated to free each group's
allocation individually instead of only group 0's.
Also fix a latent bug: the original kcalloc() had its nmemb and size
arguments swapped (harmless due to commutativity but semantically wrong).
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8e9edaaca667..c8787ab7b309 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7297,8 +7297,12 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
conf->worker_groups = new_groups;
spin_unlock_irq(&conf->device_lock);
- if (old_groups)
- kfree(old_groups[0].workers);
+ if (old_groups) {
+ int node;
+
+ for (node = 0; node < num_possible_nodes(); node++)
+ kfree(old_groups[node].workers);
+ }
kfree(old_groups);
}
}
@@ -7336,7 +7340,6 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt, int *group_cnt,
{
int i, j, k;
ssize_t size;
- struct r5worker *workers;
if (cnt == 0) {
*group_cnt = 0;
@@ -7344,24 +7347,24 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt, int *group_cnt,
return 0;
}
*group_cnt = num_possible_nodes();
- size = sizeof(struct r5worker) * cnt;
- workers = kcalloc(size, *group_cnt, GFP_NOIO);
*worker_groups = kzalloc_objs(struct r5worker_group, *group_cnt,
GFP_NOIO);
- if (!*worker_groups || !workers) {
- kfree(workers);
- kfree(*worker_groups);
+ if (!*worker_groups)
return -ENOMEM;
- }
+ size = sizeof(struct r5worker) * cnt;
for (i = 0; i < *group_cnt; i++) {
- struct r5worker_group *group;
+ struct r5worker_group *group = &(*worker_groups)[i];
+ struct r5worker *workers;
+
+ workers = kzalloc_node(size, GFP_NOIO, i);
+ if (!workers)
+ goto out_free;
- group = &(*worker_groups)[i];
INIT_LIST_HEAD(&group->handle_list);
INIT_LIST_HEAD(&group->loprio_list);
group->conf = conf;
- group->workers = workers + i * cnt;
+ group->workers = workers;
for (j = 0; j < cnt; j++) {
struct r5worker *worker = group->workers + j;
@@ -7374,12 +7377,22 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt, int *group_cnt,
}
return 0;
+
+out_free:
+ while (--i >= 0)
+ kfree((*worker_groups)[i].workers);
+ kfree(*worker_groups);
+ *worker_groups = NULL;
+ return -ENOMEM;
}
static void free_thread_groups(struct r5conf *conf)
{
+ int i;
+
if (conf->worker_groups)
- kfree(conf->worker_groups[0].workers);
+ for (i = 0; i < conf->group_cnt; i++)
+ kfree(conf->worker_groups[i].workers);
kfree(conf->worker_groups);
conf->worker_groups = NULL;
}
--
2.43.0
^ permalink raw reply related
* [PATCH 7/8] md/raid5: raise MAX_STRIPE_BATCH from 8 to 32
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
handle_active_stripes() dequeues up to MAX_STRIPE_BATCH stripes under
device_lock per pass, then processes them after dropping the lock.
Raising the batch from 8 to 32 amortizes each device_lock acquisition
over more stripes, reducing lock cycles per unit of stripe work.
The worker-spawn heuristic in raid5_wakeup_stripe_thread() also divided
by MAX_STRIPE_BATCH; split it into a separate STRIPE_BATCH_WORKERS=8 so
the spawn sensitivity is preserved unchanged while the batch size grows.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 2 +-
drivers/md/raid5.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index c8787ab7b309..ad6230415af3 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -210,7 +210,7 @@ static void raid5_wakeup_stripe_thread(struct stripe_head *sh)
/* at least one worker should run to avoid race */
queue_work_on(sh->cpu, raid5_wq, &group->workers[0].work);
- thread_cnt = group->stripes_cnt / MAX_STRIPE_BATCH - 1;
+ thread_cnt = group->stripes_cnt / STRIPE_BATCH_WORKERS - 1;
/* wakeup more workers */
for (i = 1; i < conf->worker_cnt_per_group && thread_cnt > 0; i++) {
if (group->workers[i].working == false) {
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 7aeba1fc7f09..1f37dabd727b 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -496,7 +496,8 @@ struct disk_info {
#define BYPASS_THRESHOLD 1
#define NR_HASH (PAGE_SIZE / sizeof(struct hlist_head))
#define HASH_MASK (NR_HASH - 1)
-#define MAX_STRIPE_BATCH 8
+#define MAX_STRIPE_BATCH 32 /* stripes per handle_active_stripes pass */
+#define STRIPE_BATCH_WORKERS 8 /* stripes-per-worker threshold for spawning */
#define RAID5_SYNC_WINDOW 32 /* stripes to pre-submit per sync_request call */
/* NR_STRIPE_HASH_LOCKS must be a power of two, since
--
2.43.0
^ permalink raw reply related
* [PATCH 8/8] md/raid5: reserve stripe cache for user I/O during rebuild
From: Hiroshi Nishida @ 2026-06-24 15:54 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
The resync read-ahead window (RAID5_SYNC_WINDOW) can fill the stripe
cache with rebuild stripes and starve concurrent user I/O, producing a
burst-starvation flip-flop between rebuild and application throughput.
Add two yield points to the window-submission loop:
- stop the window immediately if any thread is waiting for a stripe
(waitqueue_active(&conf->wait_for_stripe)); the check is intentionally
racy -- a waiter appearing just after is serviced by the next
sync_request call, so no barrier is needed.
- stop expanding once active_stripes reaches half the cache
(max_nr_stripes / RAID5_SYNC_HWMARK), but only when
preread_active_stripes > 0, i.e. user write I/O is actually competing.
Sync stripes never set STRIPE_PREREAD_ACTIVE, so during a pure rebuild
the counter stays zero and the window fills freely; rebuild-only
throughput is unchanged.
This bounds the share of the stripe cache a rebuild may hold while user
I/O is present, so application latency no longer collapses during the
read-ahead bursts, without throttling a rebuild that has the array to
itself.
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 21 +++++++++++++++++++++
drivers/md/raid5.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ad6230415af3..480f3aa069ef 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6656,6 +6656,27 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
win_sector < mddev->resync_max;
submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
+ /*
+ * Yield to user I/O: stop the read-ahead if anyone is waiting
+ * for a stripe. The check is intentionally racy -- a waiter
+ * appearing just after is serviced by the next sync_request
+ * call, so no barrier is needed.
+ */
+ if (waitqueue_active(&conf->wait_for_stripe))
+ break;
+ /*
+ * Reserve cache for user I/O only when it is actually competing.
+ * preread_active_stripes counts stripes queued for write I/O
+ * (including the read phase of RMW); sync stripes never set
+ * STRIPE_PREREAD_ACTIVE, so during a pure rebuild it stays zero
+ * and the window fills freely. Competing user reads do not bump
+ * the counter but are caught by the waitqueue_active() check
+ * above.
+ */
+ if (atomic_read(&conf->preread_active_stripes) > 0 &&
+ atomic_read(&conf->active_stripes) >=
+ conf->max_nr_stripes / RAID5_SYNC_HWMARK)
+ break;
sh = raid5_get_active_stripe(conf, NULL, win_sector,
R5_GAS_NOBLOCK);
if (!sh)
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 1f37dabd727b..7833cc07597f 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -499,6 +499,7 @@ struct disk_info {
#define MAX_STRIPE_BATCH 32 /* stripes per handle_active_stripes pass */
#define STRIPE_BATCH_WORKERS 8 /* stripes-per-worker threshold for spawning */
#define RAID5_SYNC_WINDOW 32 /* stripes to pre-submit per sync_request call */
+#define RAID5_SYNC_HWMARK 2 /* rebuild uses at most 1/N of stripe cache */
/* NR_STRIPE_HASH_LOCKS must be a power of two, since
* STRIPE_HASH_LOCKS_MASK masks with (NR_STRIPE_HASH_LOCKS - 1).
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 5/5] md: clarify the resync ETA comment in status_resync()
From: sashiko-bot @ 2026-06-24 15:58 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260624155421.211626-6-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added comment incorrectly states that the divisor `db` is scaled up by 32, when the code actually scales it down.
--
commit 21672ecf7d2b823245c32b9d2699f7cf1a5c2e7c
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md: clarify the resync ETA comment in status_resync()
This commit updates the block comment above the remaining-time computation
in status_resync() to clarify the calculation logic. It is a comment-only
change.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 0cd85cc92ed31..a5c0da0d11334 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8916,17 +8916,11 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
> * db: blocks written from mark until now
> * rt: remaining time
> *
> - * rt is a sector_t, which is always 64bit now. We are keeping
> - * the original algorithm, but it is not really necessary.
> - *
> - * Original algorithm:
> - * So we divide before multiply in case it is 32bit and close
> - * to the limit.
> - * We scale the divisor (db) by 32 to avoid losing precision
> - * near the end of resync when the number of remaining sectors
> - * is close to 'db'.
> - * We then divide rt by 32 after multiplying by db to compensate.
> - * The '+1' avoids division by zero if db is very small.
> + * rt is computed as (remaining sectors) * dt / db. To keep precision
> + * near the end of resync, when the remaining count is close to db, the
> + * divisor db is scaled up by 32 before the divide and rt is scaled back
[Severity: Low]
Is this new comment accurate regarding the math applied to db?
The comment states that the divisor db is scaled up by 32, but looking at
the actual code:
rt = max_sectors - resync;
rt = div64_u64(rt, db/32+1);
This divides db by 32, effectively scaling it down rather than up (which in
turn scales the resulting quotient up). Does this comment need to be
adjusted to reflect the db/32 downscaling?
> + * down by 32 afterwards. The '+1' avoids division by zero when db is
> + * very small.
> */
> dt = ((jiffies - mddev->resync_mark) / HZ);
> if (!dt) dt++;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624155421.211626-1-nishidafmly@gmail.com?part=5
^ 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