* Re: [PATCH 4/5] md: use sector_t for recovery_active in status_resync()
From: Hiroshi Nishida @ 2026-06-24 16:48 UTC (permalink / raw)
To: sashiko-reviews; +Cc: yukuai, linux-raid
In-Reply-To: <20260624160331.9C5F51F000E9@smtp.kernel.org>
> [Medium] mddev->recovery_active is a standard atomic_t, not "sector-typed";
> changing the local to sector_t doesn't prevent narrowing.
You're right -- mddev->recovery_active is an atomic_t and
atomic_read() returns int, so there's nothing wider to narrow; the
int->sector_t conversion just moves to the assignment.
The change is only a type-consistency tidy (the local is used solely
with the sector_t curr_mark_cnt/resync_mark_cnt), not a narrowing fix.
I'll reword the commit message in v2 accordingly and drop the
"sector-typed atomic"/"narrows" wording.
2026年6月24日(水) 9:03 <sashiko-bot@kernel.org>:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The commit message falsely claims `mddev->recovery_active` is a 'sector-typed atomic' and that declaring a local variable as `int` 'needlessly narrows it', but the structure field remains a standard 32-bit `atomic_t`.
> --
>
> commit 7ec77a6b233ce4d151f7fc723e5f312b20b4ac92
> Author: Hiroshi Nishida <nishidafmly@gmail.com>
>
> md: use sector_t for recovery_active in status_resync()
>
> This commit changes the local variable recovery_active in status_resync()
> from int to sector_t. The commit message states this is to match the type
> of mddev->recovery_active and avoid needlessly narrowing the value.
>
> > 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.
>
> [Severity: Medium]
> Is this description accurate? Looking at struct mddev in drivers/md/md.h,
> recovery_active is defined as a standard atomic_t:
>
> drivers/md/md.h:struct mddev {
> ...
> atomic_t recovery_active;
> ...
> }
>
> Since atomic_t is a 32-bit signed integer, and atomic_read() returns an int,
> does changing this local variable to sector_t actually prevent narrowing? The
> implicit cast from int to sector_t will now simply happen at assignment
> rather than during the arithmetic operations.
>
> Could the commit message be updated to reflect that mddev->recovery_active
> is a standard atomic_t, rather than a "sector-typed atomic"?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260624155421.211626-1-nishidafmly@gmail.com?part=4
--
Hiroshi Nishida
nishidafmly@gmail.com
^ permalink raw reply
* Re: [PATCH 1/8] md: change chunk_sectors and stripe cache counts to unsigned int
From: sashiko-bot @ 2026-06-24 16:16 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260624155452.211646-2-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] The commit's core justification is false. Changing the types to unsigned does not enable compiler right-shift optimizations for variable divisions.
- [Low] Unupdated printf-style format specifiers passing unsigned fields to '%d' cause compiler warnings (-Wformat) and break builds configured with -Werror.
Pre-existing issues:
- [Critical] The macro `round_down` is incorrectly used with a non-power-of-two divisor, causing silent data corruption in bitmap sector calculations.
--
commit 86528a8b22367a5ee16a7cb19a1d56bf58f6aa67
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md: change chunk_sectors and stripe cache counts to unsigned int
This commit changes several structural fields and local variables like
chunk_sectors and max_nr_stripes from signed to unsigned int. The stated
rationale is to prevent negative values and enable right-shift compiler
optimizations for power-of-two division in the hot I/O path.
> 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.
[Severity: Medium]
Is this optimization rationale accurate? The hot I/O path uses sector_div(),
which expands to do_div(). The do_div() macro explicitly casts the divisor
to a 32-bit unsigned integer, so the division was already operating on an
unsigned value.
Additionally, because sectors_per_chunk is a runtime variable rather than a
compile-time constant, would the compiler be able to optimize it into a
right shift without a conditional branch?
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index d8daf0f75cbbe..b9ad26844799d 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;
[Severity: Low]
Does changing this field to unsigned int cause compiler warnings with
existing format strings?
For example, chunk_size_show() in drivers/md/md.c appears to use %d to
format mddev->chunk_sectors:
chunk_size_show() {
...
return sprintf(page, "%d (%d)\n",
mddev->new_chunk_sectors << 9,
mddev->chunk_sectors << 9);
}
Will this break builds configured with -Werror?
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index cb5feae04db27..5cd9d0f36b6e0 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;
[Severity: Low]
Will the changes to min_nr_stripes and max_nr_stripes also trigger format
warnings?
Places like raid5_end_read_request() and raid5_set_cache_size() still use
%d to print these fields. For instance, in raid5_set_cache_size():
pr_err("md/raid:%s: Cannot increase cache size, ret=%d, new_size=%d, min_nr_stripes=%d, max_nr_stripes=%d\n",
...
conf->min_nr_stripes,
conf->max_nr_stripes);
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..28828e083c2b3 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;
[Severity: Critical]
This is a pre-existing issue, but could the use of sectors_per_chunk
cause problems when passed to rounding macros?
Looking at raid5_bitmap_sector():
raid5_bitmap_sector() {
...
sectors_per_chunk = conf->chunk_sectors *
(conf->raid_disks - conf->max_degraded);
start = round_down(start, sectors_per_chunk);
end = round_up(end, sectors_per_chunk);
...
}
The round_down() and round_up() macros are designed specifically for
power-of-two divisors. If the number of data disks is not a power of two,
sectors_per_chunk will not be a power of two, which would yield incorrect
sector boundaries.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624155452.211646-1-nishidafmly@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 8/8] md/raid5: reserve stripe cache for user I/O during rebuild
From: sashiko-bot @ 2026-06-24 16:12 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260624155452.211646-9-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch fails to bound the rebuild's share of the stripe cache to 50%. Breaking the read-ahead batch loop just causes the outer md_do_sync() loop to allocate stripes one-by-one, relentlessly filling the cache to 100% anyway.
--
commit 29ed9bbd7953c5d6ceeb0158cc53c81fd4d3aaf9
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: reserve stripe cache for user I/O during rebuild
This commit attempts to prevent rebuild operations from monopolizing the
stripe cache and starving concurrent user I/O. It adds yield points in the
resync read-ahead loop to stop submitting stripes if there are active
waiters or if the cache is more than half full.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> --- 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)) {
[ ... ]
> + /*
> + * 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;
[Severity: High]
Does breaking out of the read-ahead loop here actually limit the cache
usage to 50%?
When this breaks, raid5_sync_request() returns control to md_do_sync(),
which operates in a loop and will immediately call raid5_sync_request()
again for the next sector.
Because raid5_sync_request() unconditionally allocates its first stripe
earlier in the function before entering this batch loop, won't the outer
loop just continue allocating stripes one-by-one until the cache reaches
100% capacity and triggers the fallback blocking path anyway?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624155452.211646-1-nishidafmly@gmail.com?part=8
^ permalink raw reply
* Re: [PATCH 5/8] md/raid5: submit a window of stripes during resync/recovery
From: sashiko-bot @ 2026-06-24 16:12 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260624155452.211646-6-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The batch loop ignores the return value of md_bitmap_start_sync(), forcing clean stripes to be unnecessarily synchronized.
- [High] Time-of-check to time-of-use race on the array's degraded state, leading to silent data corruption via incorrect bitmap updates.
--
commit 01d1e80ee6aa253c48c233e7fc218fbdac9522fa
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: submit a window of stripes during resync/recovery
This commit alters raid5_sync_request() to dispatch up to 32 stripes per
call instead of one. It attempts to submit the remaining stripes in the
window with a non-blocking flag to avoid the per-stripe throttle sleep.
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 9cb4ed3bd85c5..8e9edaaca667b 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6620,16 +6621,7 @@ static inline sector_t raid5_sync_request(...)
[ ... ]
> - 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(...)
> 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);
> + }
[Severity: High]
Does moving the degraded check before the stripe allocation introduce a
time-of-check to time-of-use race?
If a drive fails during the potential sleep in raid5_get_active_stripe() or
schedule_timeout_uninterruptible(), the stale still_degraded value is used
below to update the bitmap. This could incorrectly clear the NEEDED bit,
potentially leaving parity inconsistent if the system crashes before the sync
completes.
> md_bitmap_start_sync(mddev, sector_nr, &sync_blocks, still_degraded);
> set_bit(STRIPE_SYNC_REQUESTED, &sh->state);
> set_bit(STRIPE_HANDLE, &sh->state);
[ ... ]
> + 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);
[Severity: High]
Does this batch loop unnecessarily synchronize clean stripes?
The boolean return value of md_bitmap_start_sync() is ignored here. If the
first stripe needs sync but subsequent stripes in the window are clean, this
loop unconditionally queues every stripe for synchronization regardless of
its clean/dirty state, forcing read-rebuild-write cycles on clean stripes.
> + raid5_release_stripe(sh);
> + }
> +
> + return submitted * RAID5_STRIPE_SECTORS(conf);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624155452.211646-1-nishidafmly@gmail.com?part=5
^ permalink raw reply related
* Re: [PATCH 7/8] md/raid5: raise MAX_STRIPE_BATCH from 8 to 32
From: sashiko-bot @ 2026-06-24 16:09 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260624155452.211646-8-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Increasing MAX_STRIPE_BATCH to 32 without proportionally increasing STRIPE_BATCH_WORKERS serializes stripe processing and causes worker starvation.
--
commit b1a0a169dabd8b46b6b9ca40375034cf879934c6
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: raise MAX_STRIPE_BATCH from 8 to 32
This commit increases the maximum stripe batch size from 8 to 32 to
amortize device_lock acquisition over more stripes. It also introduces
STRIPE_BATCH_WORKERS to decouple the worker spawning threshold from the
batch size.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index c8787ab7b309c..ad6230415af33 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++) {
[Severity: High]
Will increasing the stripe batch size while keeping the worker spawn threshold
small serialize stripe processing and starve the worker threads?
With this change, raid5_wakeup_stripe_thread() still wakes up an additional
worker for every 8 pending stripes (e.g., waking 4 threads for 32 stripes).
However, the first awakened worker executing handle_active_stripes() will
greedily dequeue up to MAX_STRIPE_BATCH (32) stripes under the lock:
drivers/md/raid5.c:handle_active_stripes() {
...
while (batch_size < MAX_STRIPE_BATCH &&
(sh = __get_priority_stripe(conf, group)) != NULL)
batch[batch_size++] = sh;
...
}
It appears the first thread will steal all 32 pending stripes and process
them sequentially, while the other awakened threads acquire the lock,
find the lists empty, and exit without doing any work.
Could this cause a performance regression by serializing CPU-intensive parity
operations onto a single thread, while also creating unnecessary lock
contention from the starved threads?
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index 7aeba1fc7f09e..1f37dabd727be 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 */
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624155452.211646-1-nishidafmly@gmail.com?part=7
^ permalink raw reply
* Re: [PATCH 6/8] md/raid5: allocate worker groups per NUMA node
From: sashiko-bot @ 2026-06-24 16:07 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260624155452.211646-7-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch incorrectly uses a sequential loop index `i` as the NUMA node ID parameter to `kzalloc_node()`.
Pre-existing issues:
- [High] The `worker_groups` array is sized using `num_possible_nodes()` but is indexed by the raw NUMA node ID, leading to out-of-bounds accesses on systems with sparse NUMA node IDs.
--
commit bce08bf6ce446b00da625ff4028f5782b756e17c
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: allocate worker groups per NUMA node
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.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 8e9edaaca667b..c8787ab7b309c 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -7336,7 +7340,6 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt, int *group_cnt,
[ ... ]
> *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);
[Severity: High]
This isn't a bug introduced by this patch, but could this array sizing cause
an out-of-bounds access on systems with sparse NUMA nodes?
We size the worker_groups array using num_possible_nodes(), which is a count.
On systems with sparse NUMA node IDs (for example, nodes 0 and 8), the array
would only have 2 elements. Later, in raid5_wakeup_stripe_thread(), we index
into this array using cpu_to_group(cpu) which expands to cpu_to_node(cpu).
If a CPU belongs to node 8, we would access conf->worker_groups[8], exceeding
the allocated size and corrupting memory.
> - 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);
[Severity: High]
Could passing the sequential loop index i directly to kzalloc_node cause a
NULL pointer dereference on systems with sparse NUMA nodes?
Since num_possible_nodes returns a count rather than an upper bound on IDs,
on a system with nodes 0 and 8, i would be evaluated as 1. Passing an
unpopulated node ID to kzalloc_node without validation can lead to a crash
when accessing NODE_DATA(nid). Should this loop use
for_each_node_state(nid, N_MEMORY) or similar to get the actual node ID
instead of using the loop counter?
> + if (!workers)
> + goto out_free;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624155452.211646-1-nishidafmly@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH 4/5] md: use sector_t for recovery_active in status_resync()
From: sashiko-bot @ 2026-06-24 16:03 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260624155421.211626-5-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message falsely claims `mddev->recovery_active` is a 'sector-typed atomic' and that declaring a local variable as `int` 'needlessly narrows it', but the structure field remains a standard 32-bit `atomic_t`.
--
commit 7ec77a6b233ce4d151f7fc723e5f312b20b4ac92
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md: use sector_t for recovery_active in status_resync()
This commit changes the local variable recovery_active in status_resync()
from int to sector_t. The commit message states this is to match the type
of mddev->recovery_active and avoid needlessly narrowing the value.
> 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.
[Severity: Medium]
Is this description accurate? Looking at struct mddev in drivers/md/md.h,
recovery_active is defined as a standard atomic_t:
drivers/md/md.h:struct mddev {
...
atomic_t recovery_active;
...
}
Since atomic_t is a 32-bit signed integer, and atomic_read() returns an int,
does changing this local variable to sector_t actually prevent narrowing? The
implicit cast from int to sector_t will now simply happen at assignment
rather than during the arithmetic operations.
Could the commit message be updated to reflect that mddev->recovery_active
is a standard atomic_t, rather than a "sector-typed atomic"?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624155421.211626-1-nishidafmly@gmail.com?part=4
^ permalink raw reply
* 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
* [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
* [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 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 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 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 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 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 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 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 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 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 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 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 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 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
* 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
* [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
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