* Re: [PATCH] md/raid1: honor REQ_NOWAIT when waiting for behind writes
From: sashiko-bot @ 2026-06-11 8:49 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: yukuai, linux-raid
In-Reply-To: <20260611083514.754922-1-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Leaking pre-allocated `r1_bio` when `REQ_NOWAIT` reads fail `wait_read_barrier()` during retry.
- [High] Permanent barrier leak if a REQ_NOWAIT write encounters a blocked device.
- [Medium] REQ_NOWAIT semantics violation: memory allocations use GFP_NOIO unconditionally.
--
commit 7fed7389f4fe20e79eece974eaf3b8d317519e27
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid1: honor REQ_NOWAIT when waiting for behind writes
This commit ensures that raid1 consistently honors REQ_NOWAIT reads
across paths that may otherwise wait for behind writes. It modifies
bitmap_wait_behind_writes() to return early instead of sleeping if
the nowait flag is set.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 5b9368bd9e700..d03da046a5920 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -1360,8 +1361,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> * Still need barrier for READ in case that whole
> * array is frozen.
> */
> - if (!wait_read_barrier(conf, bio->bi_iter.bi_sector,
> - bio->bi_opf & REQ_NOWAIT)) {
> + if (!wait_read_barrier(conf, bio->bi_iter.bi_sector, nowait)) {
> bio_wouldblock_error(bio);
> return;
> }
[Severity: High]
This is a pre-existing issue, but does this early return leak the
pre-allocated r1_bio when wait_read_barrier() fails during a retry?
When a REQ_NOWAIT read fails its initial attempt, it is queued to the
retry_list and handled by the raid1d thread via handle_read_error().
That function passes the existing, pre-allocated r1_bio back to
raid1_read_request():
drivers/md/raid1.c:handle_read_error() {
...
raid1_read_request(mddev, bio, r1_bio->sectors, r1_bio);
allow_barrier(conf, sector);
...
}
If the array is concurrently frozen, wait_read_barrier() returns false
since REQ_NOWAIT is still set on the master bio. The code handles this
by returning immediately, bypassing the err_handle cleanup path and
failing to call raid_end_bio_io().
Since handle_read_error() assumes ownership was taken and only releases
its old barrier, does this result in a permanent memory leak of the
r1_bio from the r1bio_pool mempool?
[Severity: Medium]
This is another pre-existing issue, but does the allocation path for
REQ_NOWAIT bios violate block layer non-blocking semantics by
unconditionally using GFP_NOIO?
Even when REQ_NOWAIT is requested, alloc_r1bio() uses GFP_NOIO
unconditionally:
drivers/md/raid1.c:alloc_r1bio() {
...
r1_bio = mempool_alloc(conf->r1bio_pool, GFP_NOIO);
...
}
Similarly, bio_alloc_clone() in raid1_read_request() unconditionally
uses gfp flags that include __GFP_DIRECT_RECLAIM.
Since mempool-backed allocations will sleep under memory pressure
instead of failing immediately when these flags are used, will this
cause unexpected high latencies and potential priority inversion in
asynchronous contexts?
[Severity: High]
This isn't a bug introduced by this patch, but does
raid1_write_request() permanently leak the barrier if a REQ_NOWAIT
write encounters a blocked device?
In raid1_write_request(), the code successfully acquires a barrier by
calling wait_barrier(), which internally increments conf->nr_pending.
Immediately after, it calls wait_blocked_rdev():
drivers/md/raid1.c:raid1_write_request() {
...
if (!wait_blocked_rdev(mddev, bio)) {
bio_wouldblock_error(bio);
return;
}
...
}
If the device is blocked and the request is REQ_NOWAIT,
wait_blocked_rdev() returns false and the function returns early.
However, it fails to call allow_barrier() before returning to drop
the previously acquired barrier increment.
Because freeze_array() waits for nr_pending to reach 0, could this
permanently prevent the array from ever freezing again, deadlocking
resyncs, reconfiguration, and array teardown?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
^ permalink raw reply
* [PATCH] md/raid1: free r1_bio when REQ_NOWAIT is set and read would block on retry
From: Abd-Alrhman Masalkhi @ 2026-06-11 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, vverma, axboe
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi, sashiko-bot
When a read is retried, raid1_read_request() may be called with a
pre-allocated r1_bio. If wait_read_barrier() fails for a REQ_NOWAIT
read, the bio is completed and the function returns immediately. In this
case the existing r1_bio is leaked.
This fixes a leak of pre-allocated r1_bio structures for retried reads.
Fixes: 5aa705039c4f ("md: raid1 add nowait support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/md/raid1.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index b2d7c13b64bd..6263a1d45f86 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1363,6 +1363,12 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
*/
if (!wait_read_barrier(conf, bio->bi_iter.bi_sector, nowait)) {
bio_wouldblock_error(bio);
+
+ if (r1bio_existed) {
+ set_bit(R1BIO_Returned, &r1_bio->state);
+ raid_end_bio_io(r1_bio);
+ }
+
return;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] md/raid1: honor REQ_NOWAIT when waiting for behind writes
From: Abd-Alrhman Masalkhi @ 2026-06-11 10:49 UTC (permalink / raw)
To: sashiko-reviews; +Cc: yukuai, linux-raid
In-Reply-To: <20260611084953.E078E1F00898@smtp.kernel.org>
On Thu, Jun 11, 2026 at 08:49 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Leaking pre-allocated `r1_bio` when `REQ_NOWAIT` reads fail `wait_read_barrier()` during retry.
> - [High] Permanent barrier leak if a REQ_NOWAIT write encounters a blocked device.
> - [Medium] REQ_NOWAIT semantics violation: memory allocations use GFP_NOIO unconditionally.
These appear to be pre-existing issues rather than being introduced by
this patch. I plan to address them in separate patches.
> --
>
> commit 7fed7389f4fe20e79eece974eaf3b8d317519e27
> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>
> md/raid1: honor REQ_NOWAIT when waiting for behind writes
>
> This commit ensures that raid1 consistently honors REQ_NOWAIT reads
> across paths that may otherwise wait for behind writes. It modifies
> bitmap_wait_behind_writes() to return early instead of sleeping if
> the nowait flag is set.
>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 5b9368bd9e700..d03da046a5920 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
> [ ... ]
>> @@ -1360,8 +1361,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>> * Still need barrier for READ in case that whole
>> * array is frozen.
>> */
>> - if (!wait_read_barrier(conf, bio->bi_iter.bi_sector,
>> - bio->bi_opf & REQ_NOWAIT)) {
>> + if (!wait_read_barrier(conf, bio->bi_iter.bi_sector, nowait)) {
>> bio_wouldblock_error(bio);
>> return;
>> }
>
> [Severity: High]
> This is a pre-existing issue, but does this early return leak the
> pre-allocated r1_bio when wait_read_barrier() fails during a retry?
>
> When a REQ_NOWAIT read fails its initial attempt, it is queued to the
> retry_list and handled by the raid1d thread via handle_read_error().
>
> That function passes the existing, pre-allocated r1_bio back to
> raid1_read_request():
>
> drivers/md/raid1.c:handle_read_error() {
> ...
> raid1_read_request(mddev, bio, r1_bio->sectors, r1_bio);
> allow_barrier(conf, sector);
> ...
> }
>
> If the array is concurrently frozen, wait_read_barrier() returns false
> since REQ_NOWAIT is still set on the master bio. The code handles this
> by returning immediately, bypassing the err_handle cleanup path and
> failing to call raid_end_bio_io().
>
> Since handle_read_error() assumes ownership was taken and only releases
> its old barrier, does this result in a permanent memory leak of the
> r1_bio from the r1bio_pool mempool?
>
> [Severity: Medium]
> This is another pre-existing issue, but does the allocation path for
> REQ_NOWAIT bios violate block layer non-blocking semantics by
> unconditionally using GFP_NOIO?
>
> Even when REQ_NOWAIT is requested, alloc_r1bio() uses GFP_NOIO
> unconditionally:
>
> drivers/md/raid1.c:alloc_r1bio() {
> ...
> r1_bio = mempool_alloc(conf->r1bio_pool, GFP_NOIO);
> ...
> }
>
> Similarly, bio_alloc_clone() in raid1_read_request() unconditionally
> uses gfp flags that include __GFP_DIRECT_RECLAIM.
>
> Since mempool-backed allocations will sleep under memory pressure
> instead of failing immediately when these flags are used, will this
> cause unexpected high latencies and potential priority inversion in
> asynchronous contexts?
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does
> raid1_write_request() permanently leak the barrier if a REQ_NOWAIT
> write encounters a blocked device?
>
> In raid1_write_request(), the code successfully acquires a barrier by
> calling wait_barrier(), which internally increments conf->nr_pending.
>
> Immediately after, it calls wait_blocked_rdev():
>
> drivers/md/raid1.c:raid1_write_request() {
> ...
> if (!wait_blocked_rdev(mddev, bio)) {
> bio_wouldblock_error(bio);
> return;
> }
> ...
> }
>
> If the device is blocked and the request is REQ_NOWAIT,
> wait_blocked_rdev() returns false and the function returns early.
> However, it fails to call allow_barrier() before returning to drop
> the previously acquired barrier increment.
>
> Because freeze_array() waits for nr_pending to reach 0, could this
> permanently prevent the array from ever freezing again, deadlocking
> resyncs, reconfiguration, and array teardown?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* [PATCH] md/raid1: release barrier when REQ_NOWAIT write would block
From: Abd-Alrhman Masalkhi @ 2026-06-11 13:25 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi, sashiko-bot
raid1_write_request() calls wait_barrier(), which raises
conf->nr_pending, before calling wait_blocked_rdev(). When
wait_blocked_rdev() fails for a REQ_NOWAIT write, the bio is completed
and the function returns without calling allow_barrier(), leaking the
nr_pending reference taken by wait_barrier().
freeze_array() waits for nr_pending to drop to zero, so a leaked
reference prevents the array from ever being frozen again. Any later
operation that freezes the array then hangs, including resync, reshape,
device removal and array teardown.
Release the barrier on the REQ_NOWAIT failure path.
Fixes: 88ed59c4cc6c ("md/raid1: factor out helper to handle blocked rdev from raid1_write_request()")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/md/raid1.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 6263a1d45f86..e9086ba1f796 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1522,6 +1522,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
int max_sectors;
bool write_behind = false;
bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
+ sector_t sector = bio->bi_iter.bi_sector;
if (mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1550,6 +1551,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
if (!wait_blocked_rdev(mddev, bio)) {
bio_wouldblock_error(bio);
+ allow_barrier(conf, sector);
return;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] md/raid1: release barrier when REQ_NOWAIT write would block
From: sashiko-bot @ 2026-06-11 13:41 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: yukuai, linux-raid
In-Reply-To: <20260611132500.763528-1-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing RCU read lock around `conf->mirrors[i].rdev` access in `wait_blocked_rdev()`.
- [High] Early failure paths in `raid1_write_request()` leak the `mddev->writes_pending` reference.
- [High] Allocating bios and r1bios can sleep even when the `REQ_NOWAIT` flag is set.
--
commit 3db9da0ec804800aa3afb93361ccd18de39ce5f0
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid1: release barrier when REQ_NOWAIT write would block
raid1_write_request() leaks the barrier reference taken by wait_barrier()
if wait_blocked_rdev() fails for a REQ_NOWAIT write. This patch releases
the barrier on this failure path to prevent array freeze hangs.
Closes: https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 5b9368bd9e700..f3cc30932ba2b 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1512,6 +1512,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> int max_sectors;
> bool write_behind = false;
> bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
> + sector_t sector = bio->bi_iter.bi_sector;
>
> if (mddev_is_clustered(mddev) &&
> mddev->cluster_ops->area_resyncing(mddev, WRITE,
> @@ -1540,6 +1541,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>
> if (!wait_blocked_rdev(mddev, bio)) {
> bio_wouldblock_error(bio);
> + allow_barrier(conf, sector);
[Severity: High]
This is a pre-existing issue, but does this early return path also leak the
mddev->writes_pending reference?
In raid1_make_request(), md_write_start() unconditionally takes a per-cpu
reference on mddev->writes_pending before calling raid1_write_request().
Normally, this is released via md_write_end() inside close_write().
By directly returning here after bio_wouldblock_error(bio), we never call
md_write_end(). Does this permanently leak the writes_pending counter and
prevent the array from transitioning to a clean state?
It looks like this same leak exists on the clustered area_resyncing and
wait_barrier REQ_NOWAIT failure paths earlier in this function, as well as
the err_handle path if bio_submit_split_bioset() fails.
> return;
> }
[Severity: Critical]
This is a pre-existing issue, but is it safe to call wait_blocked_rdev()
here without holding the RCU read lock?
Looking at wait_blocked_rdev():
for (i = 0; i < disks; i++) {
struct md_rdev *rdev = conf->mirrors[i].rdev;
if (!rdev)
continue;
/* don't write here until the bad block is acknowledged */
if (test_bit(WriteErrorSeen, &rdev->flags) &&
It accesses the rdev pointer and its fields without rcu_read_lock().
Could a concurrent device removal (via md_ioctl -> raid1_remove_conf)
replace the pointer with NULL and free the rdev structure while we are
dereferencing it here, resulting in a use-after-free?
[Severity: High]
This is also a pre-existing issue, but if we proceed past this point for a
REQ_NOWAIT bio, we call alloc_r1bio() and eventually bio_alloc_clone(),
both of which use GFP_NOIO.
Since memory pool allocations with GFP_NOIO can block when the mempool is
exhausted under memory pressure, could this cause REQ_NOWAIT writes to
sleep, defeating the purpose of the REQ_NOWAIT flag?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260611132500.763528-1-abd.masalkhi@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH next] drivers/md/md: Use bounded strings ops when setting disk->disk_name
From: Li Nan @ 2026-06-12 3:09 UTC (permalink / raw)
To: david.laight.linux, Kees Cook, linux-hardening, linux-kernel,
linux-raid
Cc: Arnd Bergmann, Song Liu, Yu Kuai
In-Reply-To: <20260608095523.2606-26-david.laight.linux@gmail.com>
On Mon Jun 8, 2026 at 5:55 PM CST, david.laight.linux wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> Replace strcpy() with strscpy() and sprintf() with snprintf() for safety.
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> This is one of a group of patches that remove potentially unbounded
> strcpy() calls.
>
> They are mostly replaced by strscpy() or, when strlen() has just been
> called, with memcpy() (usually including the '\0').
>
> Calls with copy string literals into arrays are left unchanged.
> They are safe and easily detected as such.
>
> The changes were made by getting the compiler to detect the calls and
> then fixing the code by hand.
>
> Note that all the changes are only compile tested.
>
> Some Makefiles were changed to allow files to contain strcpy().
> As well as 'difficult to fix' files, this included 'show' functions
> as they really need to use sysfs_emit() or seq_printf().
>
> All the patches are being sent individually to avoid very long cc lists.
> Apologies for the terse commit messages and likely unexpected tags.
> (There are about 100 patches in total.)
>
> 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 8b568eee8743..4b659d10fa4d 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6346,11 +6346,11 @@ struct mddev *md_alloc(dev_t dev, char *name)
> disk->first_minor = unit << shift;
> disk->minors = 1 << shift;
> if (name)
> - strcpy(disk->disk_name, name);
> + strscpy(disk->disk_name, name);
> else if (partitioned)
> - sprintf(disk->disk_name, "md_d%d", unit);
> + snprintf(disk->disk_name, sizeof (disk->disk_name), "md_d%d", unit);
Please remove the space after sizeof to match the style used elsewhere in this file.
> else
> - sprintf(disk->disk_name, "md%d", unit);
> + snprintf(disk->disk_name, sizeof (disk->disk_name), "md%d", unit);
> disk->fops = &md_fops;
> disk->private_data = mddev;
>
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH] raid10: badblock-aware reshape write error handling
From: Li Nan @ 2026-06-12 3:42 UTC (permalink / raw)
To: ghuicao, song, yukuai
Cc: linan122, xiao, linux-raid, linux-kernel, Cao Guanghui
In-Reply-To: <20260601054630.74483-1-ghuicao@163.com>
On Mon Jun 1, 2026 at 1:46 PM CST, ghuicao wrote:
> From: Cao Guanghui <caoguanghui@kylinos.cn>
>
> Replace the FIXME in end_reshape_write(). Instead of failing the device
> immediately on write errors during reshape, attempt to record badblocks
> using new_data_offset with is_new=1.
>
> rdev_set_badblocks() returns true on success. On failure (e.g., badblocks
> table full), it has already called md_error() internally to degrade the
> device. Queue WantReplacement for member devices regardless of badblock
> recording success, but skip this for replacement devices to avoid
> replacement loops.
>
> On successful write, clear stale badblock records at the new location
> since data has migrated.
>
> Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
> ---
> drivers/md/raid10.c | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 4901ebe45c87..08d58a1c680e 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -4991,9 +4991,30 @@ static void end_reshape_write(struct bio *bio)
> conf->mirrors[d].rdev;
>
> if (bio->bi_status) {
> - /* FIXME should record badblock */
> - md_error(mddev, rdev);
> - }
> + set_bit(WriteErrorSeen, &rdev->flags);
> +
> + /* rdev_set_badblocks returns true on success.
> + * On failure, it has already called md_error() internally.
> + * Use is_new=1 as reshape writes target the new layout
> + * (new_data_offset).
> + */
> + if (rdev_set_badblocks(rdev, r10_bio->devs[slot].addr,
> + r10_bio->sectors, 1)) {
> + /* Queue async replacement for member devices
> + * For replacement devices, do not trigger WantReplacement
> + * to avoid circular replacement storms.
> + */
> + if (!repl) {
> + if (!test_and_set_bit(WantReplacement, &rdev->flags))
> + set_bit(MD_RECOVERY_NEEDED,
> + &rdev->mddev->recovery);
The logic here seems a bit odd — several mechanisms are chained together.
The correct logic might be:
1. If there is a replacement, fail directly with md_error
2. If not, mark the badblock and set WantReplacement
> + }
> + }
> + } else {
> + /* Write succeeded, clear stale badblock records */
> + rdev_clear_badblocks(rdev, r10_bio->devs[slot].addr,
> + r10_bio->sectors, 1);
> + }
>
> rdev_dec_pending(rdev, mddev);
> end_reshape_request(r10_bio);
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH] md/raid1: release barrier when REQ_NOWAIT write would block
From: Abd-Alrhman Masalkhi @ 2026-06-12 5:27 UTC (permalink / raw)
To: sashiko-reviews; +Cc: yukuai, linux-raid
In-Reply-To: <20260611134130.B4E911F00893@smtp.kernel.org>
On Thu, Jun 11, 2026 at 13:41 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Missing RCU read lock around `conf->mirrors[i].rdev` access in `wait_blocked_rdev()`.
I'm not quite sure whether this is a real bug.
> - [High] Early failure paths in `raid1_write_request()` leak the `mddev->writes_pending` reference.
I'll submit a v2 containing this fix, along with the fix for the
mddev->writes_pending
> - [High] Allocating bios and r1bios can sleep even when the `REQ_NOWAIT` flag is set.
@kuai, Should I handle this issue as well, or should I just ignore it
for now?
> --
>
> commit 3db9da0ec804800aa3afb93361ccd18de39ce5f0
> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>
> md/raid1: release barrier when REQ_NOWAIT write would block
>
> raid1_write_request() leaks the barrier reference taken by wait_barrier()
> if wait_blocked_rdev() fails for a REQ_NOWAIT write. This patch releases
> the barrier on this failure path to prevent array freeze hangs.
>
> Closes: https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 5b9368bd9e700..f3cc30932ba2b 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -1512,6 +1512,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>> int max_sectors;
>> bool write_behind = false;
>> bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
>> + sector_t sector = bio->bi_iter.bi_sector;
>>
>> if (mddev_is_clustered(mddev) &&
>> mddev->cluster_ops->area_resyncing(mddev, WRITE,
>> @@ -1540,6 +1541,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>>
>> if (!wait_blocked_rdev(mddev, bio)) {
>> bio_wouldblock_error(bio);
>> + allow_barrier(conf, sector);
>
> [Severity: High]
> This is a pre-existing issue, but does this early return path also leak the
> mddev->writes_pending reference?
>
> In raid1_make_request(), md_write_start() unconditionally takes a per-cpu
> reference on mddev->writes_pending before calling raid1_write_request().
> Normally, this is released via md_write_end() inside close_write().
>
> By directly returning here after bio_wouldblock_error(bio), we never call
> md_write_end(). Does this permanently leak the writes_pending counter and
> prevent the array from transitioning to a clean state?
>
> It looks like this same leak exists on the clustered area_resyncing and
> wait_barrier REQ_NOWAIT failure paths earlier in this function, as well as
> the err_handle path if bio_submit_split_bioset() fails.
>
>> return;
>> }
>
> [Severity: Critical]
> This is a pre-existing issue, but is it safe to call wait_blocked_rdev()
> here without holding the RCU read lock?
>
> Looking at wait_blocked_rdev():
>
> for (i = 0; i < disks; i++) {
> struct md_rdev *rdev = conf->mirrors[i].rdev;
>
> if (!rdev)
> continue;
>
> /* don't write here until the bad block is acknowledged */
> if (test_bit(WriteErrorSeen, &rdev->flags) &&
>
> It accesses the rdev pointer and its fields without rcu_read_lock().
> Could a concurrent device removal (via md_ioctl -> raid1_remove_conf)
> replace the pointer with NULL and free the rdev structure while we are
> dereferencing it here, resulting in a use-after-free?
>
> [Severity: High]
> This is also a pre-existing issue, but if we proceed past this point for a
> REQ_NOWAIT bio, we call alloc_r1bio() and eventually bio_alloc_clone(),
> both of which use GFP_NOIO.
>
> Since memory pool allocations with GFP_NOIO can block when the mempool is
> exhausted under memory pressure, could this cause REQ_NOWAIT writes to
> sleep, defeating the purpose of the REQ_NOWAIT flag?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260611132500.763528-1-abd.masalkhi@gmail.com?part=1
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: use READ_ONCE() for rdev_attr_show() mddev check
From: Li Nan @ 2026-06-12 6:22 UTC (permalink / raw)
To: Runyu Xiao, song, yukuai3, linux-raid
Cc: lilingfeng3, linux-kernel, jianhao.xu
In-Reply-To: <20260611083510.476945-1-runyu.xiao@seu.edu.cn>
On Thu Jun 11, 2026 at 4:35 PM CST, Runyu Xiao wrote:
> md_kick_rdev_from_array() clears rdev->mddev with
> WRITE_ONCE(rdev->mddev, NULL), and rdev_attr_store() already snapshots
> that same shared pointer with READ_ONCE(rdev->mddev). rdev_attr_show()
> still tests the pointer with a plain lockless `if (!rdev->mddev)` before
> calling entry->show().
>
> A running system can reach this by reading rdev sysfs attributes while a
> device removal path is tearing the same md_rdev down. In that window,
> rdev_attr_show() can pass a stale plain guard while sibling
> rdev_attr_store() already observes NULL and returns -ENODEV, leaving the
> show path to operate after the shared mddev pointer has been revoked.
>
> Use READ_ONCE() in rdev_attr_show() so this sysfs read side matches the
> existing visibility contract on rdev->mddev.
>
> Fixes: 9cfcf99e7ed6 ("md: get rdev->mddev with READ_ONCE()")
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> drivers/md/md.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 92ec4be20db8..a021cf4a798d 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3704,7 +3704,7 @@ rdev_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
>
> if (!entry->show)
> return -EIO;
> - if (!rdev->mddev)
> + if (!READ_ONCE(rdev->mddev))
> return -ENODEV;
> return entry->show(rdev, page);
> }
LGTM
Reviewed-by: Li Nan <magiclinan@didiglobal.com>
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH] raid10: badblock-aware reshape write error handling
From: caogh @ 2026-06-12 8:05 UTC (permalink / raw)
To: Li Nan; +Cc: linux-kernel, linux-raid, song, xiao, yukuai
In-Reply-To: <tencent_1A4057A928F0CDF70F24A070B03ABFB34A05@qq.com>
Nan, thanks for the review.
You're right that the current code chains several mechanisms (badblock
recording, WantReplacement, md_error) together, which hurts readability.
I'll refactor in the next version to make the logic clearer.
However, regarding your suggested logic:
> The correct logic might be:
> 1. If there is a replacement, fail directly with md_error
> 2. If not, mark the badblock and set WantReplacement
I have some questions.
If a replacement device hits a write error, calling md_error directly
will mark it Faulty and kick it out of the array. But it may just be a
few bad sectors — recording the badblock and continuing the reshape is
sufficient. Is it really necessary to call md_error directly in this case?
The current patch does:
1、Try to record the badblock first
2、On success:
a) member device: set WantReplacement to trigger a replacement
b) replacement device: skip WantReplacement to avoid a
replacement loop
3、On failure: rdev_set_badblocks has already called md_error
internally, no further action needed
Does this approach seem reasonable?
在 2026/6/12 11:42, Li Nan 写道:
> On Mon Jun 1, 2026 at 1:46 PM CST, ghuicao wrote:
>> From: Cao Guanghui<caoguanghui@kylinos.cn>
>>
>> Replace the FIXME in end_reshape_write(). Instead of failing the device
>> immediately on write errors during reshape, attempt to record badblocks
>> using new_data_offset with is_new=1.
>>
>> rdev_set_badblocks() returns true on success. On failure (e.g., badblocks
>> table full), it has already called md_error() internally to degrade the
>> device. Queue WantReplacement for member devices regardless of badblock
>> recording success, but skip this for replacement devices to avoid
>> replacement loops.
>>
>> On successful write, clear stale badblock records at the new location
>> since data has migrated.
>>
>> Signed-off-by: Cao Guanghui<caoguanghui@kylinos.cn>
>> ---
>> drivers/md/raid10.c | 27 ++++++++++++++++++++++++---
>> 1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 4901ebe45c87..08d58a1c680e 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -4991,9 +4991,30 @@ static void end_reshape_write(struct bio *bio)
>> conf->mirrors[d].rdev;
>>
>> if (bio->bi_status) {
>> - /* FIXME should record badblock */
>> - md_error(mddev, rdev);
>> - }
>> + set_bit(WriteErrorSeen, &rdev->flags);
>> +
>> + /* rdev_set_badblocks returns true on success.
>> + * On failure, it has already called md_error() internally.
>> + * Use is_new=1 as reshape writes target the new layout
>> + * (new_data_offset).
>> + */
>> + if (rdev_set_badblocks(rdev, r10_bio->devs[slot].addr,
>> + r10_bio->sectors, 1)) {
>> + /* Queue async replacement for member devices
>> + * For replacement devices, do not trigger WantReplacement
>> + * to avoid circular replacement storms.
>> + */
>> + if (!repl) {
>> + if (!test_and_set_bit(WantReplacement, &rdev->flags))
>> + set_bit(MD_RECOVERY_NEEDED,
>> + &rdev->mddev->recovery);
> The logic here seems a bit odd — several mechanisms are chained together.
>
> The correct logic might be:
> 1. If there is a replacement, fail directly with md_error
> 2. If not, mark the badblock and set WantReplacement
>
>> + }
>> + }
>> + } else {
>> + /* Write succeeded, clear stale badblock records */
>> + rdev_clear_badblocks(rdev, r10_bio->devs[slot].addr,
>> + r10_bio->sectors, 1);
>> + }
>>
>> rdev_dec_pending(rdev, mddev);
>> end_reshape_request(r10_bio);
>>
Thanks, Cao Guanghui
^ permalink raw reply
* AVX-512 Optimization For Linux RAID Showing Up To 41% Improvement
From: Anton Gavriliuk @ 2026-06-12 16:19 UTC (permalink / raw)
To: linux-raid
Hi
https://www.phoronix.com/news/AVX-512-Linux-RAID-Optimization
I'm just want to clarify - will help to improve write performance and
raid56 build/rebuid time on modern hardware - amd zen5 (9005 series)
and pci5.0 nvme ssds ?
Anton
^ permalink raw reply
* [PATCH v2 0/4] md/raid1,raid10: fix write-path reference leaks and clean up error handling
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
Hi,
This series fixes several write-path failure handling issues in raid1 and
raid10 and then follows up with a cleanup of raid1_write_request().
The first two patches fix writes_pending leaks caused by failure paths
that complete bios without reaching the normal write completion path.
The raid1 fix also addresses a barrier reference leak when
wait_blocked_rdev() fails after wait_barrier() succeeds.
The third patch fixes additional writes_pending and barrier reference
leaks in raid10 discard handling.
The final patch simplifies raid1_write_request() error handling.
Patches:
md/raid1: fix writes_pending and barrier reference leaks on write failures
md/raid10: fix writes_pending leak on write request failures
md/raid10: fix writes_pending and barrier reference leaks on discard failures
md/raid1: simplify raid1_write_request() error handling
Changes in v2:
- fix writes_pending leaks in addition to the barrier reference leaks
- add raid10 fixes for analogous write and discard failure paths
- add a follow-up cleanup patch to simplify raid1_write_request()
- simplifies raid1_write_request() error handling.
- Link v1: https://lore.kernel.org/linux-raid/20260611132500.763528-1-abd.masalkhi@gmail.com/
Thanks,
Abd-Alrhman
Abd-Alrhman Masalkhi (4):
md/raid1: fix writes_pending and barrier reference leaks on write
failures
md/raid10: fix writes_pending leak on write request failures
md/raid10: fix writes_pending and barrier reference leaks on discard
failures
md/raid1: simplify raid1_write_request() error handling
drivers/md/raid1.c | 74 ++++++++++++++++++++++++---------------------
drivers/md/raid10.c | 28 ++++++++++++-----
2 files changed, 60 insertions(+), 42 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/4] md/raid1: fix writes_pending and barrier reference leaks on write failures
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi, sashiko-bot
In-Reply-To: <20260613182810.1317258-1-abd.masalkhi@gmail.com>
raid1_make_request() acquires a writes_pending reference with
md_write_start() before calling raid1_write_request(). Several failure
paths in raid1_write_request() complete the bio and return without
reaching the normal write completion path, causing the corresponding
md_write_end() to be skipped.
Make raid1_write_request() return a status indicating whether the write
request was successfully queued. This allows raid1_make_request() to
call md_write_end() when raid1_write_request() fails.
Additionally, if wait_blocked_rdev() fails after wait_barrier()
succeeds, the associated barrier reference is not released.
Call allow_barrier() before returning from that path to keep the barrier
accounting balanced.
Fixes: b1a7ad8b5c4f ("md/raid1: Handle bio_split() errors")
Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
Fixes: 5aa705039c4f ("md: raid1 add nowait support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
Closes: https://sashiko.dev/#/patchset/20260611132500.763528-1-abd.masalkhi@gmail.com?part=1
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- fix writes_pending leaks in addition to the barrier reference leak.
- make raid1_write_request() return whether the write was successfully
queued so raid1_make_request() can release writes_pending references
on failure paths.
- Link v1: https://lore.kernel.org/linux-raid/20260611132500.763528-1-abd.masalkhi@gmail.com/
---
drivers/md/raid1.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index b1ed4cc6ade4..f0e1c7125972 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1501,7 +1501,7 @@ static void raid1_start_write_behind(struct mddev *mddev, struct r1bio *r1_bio,
}
-static void raid1_write_request(struct mddev *mddev, struct bio *bio,
+static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
int max_write_sectors)
{
struct r1conf *conf = mddev->private;
@@ -1512,6 +1512,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
int max_sectors;
bool write_behind = false;
bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
+ sector_t sector = bio->bi_iter.bi_sector;
if (mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1519,7 +1520,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
- return;
+ return false;
}
wait_event_idle(conf->wait_barrier,
!mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1535,12 +1536,13 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
if (!wait_barrier(conf, bio->bi_iter.bi_sector,
bio->bi_opf & REQ_NOWAIT)) {
bio_wouldblock_error(bio);
- return;
+ return false;
}
if (!wait_blocked_rdev(mddev, bio)) {
bio_wouldblock_error(bio);
- return;
+ allow_barrier(conf, sector);
+ return false;
}
r1_bio = alloc_r1bio(mddev, bio);
@@ -1699,7 +1701,8 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
/* In case raid1d snuck in to freeze_array */
wake_up_barrier(conf);
- return;
+ return true;
+
err_handle:
for (k = 0; k < i; k++) {
if (r1_bio->bios[k]) {
@@ -1709,6 +1712,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
}
raid_end_bio_io(r1_bio);
+ return false;
}
static bool raid1_make_request(struct mddev *mddev, struct bio *bio)
@@ -1732,8 +1736,9 @@ static bool raid1_make_request(struct mddev *mddev, struct bio *bio)
if (bio_data_dir(bio) == READ)
raid1_read_request(mddev, bio, sectors, NULL);
else {
- md_write_start(mddev,bio);
- raid1_write_request(mddev, bio, sectors);
+ md_write_start(mddev, bio);
+ if (!raid1_write_request(mddev, bio, sectors))
+ md_write_end(mddev);
}
return true;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/4] md/raid10: fix writes_pending leak on write request failures
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260613182810.1317258-1-abd.masalkhi@gmail.com>
raid10_make_request() acquires a writes_pending reference with
md_write_start() before dispatching write requests. Several failure
paths in raid10_write_request() complete the bio and return without
reaching the normal write completion path, causing the corresponding
md_write_end() to be skipped.
Make raid10_write_request() return a status indicating whether the write
request was successfully queued. This allows raid10_make_request() to
release the writes_pending reference with md_write_end() when a write
request fails.
Fixes: 4cf58d952909 ("md/raid10: Handle bio_split() errors")
Fixes: c9aa889b035f ("md: raid10 add nowait support")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- new patch.
---
drivers/md/raid10.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5bd7698e0a1b..5ad1b0c6207a 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1349,7 +1349,7 @@ static void wait_blocked_dev(struct mddev *mddev, struct r10bio *r10_bio)
}
}
-static void raid10_write_request(struct mddev *mddev, struct bio *bio,
+static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
struct r10bio *r10_bio)
{
struct r10conf *conf = mddev->private;
@@ -1365,7 +1365,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
- return;
+ return false;
}
for (;;) {
prepare_to_wait(&conf->wait_barrier,
@@ -1381,7 +1381,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
sectors = r10_bio->sectors;
if (!regular_request_wait(mddev, conf, bio, sectors)) {
free_r10bio(r10_bio);
- return;
+ return false;
}
if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
@@ -1398,7 +1398,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
if (bio->bi_opf & REQ_NOWAIT) {
allow_barrier(conf);
bio_wouldblock_error(bio);
- return;
+ return false;
}
mddev_add_trace_msg(conf->mddev,
"raid10 wait reshape metadata");
@@ -1514,7 +1514,8 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
raid10_write_one_disk(mddev, r10_bio, bio, true, i);
}
one_write_done(r10_bio);
- return;
+ return true;
+
err_handle:
for (k = 0; k < i; k++) {
int d = r10_bio->devs[k].devnum;
@@ -1532,10 +1533,12 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
}
raid_end_bio_io(r10_bio);
+ return false;
}
-static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
+static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
{
+ bool ret;
struct r10conf *conf = mddev->private;
struct r10bio *r10_bio;
@@ -1551,10 +1554,13 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
conf->geo.raid_disks);
+ ret = true;
if (bio_data_dir(bio) == READ)
raid10_read_request(mddev, bio, r10_bio);
else
- raid10_write_request(mddev, bio, r10_bio);
+ ret = raid10_write_request(mddev, bio, r10_bio);
+
+ return ret;
}
static void raid_end_discard_bio(struct r10bio *r10bio)
@@ -1900,7 +1906,8 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sectors = chunk_sects -
(bio->bi_iter.bi_sector &
(chunk_sects - 1));
- __make_request(mddev, bio, sectors);
+ if (!__make_request(mddev, bio, sectors))
+ md_write_end(mddev);
/* In case raid10d snuck in to freeze_array */
wake_up_barrier(conf);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/4] md/raid10: fix writes_pending and barrier reference leaks on discard failures
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260613182810.1317258-1-abd.masalkhi@gmail.com>
raid10_make_request() acquires a writes_pending reference with
md_write_start() before calling raid10_handle_discard(). Several failure
paths in raid10_handle_discard() complete the bio and return without
releasing the corresponding reference, causing md_write_end() to be
skipped.
Call md_write_end() before returning from these failure paths to keep
writes_pending accounting balanced.
Additionally, discard split allocation failures can occur after
wait_barrier() succeeds. Those paths return without calling
allow_barrier(), leaking the associated barrier reference.
Release the barrier before returning from those paths.
Fixes: c9aa889b035f ("md: raid10 add nowait support")
Fixes: 4cf58d952909 ("md/raid10: Handle bio_split() errors")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- new patch.
---
drivers/md/raid10.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5ad1b0c6207a..aacf160ee9f2 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1639,6 +1639,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
bio_wouldblock_error(bio);
+ md_write_end(mddev);
return 0;
}
@@ -1681,6 +1682,8 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
if (IS_ERR(split)) {
bio->bi_status = errno_to_blk_status(PTR_ERR(split));
bio_endio(bio);
+ md_write_end(mddev);
+ allow_barrier(conf);
return 0;
}
@@ -1698,6 +1701,8 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
if (IS_ERR(split)) {
bio->bi_status = errno_to_blk_status(PTR_ERR(split));
bio_endio(bio);
+ md_write_end(mddev);
+ allow_barrier(conf);
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 4/4] md/raid1: simplify raid1_write_request() error handling
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260613182810.1317258-1-abd.masalkhi@gmail.com>
raid1_write_request() increments rdev->nr_pending before checking the
badblocks and then immediately decrements it again when a device is
skipped. Move the increment until after the checks succeed so the
reference accounting is easier to follow.
Consolidate the failure paths so that each error label releases exactly
the resources acquired up to that point. err_dec_pending drops pending
references and frees the r1bio, while err_allow_barrier handles the
barrier release before returning.
When a REQ_ATOMIC write cannot be satisfied due to a badblock range,
complete the bio with BLK_STS_NOTSUPP rather than reporting an I/O
error, since the operation is unsupported rather than having failed
during I/O.
Rename max_write_sectors to max_sectors and remove the redundant local
copy.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- new patch, depends on patch 1.
---
drivers/md/raid1.c | 59 +++++++++++++++++++++++-----------------------
1 file changed, 30 insertions(+), 29 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index f0e1c7125972..dc0b7b8bc2f8 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1502,29 +1502,29 @@ static void raid1_start_write_behind(struct mddev *mddev, struct r1bio *r1_bio,
}
static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
- int max_write_sectors)
+ int max_sectors)
{
struct r1conf *conf = mddev->private;
struct r1bio *r1_bio;
int i, disks, k;
unsigned long flags;
int first_clone;
- int max_sectors;
bool write_behind = false;
- bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
+ bool nowait = bio->bi_opf & REQ_NOWAIT;
+ bool is_discard = op_is_discard(bio->bi_opf);
sector_t sector = bio->bi_iter.bi_sector;
if (mddev_is_clustered(mddev) &&
- mddev->cluster_ops->area_resyncing(mddev, WRITE,
- bio->bi_iter.bi_sector, bio_end_sector(bio))) {
+ mddev->cluster_ops->area_resyncing(mddev, WRITE, sector,
+ bio_end_sector(bio))) {
- if (bio->bi_opf & REQ_NOWAIT) {
+ if (nowait) {
bio_wouldblock_error(bio);
return false;
}
wait_event_idle(conf->wait_barrier,
!mddev->cluster_ops->area_resyncing(mddev, WRITE,
- bio->bi_iter.bi_sector,
+ sector,
bio_end_sector(bio)));
}
@@ -1533,20 +1533,18 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* thread has put up a bar for new requests.
* Continue immediately if no resync is active currently.
*/
- if (!wait_barrier(conf, bio->bi_iter.bi_sector,
- bio->bi_opf & REQ_NOWAIT)) {
+ if (!wait_barrier(conf, sector, nowait)) {
bio_wouldblock_error(bio);
return false;
}
if (!wait_blocked_rdev(mddev, bio)) {
bio_wouldblock_error(bio);
- allow_barrier(conf, sector);
- return false;
+ goto err_allow_barrier;
}
r1_bio = alloc_r1bio(mddev, bio);
- r1_bio->sectors = max_write_sectors;
+ r1_bio->sectors = max_sectors;
/* first select target devices under rcu_lock and
* inc refcount on their rdev. Record them by setting
@@ -1560,7 +1558,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
*/
disks = conf->raid_disks * 2;
- max_sectors = r1_bio->sectors;
for (i = 0; i < disks; i++) {
struct md_rdev *rdev = conf->mirrors[i].rdev;
@@ -1576,23 +1573,21 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
if (!rdev || test_bit(Faulty, &rdev->flags))
continue;
- atomic_inc(&rdev->nr_pending);
if (test_bit(WriteErrorSeen, &rdev->flags)) {
sector_t first_bad;
sector_t bad_sectors;
int is_bad;
- is_bad = is_badblock(rdev, r1_bio->sector, max_sectors,
+ is_bad = is_badblock(rdev, sector, max_sectors,
&first_bad, &bad_sectors);
- if (is_bad && first_bad <= r1_bio->sector) {
+ if (is_bad && first_bad <= sector) {
/* Cannot write here at all */
- bad_sectors -= (r1_bio->sector - first_bad);
+ bad_sectors -= (sector - first_bad);
if (bad_sectors < max_sectors)
/* mustn't write more than bad_sectors
* to other devices yet
*/
max_sectors = bad_sectors;
- rdev_dec_pending(rdev, mddev);
continue;
}
if (is_bad) {
@@ -1606,15 +1601,18 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* the benefit.
*/
if (bio->bi_opf & REQ_ATOMIC) {
- rdev_dec_pending(rdev, mddev);
- goto err_handle;
+ bio->bi_status = BLK_STS_NOTSUPP;
+ bio_endio(bio);
+ goto err_dec_pending;
}
- good_sectors = first_bad - r1_bio->sector;
+ good_sectors = first_bad - sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
}
}
+
+ atomic_inc(&rdev->nr_pending);
r1_bio->bios[i] = bio;
}
@@ -1630,10 +1628,8 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
if (max_sectors < bio_sectors(bio)) {
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
- if (!bio) {
- set_bit(R1BIO_Returned, &r1_bio->state);
- goto err_handle;
- }
+ if (!bio)
+ goto err_dec_pending;
r1_bio->master_bio = bio;
r1_bio->sectors = max_sectors;
@@ -1677,7 +1673,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
mbio->bi_opf &= ~REQ_NOWAIT;
r1_bio->bios[i] = mbio;
- mbio->bi_iter.bi_sector = (r1_bio->sector + rdev->data_offset);
+ mbio->bi_iter.bi_sector = sector + rdev->data_offset;
mbio->bi_end_io = raid1_end_write_request;
if (test_bit(FailFast, &rdev->flags) &&
!test_bit(WriteMostly, &rdev->flags) &&
@@ -1686,7 +1682,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
mbio->bi_private = r1_bio;
atomic_inc(&r1_bio->remaining);
- mddev_trace_remap(mddev, mbio, r1_bio->sector);
+ mddev_trace_remap(mddev, mbio, sector);
/* flush_pending_writes() needs access to the rdev so...*/
mbio->bi_bdev = (void *)rdev;
if (!raid1_add_bio_to_plug(mddev, mbio, raid1_unplug, disks)) {
@@ -1701,9 +1697,10 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
/* In case raid1d snuck in to freeze_array */
wake_up_barrier(conf);
+
return true;
-err_handle:
+err_dec_pending:
for (k = 0; k < i; k++) {
if (r1_bio->bios[k]) {
rdev_dec_pending(conf->mirrors[k].rdev, mddev);
@@ -1711,7 +1708,11 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
}
}
- raid_end_bio_io(r1_bio);
+ free_r1bio(r1_bio);
+
+err_allow_barrier:
+ allow_barrier(conf, sector);
+
return false;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 2/4] md/raid10: fix writes_pending leak on write request failures
From: sashiko-bot @ 2026-06-13 18:40 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: linux-raid, yukuai
In-Reply-To: <20260613182810.1317258-3-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `r10_bio` structure allocated from the mempool is leaked when `raid10_write_request()` bails out early due to `REQ_NOWAIT`.
--
commit 3940ccdb4c7c7e19c20f9b5e2e1d12545c2558b5
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid10: fix writes_pending leak on write request failures
raid10_make_request() acquires a writes_pending reference with
md_write_start() before dispatching write requests. Several failure
paths in raid10_write_request() complete the bio and return without
reaching the normal write completion path, causing the corresponding
md_write_end() to be skipped.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index cee5a253a281d..c123a8c76ddcd 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -1365,7 +1365,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> /* Bail out if REQ_NOWAIT is set for the bio */
> if (bio->bi_opf & REQ_NOWAIT) {
> bio_wouldblock_error(bio);
> - return;
> + return false;
[Severity: High]
This isn't a bug introduced by this patch, but does this early return leak
the r10_bio structure?
__make_request() allocates r10_bio using:
r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
It then passes it to raid10_write_request(). When the bio has REQ_NOWAIT set
during a clustered area resync, we complete the bio with an error and return
false without freeing the r10_bio. Could repeatedly triggering this exhaust
the r10bio_pool, causing deadlocks on subsequent operations?
Note that the regular_request_wait() error path right below correctly calls
free_r10bio(r10_bio) before returning.
> }
> for (;;) {
[ ... ]
> @@ -1398,7 +1398,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> if (bio->bi_opf & REQ_NOWAIT) {
> allow_barrier(conf);
> bio_wouldblock_error(bio);
> - return;
> + return false;
[Severity: High]
This is a pre-existing issue, but does this path also leak the r10_bio
mempool allocation when waiting for a reshape metadata update?
Similar to the clustered area resync case above, the r10_bio allocated in
__make_request() does not appear to be freed before returning here.
> }
> mddev_add_trace_msg(conf->mddev,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613182810.1317258-1-abd.masalkhi@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v2 4/4] md/raid1: simplify raid1_write_request() error handling
From: sashiko-bot @ 2026-06-13 18:47 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: linux-raid, yukuai
In-Reply-To: <20260613182810.1317258-5-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Moving `atomic_inc(&rdev->nr_pending)` down opens a race window with `hot_remove_disk`, leading to a NULL pointer dereference during error handling or a Use-After-Free in clustered configurations.
Pre-existing issues:
- [High] A `REQ_ATOMIC` write can be silently split if a bad block overlaps the start of the write, violating atomicity guarantees.
--
commit c334f57d91677f32131f21e50bb32b83136e5a60
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid1: simplify raid1_write_request() error handling
This commit delays the rdev->nr_pending increment until after badblocks
are checked to avoid incrementing and immediately decrementing on skipped
devices. It also consolidates error handling paths and modifies REQ_ATOMIC
failures to complete with BLK_STS_NOTSUPP instead of I/O errors.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 632d72607e11e..86d4f224ffb11 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -1576,23 +1573,21 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
> if (!rdev || test_bit(Faulty, &rdev->flags))
> continue;
>
> - atomic_inc(&rdev->nr_pending);
> if (test_bit(WriteErrorSeen, &rdev->flags)) {
> sector_t first_bad;
> sector_t bad_sectors;
> int is_bad;
>
> - is_bad = is_badblock(rdev, r1_bio->sector, max_sectors,
> + is_bad = is_badblock(rdev, sector, max_sectors,
> &first_bad, &bad_sectors);
> - if (is_bad && first_bad <= r1_bio->sector) {
> + if (is_bad && first_bad <= sector) {
> /* Cannot write here at all */
> - bad_sectors -= (r1_bio->sector - first_bad);
> + bad_sectors -= (sector - first_bad);
> if (bad_sectors < max_sectors)
> /* mustn't write more than bad_sectors
> * to other devices yet
> */
> max_sectors = bad_sectors;
> - rdev_dec_pending(rdev, mddev);
> continue;
[Severity: High]
This is a pre-existing issue, but does this logic skip the rejection of
REQ_ATOMIC bios if a bad block overlaps the start of the write?
When is_badblock() finds a bad block at or before the write sector, the
code shrinks max_sectors to force a split at the boundary and then calls
continue. Because of the continue, the REQ_ATOMIC bio skips the rejection
logic below.
Could this silently split an atomic write into smaller pieces and submit
them to the healthy mirrors via bio_submit_split_bioset(), violating
block-level atomicity guarantees?
> }
> if (is_bad) {
[ ... ]
> if (bio->bi_opf & REQ_ATOMIC) {
> - rdev_dec_pending(rdev, mddev);
> - goto err_handle;
> + bio->bi_status = BLK_STS_NOTSUPP;
> + bio_endio(bio);
> + goto err_dec_pending;
> }
>
> - good_sectors = first_bad - r1_bio->sector;
> + good_sectors = first_bad - sector;
> if (good_sectors < max_sectors)
> max_sectors = good_sectors;
> }
> }
> +
> + atomic_inc(&rdev->nr_pending);
[Severity: Critical]
Does moving atomic_inc(&rdev->nr_pending) down open a race window with
hot_remove_disk()?
Because nr_pending acts as an existence lock that prevents the device from
being removed from the array configuration by raid1_remove_conf(),
incrementing it after the is_badblock() and test_bit(WriteErrorSeen, ...)
checks means md_check_recovery() could evaluate rdev_removeable() while
nr_pending is 0 and call hot_remove_disk().
If raid1_remove_conf() unlinks the device by setting
conf->mirrors[i].rdev = NULL, when this thread resumes and the I/O
eventually fails, would the error handling path (handle_write_finished()
calling narrow_write_error()) read conf->mirrors[i].rdev and dereference a
NULL pointer?
Additionally, in clustered configurations, if the unlinked rdev is
immediately freed by md_kick_rdev_from_array(), could this lead to a
use-after-free directly in raid1_write_request() when evaluating
test_bit(WriteErrorSeen, &rdev->flags) or is_badblock()?
> r1_bio->bios[i] = bio;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613182810.1317258-1-abd.masalkhi@gmail.com?part=4
^ permalink raw reply
* [PATCH v2] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Eric Biggers @ 2026-06-14 1:03 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Christoph Hellwig, linux-crypto, x86, Eric Biggers, David Laight,
linux-raid
Add an implementation of xor_gen() using AVX-512.
It uses 512-bit vectors, i.e. ZMM registers. It also uses the
vpternlogq instruction to do three-input XORs when applicable.
It's enabled on x86_64 CPUs that have AVX512F && !PREFER_YMM. In
practice that means:
- AMD Zen 4 and later (client and server)
- Intel Sapphire Rapids and later (server)
- Intel Rocket Lake (client)
- Intel Nova Lake and later (client)
The !PREFER_YMM condition excludes the older AVX-512 implementations in
Intel Skylake Server and Intel Ice Lake. They could run this code, but
they're known to have overly-eager downclocking when ZMM registers are
used. This is the same policy that the crypto and CRC code uses.
Benchmark on AMD Ryzen 9 9950X (Zen 5):
src_cnt avx avx512 Improvement
======= ========== ========== ===========
1 56353 MB/s 75388 MB/s 33%
2 54274 MB/s 68409 MB/s 26%
3 44649 MB/s 64042 MB/s 43%
4 41315 MB/s 55002 MB/s 33%
Note: for now I omitted the cpu_has_xfeatures() check that the AVX-512
optimized crypto and CRC code does, since it's not implemented on
User-Mode Linux and it's never been present in the RAID6 code either.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
Changed in v2:
- Fixed build on UML
- Reworked the implementation
lib/raid/xor/Makefile | 2 +-
lib/raid/xor/x86/xor-avx512.c | 121 ++++++++++++++++++++++++++++++++++
lib/raid/xor/x86/xor_arch.h | 26 ++++----
3 files changed, 137 insertions(+), 12 deletions(-)
create mode 100644 lib/raid/xor/x86/xor-avx512.c
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index 4d633dfd5b90..4af945861a51 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -26,11 +26,11 @@ xor-$(CONFIG_ALTIVEC) += powerpc/xor_vmx.o powerpc/xor_vmx_glue.o
xor-$(CONFIG_RISCV_ISA_V) += riscv/xor.o riscv/xor-glue.o
xor-$(CONFIG_SPARC32) += sparc/xor-sparc32.o
xor-$(CONFIG_SPARC64) += sparc/xor-sparc64.o sparc/xor-sparc64-glue.o
xor-$(CONFIG_S390) += s390/xor.o
xor-$(CONFIG_X86_32) += x86/xor-avx.o x86/xor-sse.o x86/xor-mmx.o
-xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o
+xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o x86/xor-avx512.o
obj-y += tests/
CFLAGS_arm/xor-neon.o += $(CC_FLAGS_FPU)
CFLAGS_REMOVE_arm/xor-neon.o += $(CC_FLAGS_NO_FPU)
diff --git a/lib/raid/xor/x86/xor-avx512.c b/lib/raid/xor/x86/xor-avx512.c
new file mode 100644
index 000000000000..87b981d74c90
--- /dev/null
+++ b/lib/raid/xor/x86/xor-avx512.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AVX-512 optimized implementation of xor_gen()
+ *
+ * Copyright 2026 Google LLC
+ */
+
+#include <linux/types.h>
+#include <asm/fpu/api.h>
+#include "xor_impl.h"
+#include "xor_arch.h"
+
+/*
+ * Implementation notes:
+ *
+ * Unrolling by the number of buffers (2-5) is very important.
+ *
+ * Unrolling by length is less important, especially when using register-indexed
+ * addressing with negative indices from the end of the buffers. That approach
+ * results in just two loop control instructions being needed per iteration,
+ * regardless of the number of buffers.
+ *
+ * In fact, benchmarks showed that the 2 and 3 buffer cases require only 2x
+ * unrolling by length, while the 4 and 5 buffer cases don't require any
+ * unrolling by length. Benchmarks also showed that the register-indexed
+ * addressing isn't a bottleneck either; i.e., we can't do any better by
+ * incrementing the pointers as we go along, even with more unrolling.
+ */
+
+static void xor_avx512_2(long bytes, u8 *p0, const u8 *p1)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
+ "vmovdqa64 64(%0,%1), %%zmm1\n"
+ "vpxorq (%0,%2), %%zmm0, %%zmm0\n"
+ "vpxorq 64(%0,%2), %%zmm1, %%zmm1\n"
+ "vmovdqa64 %%zmm0, (%0,%1)\n"
+ "vmovdqa64 %%zmm1, 64(%0,%1)\n"
+ "add $128, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p0 + bytes), "r"(p1 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_3(long bytes, u8 *p0, const u8 *p1, const u8 *p2)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
+ "vmovdqa64 64(%0,%1), %%zmm1\n"
+ "vmovdqa64 (%0,%2), %%zmm2\n"
+ "vmovdqa64 64(%0,%2), %%zmm3\n"
+ "vpternlogq $0x96, (%0,%3), %%zmm2, %%zmm0\n"
+ "vpternlogq $0x96, 64(%0,%3), %%zmm3, %%zmm1\n"
+ "vmovdqa64 %%zmm0, (%0,%1)\n"
+ "vmovdqa64 %%zmm1, 64(%0,%1)\n"
+ "add $128, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_4(long bytes, u8 *p0, const u8 *p1, const u8 *p2,
+ const u8 *p3)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
+ "vmovdqa64 (%0,%2), %%zmm1\n"
+ "vpxorq (%0,%3), %%zmm0, %%zmm0\n"
+ "vpternlogq $0x96, (%0,%4), %%zmm1, %%zmm0\n"
+ "vmovdqa64 %%zmm0, (%0,%1)\n"
+ "add $64, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes),
+ "r"(p3 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_5(long bytes, u8 *p0, const u8 *p1, const u8 *p2,
+ const u8 *p3, const u8 *p4)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
+ "vmovdqa64 (%0,%2), %%zmm1\n"
+ "vpternlogq $0x96, (%0,%3), %%zmm1, %%zmm0\n"
+ "vmovdqa64 (%0,%4), %%zmm1\n"
+ "vpternlogq $0x96, (%0,%5), %%zmm1, %%zmm0\n"
+ "vmovdqa64 %%zmm0, (%0,%1)\n"
+ "add $64, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes),
+ "r"(p3 + bytes), "r"(p4 + bytes)
+ : "memory", "cc");
+}
+
+DO_XOR_BLOCKS(avx512_inner, xor_avx512_2, xor_avx512_3, xor_avx512_4,
+ xor_avx512_5);
+
+/*
+ * Preconditions: bytes is a nonzero multiple of 512, and all buffers are
+ * 64-byte aligned.
+ */
+static void xor_gen_avx512(void *dest, void **srcs, unsigned int src_cnt,
+ unsigned int bytes)
+{
+ kernel_fpu_begin();
+ xor_gen_avx512_inner(dest, srcs, src_cnt, bytes);
+ kernel_fpu_end();
+}
+
+struct xor_block_template xor_block_avx512 = {
+ .name = "avx512",
+ .xor_gen = xor_gen_avx512,
+};
diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
index 99fe85a213c6..b5d49376fc97 100644
--- a/lib/raid/xor/x86/xor_arch.h
+++ b/lib/raid/xor/x86/xor_arch.h
@@ -4,26 +4,30 @@
extern struct xor_block_template xor_block_pII_mmx;
extern struct xor_block_template xor_block_p5_mmx;
extern struct xor_block_template xor_block_sse;
extern struct xor_block_template xor_block_sse_pf64;
extern struct xor_block_template xor_block_avx;
+extern struct xor_block_template xor_block_avx512;
-/*
- * When SSE is available, use it as it can write around L2. We may also be able
- * to load into the L1 only depending on how the cpu deals with a load to a line
- * that is being prefetched.
- *
- * When AVX2 is available, force using it as it is better by all measures.
- *
- * 32-bit without MMX can fall back to the generic routines.
- */
static __always_inline void __init arch_xor_init(void)
{
- if (boot_cpu_has(X86_FEATURE_AVX) &&
- boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+ if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
+ boot_cpu_has(X86_FEATURE_OSXSAVE) &&
+ !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
+ /* AVX-512 will be the best; no need to try others. */
+ /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
+ xor_force(&xor_block_avx512);
+ } else if (boot_cpu_has(X86_FEATURE_AVX) &&
+ boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+ /* AVX will be the best; no need to try others. */
xor_force(&xor_block_avx);
} else if (IS_ENABLED(CONFIG_X86_64) || boot_cpu_has(X86_FEATURE_XMM)) {
+ /*
+ * When SSE is available, use it as it can write around L2. We
+ * may also be able to load into the L1 only depending on how
+ * the cpu deals with a load to a line that is being prefetched.
+ */
xor_register(&xor_block_sse);
xor_register(&xor_block_sse_pf64);
} else if (boot_cpu_has(X86_FEATURE_MMX)) {
xor_register(&xor_block_pII_mmx);
xor_register(&xor_block_p5_mmx);
base-commit: 2b07ea76fd28989bde5993532d7a943a6f90e246
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: sashiko-bot @ 2026-06-14 1:23 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-raid, yukuai
In-Reply-To: <20260614010357.69416-1-ebiggers@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The AVX-512 XOR implementation relies solely on CPUID hardware checks (`boot_cpu_has(X86_FEATURE_AVX512F)`) and skips verifying if the OS actually enabled AVX-512 state management in the XCR0 register, risking a #UD exception.
--
commit d9094ea6909535dc1fa7dcdb6976401cdf1f90e8
Author: Eric Biggers <ebiggers@kernel.org>
lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
This commit adds an AVX-512 optimized implementation of xor_gen() using
512-bit vectors and the vpternlogq instruction. The optimization is enabled
on x86_64 CPUs with AVX512F support that do not have overly-eager
downclocking (!PREFER_YMM). The author intentionally omitted the
cpu_has_xfeatures() check to avoid build issues on User-Mode Linux.
> diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
> index 99fe85a213c66..b5d49376fc974 100644
> --- a/lib/raid/xor/x86/xor_arch.h
> +++ b/lib/raid/xor/x86/xor_arch.h
[ ... ]
> static __always_inline void __init arch_xor_init(void)
> {
> - if (boot_cpu_has(X86_FEATURE_AVX) &&
> - boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE) &&
> + !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
[Severity: High]
Could omitting the cpu_has_xfeatures() check in arch_xor_init() result in a
#UD exception?
While CR4.OSXSAVE indicates that XCR0 is enabled, it does not guarantee that
the OS or hypervisor actually enabled the AVX-512 state components (OPMASK,
ZMM_Hi256, Hi16_ZMM) in XCR0.
If a hypervisor advertises AVX-512 CPUID bits but masks out the corresponding
XCR0 bits, executing EVEX-encoded AVX-512 instructions could trigger an
Invalid Opcode exception and cause a kernel panic.
> + /* AVX-512 will be the best; no need to try others. */
> + /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
> + xor_force(&xor_block_avx512);
> + } else if (boot_cpu_has(X86_FEATURE_AVX) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + /* AVX will be the best; no need to try others. */
> xor_force(&xor_block_avx);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614010357.69416-1-ebiggers@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH v2] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: David Laight @ 2026-06-14 10:16 UTC (permalink / raw)
To: Eric Biggers
Cc: Andrew Morton, linux-kernel, Christoph Hellwig, linux-crypto, x86,
linux-raid
In-Reply-To: <20260614010357.69416-1-ebiggers@kernel.org>
On Sat, 13 Jun 2026 18:03:57 -0700
Eric Biggers <ebiggers@kernel.org> wrote:
> Add an implementation of xor_gen() using AVX-512.
>
> It uses 512-bit vectors, i.e. ZMM registers. It also uses the
> vpternlogq instruction to do three-input XORs when applicable.
>
> It's enabled on x86_64 CPUs that have AVX512F && !PREFER_YMM. In
> practice that means:
>
> - AMD Zen 4 and later (client and server)
Doesn't zen4 only have a 256bit bus between the cpu and cache?
So avx512 reads take two clocks.
Since this is memory limited it is unlikely to run faster than the
avx256 version.
OTOH if it doesn't cause down-clocking as well then it won't be slower.
> - Intel Sapphire Rapids and later (server)
> - Intel Rocket Lake (client)
> - Intel Nova Lake and later (client)
>
> The !PREFER_YMM condition excludes the older AVX-512 implementations in
> Intel Skylake Server and Intel Ice Lake. They could run this code, but
> they're known to have overly-eager downclocking when ZMM registers are
> used. This is the same policy that the crypto and CRC code uses.
>
> Benchmark on AMD Ryzen 9 9950X (Zen 5):
>
> src_cnt avx avx512 Improvement
> ======= ========== ========== ===========
> 1 56353 MB/s 75388 MB/s 33%
> 2 54274 MB/s 68409 MB/s 26%
> 3 44649 MB/s 64042 MB/s 43%
> 4 41315 MB/s 55002 MB/s 33%
>
> Note: for now I omitted the cpu_has_xfeatures() check that the AVX-512
> optimized crypto and CRC code does, since it's not implemented on
> User-Mode Linux and it's never been present in the RAID6 code either.
>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Since I suggested it :-)
Reviewed-By: David Laight <david.laight.linux@gmail.com>
Some 'not very important' comments:
I did wonder whether moving the loop into the asm() would help.
gcc has a nasty habit of pessimising loops when you try to be clever.
It is certainly safer for tight loops like these.
That does have the side effect of making p0 be %1 which doesn't improve
readability. Either used named parameters or possibly just change p0 to p1 (etc)
so they match.
The code should be limited by the memory reads, so the 3-argument xor and
the interleave of the unroll may make no difference.
Some cpu do have constraints on the cache alignment in order to do two
reads per clock, but I've forgotten them and they got better before AVX-512.
If that were affecting this code (on the tested cpu) then I'd expect the
interleaved unroll would improve the _4 and -5 functions.
So it probably doesn't affect this code.
Using the same loop for the avx-256 and sse (and even smaller) functions could
well generate code that runs 'pretty much as fast as possible' on older cpu.
Intel cpu (going back to Sandy bridge) are likely to execute the loop in the
same number of clocks - but clearly copying half or a quarter of the data.
But I've no experience of zen1.
Might be worth doing for avx-256, does any care about anything older :-)
David
> ---
>
> Changed in v2:
> - Fixed build on UML
> - Reworked the implementation
>
> lib/raid/xor/Makefile | 2 +-
> lib/raid/xor/x86/xor-avx512.c | 121 ++++++++++++++++++++++++++++++++++
> lib/raid/xor/x86/xor_arch.h | 26 ++++----
> 3 files changed, 137 insertions(+), 12 deletions(-)
> create mode 100644 lib/raid/xor/x86/xor-avx512.c
>
> diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
> index 4d633dfd5b90..4af945861a51 100644
> --- a/lib/raid/xor/Makefile
> +++ b/lib/raid/xor/Makefile
> @@ -26,11 +26,11 @@ xor-$(CONFIG_ALTIVEC) += powerpc/xor_vmx.o powerpc/xor_vmx_glue.o
> xor-$(CONFIG_RISCV_ISA_V) += riscv/xor.o riscv/xor-glue.o
> xor-$(CONFIG_SPARC32) += sparc/xor-sparc32.o
> xor-$(CONFIG_SPARC64) += sparc/xor-sparc64.o sparc/xor-sparc64-glue.o
> xor-$(CONFIG_S390) += s390/xor.o
> xor-$(CONFIG_X86_32) += x86/xor-avx.o x86/xor-sse.o x86/xor-mmx.o
> -xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o
> +xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o x86/xor-avx512.o
> obj-y += tests/
>
> CFLAGS_arm/xor-neon.o += $(CC_FLAGS_FPU)
> CFLAGS_REMOVE_arm/xor-neon.o += $(CC_FLAGS_NO_FPU)
>
> diff --git a/lib/raid/xor/x86/xor-avx512.c b/lib/raid/xor/x86/xor-avx512.c
> new file mode 100644
> index 000000000000..87b981d74c90
> --- /dev/null
> +++ b/lib/raid/xor/x86/xor-avx512.c
> @@ -0,0 +1,121 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AVX-512 optimized implementation of xor_gen()
> + *
> + * Copyright 2026 Google LLC
> + */
> +
> +#include <linux/types.h>
> +#include <asm/fpu/api.h>
> +#include "xor_impl.h"
> +#include "xor_arch.h"
> +
> +/*
> + * Implementation notes:
> + *
> + * Unrolling by the number of buffers (2-5) is very important.
> + *
> + * Unrolling by length is less important, especially when using register-indexed
> + * addressing with negative indices from the end of the buffers. That approach
> + * results in just two loop control instructions being needed per iteration,
> + * regardless of the number of buffers.
> + *
> + * In fact, benchmarks showed that the 2 and 3 buffer cases require only 2x
> + * unrolling by length, while the 4 and 5 buffer cases don't require any
> + * unrolling by length. Benchmarks also showed that the register-indexed
> + * addressing isn't a bottleneck either; i.e., we can't do any better by
> + * incrementing the pointers as we go along, even with more unrolling.
> + */
> +
> +static void xor_avx512_2(long bytes, u8 *p0, const u8 *p1)
> +{
> + long i = -bytes;
> +
> + asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
> + "vmovdqa64 64(%0,%1), %%zmm1\n"
> + "vpxorq (%0,%2), %%zmm0, %%zmm0\n"
> + "vpxorq 64(%0,%2), %%zmm1, %%zmm1\n"
> + "vmovdqa64 %%zmm0, (%0,%1)\n"
> + "vmovdqa64 %%zmm1, 64(%0,%1)\n"
> + "add $128, %0\n"
> + "jnz 1b\n"
> + : "+&r"(i)
> + : "r"(p0 + bytes), "r"(p1 + bytes)
> + : "memory", "cc");
> +}
> +
> +static void xor_avx512_3(long bytes, u8 *p0, const u8 *p1, const u8 *p2)
> +{
> + long i = -bytes;
> +
> + asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
> + "vmovdqa64 64(%0,%1), %%zmm1\n"
> + "vmovdqa64 (%0,%2), %%zmm2\n"
> + "vmovdqa64 64(%0,%2), %%zmm3\n"
> + "vpternlogq $0x96, (%0,%3), %%zmm2, %%zmm0\n"
> + "vpternlogq $0x96, 64(%0,%3), %%zmm3, %%zmm1\n"
> + "vmovdqa64 %%zmm0, (%0,%1)\n"
> + "vmovdqa64 %%zmm1, 64(%0,%1)\n"
> + "add $128, %0\n"
> + "jnz 1b\n"
> + : "+&r"(i)
> + : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes)
> + : "memory", "cc");
> +}
> +
> +static void xor_avx512_4(long bytes, u8 *p0, const u8 *p1, const u8 *p2,
> + const u8 *p3)
> +{
> + long i = -bytes;
> +
> + asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
> + "vmovdqa64 (%0,%2), %%zmm1\n"
> + "vpxorq (%0,%3), %%zmm0, %%zmm0\n"
> + "vpternlogq $0x96, (%0,%4), %%zmm1, %%zmm0\n"
> + "vmovdqa64 %%zmm0, (%0,%1)\n"
> + "add $64, %0\n"
> + "jnz 1b\n"
> + : "+&r"(i)
> + : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes),
> + "r"(p3 + bytes)
> + : "memory", "cc");
> +}
> +
> +static void xor_avx512_5(long bytes, u8 *p0, const u8 *p1, const u8 *p2,
> + const u8 *p3, const u8 *p4)
> +{
> + long i = -bytes;
> +
> + asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
> + "vmovdqa64 (%0,%2), %%zmm1\n"
> + "vpternlogq $0x96, (%0,%3), %%zmm1, %%zmm0\n"
> + "vmovdqa64 (%0,%4), %%zmm1\n"
> + "vpternlogq $0x96, (%0,%5), %%zmm1, %%zmm0\n"
> + "vmovdqa64 %%zmm0, (%0,%1)\n"
> + "add $64, %0\n"
> + "jnz 1b\n"
> + : "+&r"(i)
> + : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes),
> + "r"(p3 + bytes), "r"(p4 + bytes)
> + : "memory", "cc");
> +}
> +
> +DO_XOR_BLOCKS(avx512_inner, xor_avx512_2, xor_avx512_3, xor_avx512_4,
> + xor_avx512_5);
> +
> +/*
> + * Preconditions: bytes is a nonzero multiple of 512, and all buffers are
> + * 64-byte aligned.
> + */
> +static void xor_gen_avx512(void *dest, void **srcs, unsigned int src_cnt,
> + unsigned int bytes)
> +{
> + kernel_fpu_begin();
> + xor_gen_avx512_inner(dest, srcs, src_cnt, bytes);
> + kernel_fpu_end();
> +}
> +
> +struct xor_block_template xor_block_avx512 = {
> + .name = "avx512",
> + .xor_gen = xor_gen_avx512,
> +};
> diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
> index 99fe85a213c6..b5d49376fc97 100644
> --- a/lib/raid/xor/x86/xor_arch.h
> +++ b/lib/raid/xor/x86/xor_arch.h
> @@ -4,26 +4,30 @@
> extern struct xor_block_template xor_block_pII_mmx;
> extern struct xor_block_template xor_block_p5_mmx;
> extern struct xor_block_template xor_block_sse;
> extern struct xor_block_template xor_block_sse_pf64;
> extern struct xor_block_template xor_block_avx;
> +extern struct xor_block_template xor_block_avx512;
>
> -/*
> - * When SSE is available, use it as it can write around L2. We may also be able
> - * to load into the L1 only depending on how the cpu deals with a load to a line
> - * that is being prefetched.
> - *
> - * When AVX2 is available, force using it as it is better by all measures.
> - *
> - * 32-bit without MMX can fall back to the generic routines.
> - */
> static __always_inline void __init arch_xor_init(void)
> {
> - if (boot_cpu_has(X86_FEATURE_AVX) &&
> - boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE) &&
> + !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
> + /* AVX-512 will be the best; no need to try others. */
> + /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
> + xor_force(&xor_block_avx512);
> + } else if (boot_cpu_has(X86_FEATURE_AVX) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + /* AVX will be the best; no need to try others. */
> xor_force(&xor_block_avx);
> } else if (IS_ENABLED(CONFIG_X86_64) || boot_cpu_has(X86_FEATURE_XMM)) {
> + /*
> + * When SSE is available, use it as it can write around L2. We
> + * may also be able to load into the L1 only depending on how
> + * the cpu deals with a load to a line that is being prefetched.
> + */
> xor_register(&xor_block_sse);
> xor_register(&xor_block_sse_pf64);
> } else if (boot_cpu_has(X86_FEATURE_MMX)) {
> xor_register(&xor_block_pII_mmx);
> xor_register(&xor_block_p5_mmx);
>
> base-commit: 2b07ea76fd28989bde5993532d7a943a6f90e246
^ permalink raw reply
* Re: [PATCH v2 4/4] md/raid1: simplify raid1_write_request() error handling
From: Abd-Alrhman Masalkhi @ 2026-06-15 8:11 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-raid, yukuai
In-Reply-To: <20260613184757.57BC51F000E9@smtp.kernel.org>
Hi Kuai,
On Sat, Jun 13, 2026 at 18:47 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Critical] Moving `atomic_inc(&rdev->nr_pending)` down opens a race window with `hot_remove_disk`, leading to a NULL pointer dereference during error handling or a Use-After-Free in clustered configurations.
>
I don't think this can happen. the array must be suspended before any
disk can be removed.
> Pre-existing issues:
> - [High] A `REQ_ATOMIC` write can be silently split if a bad block overlaps the start of the write, violating atomicity guarantees.
I don't think this is possible. bio_submit_split_bioset() will fail
rather than split the bio.
> --
>
> commit c334f57d91677f32131f21e50bb32b83136e5a60
> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>
> md/raid1: simplify raid1_write_request() error handling
>
> This commit delays the rdev->nr_pending increment until after badblocks
> are checked to avoid incrementing and immediately decrementing on skipped
> devices. It also consolidates error handling paths and modifies REQ_ATOMIC
> failures to complete with BLK_STS_NOTSUPP instead of I/O errors.
>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 632d72607e11e..86d4f224ffb11 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
> [ ... ]
>> @@ -1576,23 +1573,21 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>> if (!rdev || test_bit(Faulty, &rdev->flags))
>> continue;
>>
>> - atomic_inc(&rdev->nr_pending);
>> if (test_bit(WriteErrorSeen, &rdev->flags)) {
>> sector_t first_bad;
>> sector_t bad_sectors;
>> int is_bad;
>>
>> - is_bad = is_badblock(rdev, r1_bio->sector, max_sectors,
>> + is_bad = is_badblock(rdev, sector, max_sectors,
>> &first_bad, &bad_sectors);
>> - if (is_bad && first_bad <= r1_bio->sector) {
>> + if (is_bad && first_bad <= sector) {
>> /* Cannot write here at all */
>> - bad_sectors -= (r1_bio->sector - first_bad);
>> + bad_sectors -= (sector - first_bad);
>> if (bad_sectors < max_sectors)
>> /* mustn't write more than bad_sectors
>> * to other devices yet
>> */
>> max_sectors = bad_sectors;
>> - rdev_dec_pending(rdev, mddev);
>> continue;
>
> [Severity: High]
> This is a pre-existing issue, but does this logic skip the rejection of
> REQ_ATOMIC bios if a bad block overlaps the start of the write?
>
> When is_badblock() finds a bad block at or before the write sector, the
> code shrinks max_sectors to force a split at the boundary and then calls
> continue. Because of the continue, the REQ_ATOMIC bio skips the rejection
> logic below.
>
> Could this silently split an atomic write into smaller pieces and submit
> them to the healthy mirrors via bio_submit_split_bioset(), violating
> block-level atomicity guarantees?
>
>> }
>> if (is_bad) {
> [ ... ]
>> if (bio->bi_opf & REQ_ATOMIC) {
>> - rdev_dec_pending(rdev, mddev);
>> - goto err_handle;
>> + bio->bi_status = BLK_STS_NOTSUPP;
>> + bio_endio(bio);
I misunderstood the comment. it should call bio_io_error(bio) instead.
I'll fix this in v3.
>> + goto err_dec_pending;
>> }
>>
>> - good_sectors = first_bad - r1_bio->sector;
>> + good_sectors = first_bad - sector;
>> if (good_sectors < max_sectors)
>> max_sectors = good_sectors;
>> }
>> }
>> +
>> + atomic_inc(&rdev->nr_pending);
>
> [Severity: Critical]
> Does moving atomic_inc(&rdev->nr_pending) down open a race window with
> hot_remove_disk()?
>
> Because nr_pending acts as an existence lock that prevents the device from
> being removed from the array configuration by raid1_remove_conf(),
> incrementing it after the is_badblock() and test_bit(WriteErrorSeen, ...)
> checks means md_check_recovery() could evaluate rdev_removeable() while
> nr_pending is 0 and call hot_remove_disk().
>
> If raid1_remove_conf() unlinks the device by setting
> conf->mirrors[i].rdev = NULL, when this thread resumes and the I/O
> eventually fails, would the error handling path (handle_write_finished()
> calling narrow_write_error()) read conf->mirrors[i].rdev and dereference a
> NULL pointer?
>
> Additionally, in clustered configurations, if the unlinked rdev is
> immediately freed by md_kick_rdev_from_array(), could this lead to a
> use-after-free directly in raid1_write_request() when evaluating
> test_bit(WriteErrorSeen, &rdev->flags) or is_badblock()?
>
>> r1_bio->bios[i] = bio;
>> }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260613182810.1317258-1-abd.masalkhi@gmail.com?part=4
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md/md-llbitmap: track bitmap sync_size explicitly
From: Su Yue @ 2026-06-15 10:48 UTC (permalink / raw)
To: Yu Kuai; +Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260605091527.2463539-5-yukuai@kernel.org>
On Fri 05 Jun 2026 at 17:15, Yu Kuai <yukuai@kernel.org> wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> Track llbitmap's own sync_size instead of always using
> mddev->resync_max_sectors directly.
>
> This is the minimal bookkeeping needed before llbitmap can track
> old
> and new reshape geometry independently.
>
> Signed-off-by: Yu Kuai <yukuai@fygo.io>
>
Reviewed-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md-llbitmap.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index bcf34f0c9af6..ecf3ed712315 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -285,10 +285,12 @@ struct llbitmap {
> unsigned long chunkshift;
> /* size of one chunk in sector */
> unsigned long chunksize;
> /* total number of chunks */
> unsigned long chunks;
> + /* total number of sectors tracked by current bitmap geometry
> */
> + sector_t sync_size;
> unsigned long last_end_sync;
> /*
> * time in seconds that dirty bits will be cleared if the page
> is not
> * accessed.
> */
> @@ -916,10 +918,11 @@ static int llbitmap_init(struct llbitmap
> *llbitmap)
>
> llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
> llbitmap->chunkshift = ffz(~chunksize);
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = chunks;
> + llbitmap->sync_size = blocks;
> mddev->bitmap_info.daemon_sleep = DEFAULT_DAEMON_SLEEP;
>
> ret = llbitmap_cache_pages(llbitmap);
> if (ret)
> return ret;
> @@ -936,10 +939,11 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> {
> struct mddev *mddev = llbitmap->mddev;
> unsigned long daemon_sleep;
> unsigned long chunksize;
> unsigned long events;
> + sector_t sync_size;
> struct page *sb_page;
> bitmap_super_t *sb;
> int ret = -EINVAL;
>
> if (!mddev->bitmap_info.offset) {
> @@ -985,10 +989,13 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> if (test_and_clear_bit(BITMAP_FIRST_USE, &llbitmap->flags)) {
> ret = llbitmap_init(llbitmap);
> goto out_put_page;
> }
>
> + sync_size = le64_to_cpu(sb->sync_size);
> + if (!sync_size)
> + sync_size = mddev->resync_max_sectors;
> chunksize = le32_to_cpu(sb->chunksize);
> if (!is_power_of_2(chunksize)) {
> pr_err("md/llbitmap: %s: chunksize not a power of 2",
> mdname(mddev));
> goto out_put_page;
> @@ -1020,12 +1027,13 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> mddev->bitmap_info.chunksize = chunksize;
> mddev->bitmap_info.daemon_sleep = daemon_sleep;
>
> llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
> llbitmap->chunksize = chunksize;
> - llbitmap->chunks =
> DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors, chunksize);
> + llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size,
> chunksize);
> llbitmap->chunkshift = ffz(~chunksize);
> + llbitmap->sync_size = sync_size;
> ret = llbitmap_cache_pages(llbitmap);
>
> out_put_page:
> __free_page(sb_page);
> kunmap_local(sb);
> @@ -1151,10 +1159,11 @@ static int llbitmap_resize(struct mddev
> *mddev, sector_t blocks, int chunksize)
> }
>
> llbitmap->chunkshift = ffz(~chunksize);
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = chunks;
> + llbitmap->sync_size = blocks;
>
> return 0;
> }
>
> static int llbitmap_load(struct mddev *mddev)
> @@ -1524,11 +1533,11 @@ static void llbitmap_update_sb(void
> *data)
>
> sb = kmap_local_page(sb_page);
> sb->events = cpu_to_le64(mddev->events);
> sb->state = cpu_to_le32(llbitmap->flags);
> sb->chunksize = cpu_to_le32(llbitmap->chunksize);
> - sb->sync_size = cpu_to_le64(mddev->resync_max_sectors);
> + sb->sync_size = cpu_to_le64(llbitmap->sync_size);
> sb->events_cleared = cpu_to_le64(llbitmap->events_cleared);
> sb->sectors_reserved = cpu_to_le32(mddev->bitmap_info.space);
> sb->daemon_sleep =
> cpu_to_le32(mddev->bitmap_info.daemon_sleep);
>
> kunmap_local(sb);
> @@ -1542,10 +1551,11 @@ static int llbitmap_get_stats(void
> *data, struct md_bitmap_stats *stats)
> memset(stats, 0, sizeof(*stats));
>
> stats->missing_pages = 0;
> stats->pages = llbitmap->nr_pages;
> stats->file_pages = llbitmap->nr_pages;
> + stats->sync_size = llbitmap->sync_size;
>
> stats->behind_writes = atomic_read(&llbitmap->behind_writes);
> stats->behind_wait = wq_has_sleeper(&llbitmap->behind_wait);
> stats->events_cleared = llbitmap->events_cleared;
^ permalink raw reply
* Re: [PATCH] md/md-llbitmap: allocate page controls independently
From: Su Yue @ 2026-06-15 11:06 UTC (permalink / raw)
To: Yu Kuai; +Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260605091527.2463539-6-yukuai@kernel.org>
On Fri 05 Jun 2026 at 17:15, Yu Kuai <yukuai@kernel.org> wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> Allocate one llbitmap page-control object at a time and free
> each
> object through the same model.
>
> Let llbitmap_read_page() return a zeroed page without reading
> disk when
> the page index is beyond the current bitmap size, so
> page-control
> allocation no longer needs a separate read_existing flag.
>
> This keeps the llbitmap page-control lifetime self-consistent
> and
> prepares the page-cache code for later in-place growth.
>
> Signed-off-by: Yu Kuai <yukuai@fygo.io>
>
Straight enough.
Reviewed-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md-llbitmap.c | 99
> +++++++++++++++++++++++++---------------
> 1 file changed, 62 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index ecf3ed712315..2f2896fe4d6f 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -510,24 +510,32 @@ static void llbitmap_write(struct llbitmap
> *llbitmap, enum llbitmap_state state,
> llbitmap_set_page_dirty(llbitmap, idx, bit, true);
> else if (state == BitNeedSyncUnwritten)
> llbitmap_set_page_dirty(llbitmap, idx, bit, false);
> }
>
> +static unsigned int llbitmap_used_pages(struct llbitmap
> *llbitmap,
> + unsigned long chunks)
> +{
> + return DIV_ROUND_UP(chunks + BITMAP_DATA_OFFSET, PAGE_SIZE);
> +}
> +
> static struct page *llbitmap_read_page(struct llbitmap
> *llbitmap, int idx)
> {
> struct mddev *mddev = llbitmap->mddev;
> struct page *page = NULL;
> struct md_rdev *rdev;
>
> - if (llbitmap->pctl && llbitmap->pctl[idx])
> + if (llbitmap->pctl && idx < llbitmap->nr_pages &&
> llbitmap->pctl[idx])
> page = llbitmap->pctl[idx]->page;
> if (page)
> return page;
>
> page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> if (!page)
> return ERR_PTR(-ENOMEM);
> + if (idx >= llbitmap_used_pages(llbitmap, llbitmap->chunks))
> + return page;
>
> rdev_for_each(rdev, mddev) {
> sector_t sector;
>
> if (rdev->raid_disk < 0 || test_bit(Faulty, &rdev->flags)
> ||
> @@ -594,65 +602,82 @@ static void llbitmap_free_pages(struct
> llbitmap *llbitmap)
> return;
>
> for (i = 0; i < llbitmap->nr_pages; i++) {
> struct llbitmap_page_ctl *pctl = llbitmap->pctl[i];
>
> - if (!pctl || !pctl->page)
> - break;
> -
> - __free_page(pctl->page);
> + if (!pctl)
> + continue;
> + if (pctl->page)
> + __free_page(pctl->page);
> percpu_ref_exit(&pctl->active);
> + kfree(pctl);
> }
>
> - kfree(llbitmap->pctl[0]);
> kfree(llbitmap->pctl);
> llbitmap->pctl = NULL;
> }
>
> -static int llbitmap_cache_pages(struct llbitmap *llbitmap)
> +static struct llbitmap_page_ctl *
> +llbitmap_alloc_page_ctl(struct llbitmap *llbitmap, int idx)
> {
> struct llbitmap_page_ctl *pctl;
> - unsigned int nr_pages = DIV_ROUND_UP(llbitmap->chunks +
> - BITMAP_DATA_OFFSET, PAGE_SIZE);
> + struct page *page;
> unsigned int size = struct_size(pctl, dirty, BITS_TO_LONGS(
> llbitmap->blocks_per_page));
> - int i;
> -
> - llbitmap->pctl = kmalloc_array(nr_pages, sizeof(void *),
> - GFP_KERNEL | __GFP_ZERO);
> - if (!llbitmap->pctl)
> - return -ENOMEM;
>
> size = round_up(size, cache_line_size());
> - pctl = kmalloc_array(nr_pages, size, GFP_KERNEL | __GFP_ZERO);
> - if (!pctl) {
> - kfree(llbitmap->pctl);
> - return -ENOMEM;
> + pctl = kzalloc(size, GFP_KERNEL);
> + if (!pctl)
> + return ERR_PTR(-ENOMEM);
> +
> + page = llbitmap_read_page(llbitmap, idx);
> +
> + if (IS_ERR(page)) {
> + kfree(pctl);
> + return ERR_CAST(page);
> }
>
> - llbitmap->nr_pages = nr_pages;
> + if (percpu_ref_init(&pctl->active, active_release,
> + PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
> + __free_page(page);
> + kfree(pctl);
> + return ERR_PTR(-ENOMEM);
> + }
>
> - for (i = 0; i < nr_pages; i++, pctl = (void *)pctl + size) {
> - struct page *page = llbitmap_read_page(llbitmap, i);
> + pctl->page = page;
> + pctl->state = page_address(page);
> + init_waitqueue_head(&pctl->wait);
> + return pctl;
> +}
>
> - llbitmap->pctl[i] = pctl;
> +static unsigned int llbitmap_reserved_pages(struct llbitmap
> *llbitmap)
> +{
> + return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space <<
> SECTOR_SHIFT,
> + PAGE_SIZE);
> +}
>
> - if (IS_ERR(page)) {
> - llbitmap_free_pages(llbitmap);
> - return PTR_ERR(page);
> - }
> +static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
> +{
> + unsigned int used_pages = llbitmap_used_pages(llbitmap,
> llbitmap->chunks);
> + unsigned int nr_pages = max(used_pages,
> llbitmap_reserved_pages(llbitmap));
> + int i;
> +
> + llbitmap->pctl = kcalloc(nr_pages, sizeof(*llbitmap->pctl),
> GFP_KERNEL);
> + if (!llbitmap->pctl)
> + return -ENOMEM;
>
> - if (percpu_ref_init(&pctl->active, active_release,
> - PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
> - __free_page(page);
> + llbitmap->nr_pages = nr_pages;
> +
> + for (i = 0; i < nr_pages; i++) {
> + llbitmap->pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
> + if (IS_ERR(llbitmap->pctl[i])) {
> + int ret = PTR_ERR(llbitmap->pctl[i]);
> +
> + llbitmap->pctl[i] = NULL;
> llbitmap_free_pages(llbitmap);
> - return -ENOMEM;
> + return ret;
> }
> -
> - pctl->page = page;
> - pctl->state = page_address(page);
> - init_waitqueue_head(&pctl->wait);
> }
>
> return 0;
> }
>
> @@ -921,11 +946,11 @@ static int llbitmap_init(struct llbitmap
> *llbitmap)
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = chunks;
> llbitmap->sync_size = blocks;
> mddev->bitmap_info.daemon_sleep = DEFAULT_DAEMON_SLEEP;
>
> - ret = llbitmap_cache_pages(llbitmap);
> + ret = llbitmap_alloc_pages(llbitmap);
> if (ret)
> return ret;
>
> llbitmap_state_machine(llbitmap, 0, llbitmap->chunks - 1,
> BitmapActionInit);
> @@ -1030,11 +1055,11 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size,
> chunksize);
> llbitmap->chunkshift = ffz(~chunksize);
> llbitmap->sync_size = sync_size;
> - ret = llbitmap_cache_pages(llbitmap);
> + ret = llbitmap_alloc_pages(llbitmap);
>
> out_put_page:
> __free_page(sb_page);
> kunmap_local(sb);
> return ret;
^ permalink raw reply
* Re: [PATCH] md/md-llbitmap: grow the page cache in place for reshape
From: Su Yue @ 2026-06-15 11:16 UTC (permalink / raw)
To: Yu Kuai; +Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260605091527.2463539-7-yukuai@kernel.org>
On Fri 05 Jun 2026 at 17:15, Yu Kuai <yukuai@kernel.org> wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> Use the page-control helpers to grow llbitmap's cached pages in
> place
> for resize and later reshape preparation, instead of rebuilding
> the
> whole cache.
>
> Signed-off-by: Yu Kuai <yukuai@fygo.io>
> ---
> drivers/md/md-llbitmap.c | 139
> +++++++++++++++++++++++++++++++++++----
> 1 file changed, 127 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 2f2896fe4d6f..91d3dec43d48 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -414,10 +414,23 @@ static char
> state_machine[BitStateCount][BitmapActionCount] = {
> [BitmapActionClearUnwritten] = BitUnwritten,
> },
> };
>
> static void __llbitmap_flush(struct mddev *mddev);
> +static void llbitmap_flush(struct mddev *mddev);
> +static void llbitmap_update_sb(void *data);
> +
> +static void llbitmap_resize_chunks(struct mddev *mddev,
> sector_t blocks,
> + unsigned long *chunksize,
> + unsigned long *chunks)
>
NIT: I would like call it llbitmap_calculate_chunks.
> +{
> + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
> + while (*chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
> + *chunksize = *chunksize << 1;
> + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
> + }
> +}
>
> static enum llbitmap_state llbitmap_read(struct llbitmap
> *llbitmap, loff_t pos)
> {
> unsigned int idx;
> unsigned int offset;
> @@ -653,10 +666,52 @@ static unsigned int
> llbitmap_reserved_pages(struct llbitmap *llbitmap)
> {
> return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space <<
> SECTOR_SHIFT,
> PAGE_SIZE);
> }
>
> +static int llbitmap_expand_pages(struct llbitmap *llbitmap,
> + unsigned long chunks)
> +{
> + struct llbitmap_page_ctl **pctl;
> + unsigned int old_nr_pages = llbitmap->nr_pages;
> + unsigned int nr_pages = llbitmap_used_pages(llbitmap, chunks);
> + int i;
> + int ret;
> +
> + if (nr_pages <= old_nr_pages)
> + return 0;
> +
> + pctl = kcalloc(nr_pages, sizeof(*pctl), GFP_KERNEL);
> + if (!pctl)
> + return -ENOMEM;
> +
> + if (llbitmap->pctl)
> + memcpy(pctl, llbitmap->pctl,
> + array_size(old_nr_pages, sizeof(*pctl)));
> +
> + for (i = old_nr_pages; i < nr_pages; i++) {
> + pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
> + if (IS_ERR(pctl[i]))
> + goto err_alloc_ptr;
> + }
> +
> + kfree(llbitmap->pctl);
> + llbitmap->pctl = pctl;
> + llbitmap->nr_pages = nr_pages;
> + return 0;
> +
> +err_alloc_ptr:
> + ret = PTR_ERR(pctl[i]);
> + for (i--; i >= (int)old_nr_pages; i--) {
>
Confused about why not just declare i as an unsigned int?
--
Su
> + __free_page(pctl[i]->page);
> + percpu_ref_exit(&pctl[i]->active);
> + kfree(pctl[i]);
> + }
> + kfree(pctl);
> + return ret;
> +}
> +
> static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
> {
> unsigned int used_pages = llbitmap_used_pages(llbitmap,
> llbitmap->chunks);
> unsigned int nr_pages = max(used_pages,
> llbitmap_reserved_pages(llbitmap));
> int i;
> @@ -728,10 +783,38 @@ static bool llbitmap_zero_all_disks(struct
> llbitmap *llbitmap)
> }
>
> return true;
> }
>
> +static void llbitmap_mark_range(struct llbitmap *llbitmap,
> + unsigned long start,
> + unsigned long end,
> + enum llbitmap_state state)
> +{
> + while (start <= end) {
> + llbitmap_write(llbitmap, state, start);
> + start++;
> + }
> +}
> +
> +static int llbitmap_prepare_resize(struct llbitmap *llbitmap,
> + unsigned long old_chunks,
> + unsigned long new_chunks,
> + unsigned long cache_chunks)
> +{
> + int ret;
> +
> + llbitmap_flush(llbitmap->mddev);
> + ret = llbitmap_expand_pages(llbitmap, cache_chunks);
> + if (ret)
> + return ret;
> + if (new_chunks > old_chunks)
> + llbitmap_mark_range(llbitmap, old_chunks, new_chunks - 1,
> + BitUnwritten);
> + return 0;
> +}
> +
> static void llbitmap_init_state(struct llbitmap *llbitmap)
> {
> struct mddev *mddev = llbitmap->mddev;
> enum llbitmap_state state = BitUnwritten;
> unsigned long i;
> @@ -1024,14 +1107,14 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> pr_err("md/llbitmap: %s: chunksize not a power of 2",
> mdname(mddev));
> goto out_put_page;
> }
>
> - if (chunksize <
> DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors,
> + if (chunksize < DIV_ROUND_UP_SECTOR_T(sync_size,
> mddev->bitmap_info.space <<
> SECTOR_SHIFT)) {
> pr_err("md/llbitmap: %s: chunksize too small %lu < %llu /
> %lu",
> - mdname(mddev), chunksize,
> mddev->resync_max_sectors,
> + mdname(mddev), chunksize, sync_size,
> mddev->bitmap_info.space);
> goto out_put_page;
> }
>
> daemon_sleep = le32_to_cpu(sb->daemon_sleep);
> @@ -1169,28 +1252,60 @@ static int llbitmap_create(struct mddev
> *mddev)
> }
>
> static int llbitmap_resize(struct mddev *mddev, sector_t
> blocks, int chunksize)
> {
> struct llbitmap *llbitmap = mddev->bitmap;
> + sector_t old_blocks = llbitmap->sync_size;
> + unsigned long old_chunks = llbitmap->chunks;
> unsigned long chunks;
> + unsigned long cache_chunks;
> + int ret = 0;
> + unsigned long bitmap_chunksize;
> + bool reshape;
>
> if (chunksize == 0)
> chunksize = llbitmap->chunksize;
>
> - /* If there is enough space, leave the chunksize unchanged. */
> - chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
> - while (chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
> - chunksize = chunksize << 1;
> - chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
> - }
> + bitmap_chunksize = chunksize;
> + llbitmap_resize_chunks(mddev, blocks, &bitmap_chunksize,
> &chunks);
>
> - llbitmap->chunkshift = ffz(~chunksize);
> - llbitmap->chunksize = chunksize;
> - llbitmap->chunks = chunks;
> - llbitmap->sync_size = blocks;
> + reshape = mddev->delta_disks || mddev->new_level !=
> mddev->level ||
> + mddev->new_layout != mddev->layout ||
> + mddev->new_chunk_sectors != mddev->chunk_sectors;
> + if (!reshape && bitmap_chunksize != llbitmap->chunksize)
> + return -EOPNOTSUPP;
> + if (blocks == old_blocks && chunks == llbitmap->chunks)
> + return 0;
> +
> + mutex_lock(&mddev->bitmap_info.mutex);
>
> + cache_chunks = reshape ? max(old_chunks, chunks) : chunks;
> + ret = llbitmap_prepare_resize(llbitmap, old_chunks, chunks,
> cache_chunks);
> + if (ret)
> + goto out;
> +
> + if (reshape) {
> + llbitmap->reshape_sync_size = blocks;
> + llbitmap->reshape_chunksize = bitmap_chunksize;
> + llbitmap->reshape_chunks = chunks;
> + llbitmap->chunks = max(old_chunks, chunks);
> + } else {
> + if (blocks < old_blocks && chunks < old_chunks)
> + llbitmap_mark_range(llbitmap, chunks, old_chunks - 1,
> + BitUnwritten);
> + mddev->bitmap_info.chunksize = bitmap_chunksize;
> + llbitmap->chunks = chunks;
> + llbitmap->sync_size = blocks;
> + llbitmap_update_sb(llbitmap);
> + }
> + __llbitmap_flush(mddev);
> + mutex_unlock(&mddev->bitmap_info.mutex);
> return 0;
> +
> +out:
> + mutex_unlock(&mddev->bitmap_info.mutex);
> + return ret;
> }
>
> static int llbitmap_load(struct mddev *mddev)
> {
> enum llbitmap_action action = BitmapActionReload;
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox