* [PATCH v3 7/7] md/raid10: simplify read request error handling
From: Abd-Alrhman Masalkhi @ 2026-07-08 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, Abd-Alrhman Masalkhi
In-Reply-To: <20260708101341.473750-1-abd.masalkhi@gmail.com>
raid10_read_request() currently handles bio completion, barrier
handling, and r10_bio lifetime management in several different error
paths. This results in duplicated cleanup logic and increases the risk
of introducing bugs in future modifications.
Make raid10_read_request() return a status to its callers, consolidate
the read error paths, and free r10_bio from a single location in the
callers. Since the callers allocate r10_bio, they should also be
responsible for freeing it when the request fails.
This makes the read path follow the same ownership model as the write
path and simplifies the error handling flow.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-8-abd.masalkhi@gmail.com/
Changes in v2:
- Fix a compilation error (bi -> bio).
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-8-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 45 +++++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index d94c1f28a6f6..01162c483644 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1143,7 +1143,7 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
return true;
}
-static void raid10_read_request(struct mddev *mddev, struct bio *bio,
+static bool raid10_read_request(struct mddev *mddev, struct bio *bio,
struct r10bio *r10_bio)
{
struct r10conf *conf = mddev->private;
@@ -1191,8 +1191,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
- return;
+ return false;
}
rdev = read_balance(conf, r10_bio, &max_sectors);
@@ -1202,8 +1201,8 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
mdname(mddev), b,
(unsigned long long)r10_bio->sector);
}
- raid_end_bio_io(r10_bio);
- return;
+ bio_io_error(bio);
+ goto err_allow_barrier;
}
if (err_rdev)
pr_err_ratelimited("md/raid10:%s: %pg: redirecting sector %llu to another mirror\n",
@@ -1215,10 +1214,8 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
wait_barrier(conf, false);
- if (!bio) {
- set_bit(R10BIO_Returned, &r10_bio->state);
- goto err_handle;
- }
+ if (!bio)
+ goto err_dec_pending;
r10_bio->master_bio = bio;
r10_bio->sectors = max_sectors;
@@ -1244,10 +1241,16 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
read_bio->bi_private = r10_bio;
mddev_trace_remap(mddev, read_bio, r10_bio->sector);
submit_bio_noacct(read_bio);
- return;
-err_handle:
+
+ return true;
+
+err_dec_pending:
atomic_dec(&rdev->nr_pending);
- raid_end_bio_io(r10_bio);
+
+err_allow_barrier:
+ allow_barrier(conf);
+
+ return false;
}
static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
@@ -1538,14 +1541,13 @@ static bool __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 {
+ ret = raid10_read_request(mddev, bio, r10_bio);
+ else
ret = raid10_write_request(mddev, bio, r10_bio);
- if (!ret)
- free_r10bio(r10_bio);
- }
+
+ if (!ret)
+ free_r10bio(r10_bio);
return ret;
}
@@ -1875,6 +1877,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sector_t chunk_mask = (conf->geo.chunk_mask & conf->prev.chunk_mask);
int chunk_sects = chunk_mask + 1;
int sectors = bio_sectors(bio);
+ bool write = bio_data_dir(bio) == WRITE;
if (unlikely(bio->bi_opf & REQ_PREFLUSH)
&& md_flush_request(mddev, bio))
@@ -1898,7 +1901,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sectors = chunk_sects -
(bio->bi_iter.bi_sector &
(chunk_sects - 1));
- if (!__make_request(mddev, bio, sectors))
+ if (!__make_request(mddev, bio, sectors) && write)
md_write_end(mddev);
/* In case raid10d snuck in to freeze_array */
@@ -2866,7 +2869,9 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
rdev_dec_pending(rdev, mddev);
r10_bio->state = 0;
- raid10_read_request(mddev, r10_bio->master_bio, r10_bio);
+ if (!raid10_read_request(mddev, r10_bio->master_bio, r10_bio))
+ free_r10bio(r10_bio);
+
/*
* allow_barrier after re-submit to ensure no sync io
* can be issued while regular io pending.
--
2.43.0
^ permalink raw reply related
* [PATCH v3 6/7] md/raid10: simplify write request error handling
From: Abd-Alrhman Masalkhi @ 2026-07-08 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, Abd-Alrhman Masalkhi
In-Reply-To: <20260708101341.473750-1-abd.masalkhi@gmail.com>
raid10_write_request() currently handles bio completion, barrier
handling, and r10_bio lifetime management in several different error
paths. This results in duplicated cleanup logic and increases the risk
of introducing bugs in future modifications.
Move bio_wouldblock_error() handling to the callers of
regular_request_wait(), consolidate the write error paths, and free
r10_bio from a single location in __make_request() when
raid10_write_request() fails.
It remove redundant local copies of r10_bio->sectors and use a single
max_sectors variable throughout the function.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-7-abd.masalkhi@gmail.com/
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-7-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 58 ++++++++++++++++++++++-----------------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 57813f249578..d94c1f28a6f6 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1123,18 +1123,16 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
struct bio *bio, sector_t sectors)
{
/* Bail out if REQ_NOWAIT is set for the bio */
- if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
- bio_wouldblock_error(bio);
+ if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT))
return false;
- }
+
while (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
bio->bi_iter.bi_sector < conf->reshape_progress &&
bio->bi_iter.bi_sector + sectors > conf->reshape_progress) {
allow_barrier(conf);
- if (bio->bi_opf & REQ_NOWAIT) {
- bio_wouldblock_error(bio);
+ if (bio->bi_opf & REQ_NOWAIT)
return false;
- }
+
mddev_add_trace_msg(conf->mddev, "raid10 wait reshape");
wait_event(conf->wait_barrier,
conf->reshape_progress <= bio->bi_iter.bi_sector ||
@@ -1192,6 +1190,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
}
if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
+ bio_wouldblock_error(bio);
free_r10bio(r10_bio);
return;
}
@@ -1354,8 +1353,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
{
struct r10conf *conf = mddev->private;
int i, k;
- sector_t sectors;
- int max_sectors;
+ int max_sectors = r10_bio->sectors;
+ bool nowait = bio->bi_opf & REQ_NOWAIT;
bool atomic = bio->bi_opf & REQ_ATOMIC;
if ((mddev_is_clustered(mddev) &&
@@ -1363,9 +1362,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
bio->bi_iter.bi_sector,
bio_end_sector(bio)))) {
/* Bail out if REQ_NOWAIT is set for the bio */
- if (bio->bi_opf & REQ_NOWAIT) {
+ if (nowait) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
return false;
}
@@ -1375,28 +1373,25 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
bio_end_sector(bio)));
}
- sectors = r10_bio->sectors;
- if (!regular_request_wait(mddev, conf, bio, sectors)) {
- free_r10bio(r10_bio);
+ if (!regular_request_wait(mddev, conf, bio, max_sectors)) {
+ bio_wouldblock_error(bio);
return false;
}
if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
(mddev->reshape_backwards
? (bio->bi_iter.bi_sector < conf->reshape_safe &&
- bio->bi_iter.bi_sector + sectors > conf->reshape_progress)
- : (bio->bi_iter.bi_sector + sectors > conf->reshape_safe &&
+ bio->bi_iter.bi_sector + max_sectors > conf->reshape_progress)
+ : (bio->bi_iter.bi_sector + max_sectors > conf->reshape_safe &&
bio->bi_iter.bi_sector < conf->reshape_progress))) {
/* Need to update reshape_position in metadata */
mddev->reshape_position = conf->reshape_progress;
set_mask_bits(&mddev->sb_flags, 0,
BIT(MD_SB_CHANGE_DEVS) | BIT(MD_SB_CHANGE_PENDING));
md_wakeup_thread(mddev->thread);
- if (bio->bi_opf & REQ_NOWAIT) {
- allow_barrier(conf);
+ if (nowait) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
- return false;
+ goto err_allow_barrier;
}
mddev_add_trace_msg(conf->mddev,
"raid10 wait reshape metadata");
@@ -1421,8 +1416,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
wait_blocked_dev(mddev, r10_bio);
- max_sectors = r10_bio->sectors;
-
for (i = 0; i < conf->copies; i++) {
int d = r10_bio->devs[i].devnum;
struct md_rdev *rdev, *rrdev;
@@ -1479,15 +1472,15 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
r10_bio->sectors = max_sectors;
if (r10_bio->sectors < bio_sectors(bio)) {
- if (atomic)
- goto err_handle;
+ if (atomic) {
+ bio_io_error(bio);
+ goto err_dec_pending;
+ }
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
- if (!bio) {
- set_bit(R10BIO_Returned, &r10_bio->state);
- goto err_handle;
- }
+ if (!bio)
+ goto err_dec_pending;
r10_bio->master_bio = bio;
}
@@ -1505,7 +1498,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
one_write_done(r10_bio);
return true;
-err_handle:
+err_dec_pending:
for (k = 0; k < i; k++) {
int d = r10_bio->devs[k].devnum;
struct md_rdev *rdev = conf->mirrors[d].rdev;
@@ -1521,7 +1514,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
}
}
- raid_end_bio_io(r10_bio);
+err_allow_barrier:
+ allow_barrier(conf);
+
return false;
}
@@ -1546,8 +1541,11 @@ static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
ret = true;
if (bio_data_dir(bio) == READ)
raid10_read_request(mddev, bio, r10_bio);
- else
+ else {
ret = raid10_write_request(mddev, bio, r10_bio);
+ if (!ret)
+ free_r10bio(r10_bio);
+ }
return ret;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v3 5/7] md/raid10: replace wait loop with wait_event_idle()
From: Abd-Alrhman Masalkhi @ 2026-07-08 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, Abd-Alrhman Masalkhi
In-Reply-To: <20260708101341.473750-1-abd.masalkhi@gmail.com>
The wait loop is equivalent to wait_event_idle() and can be simplified
by usaing it for improving readability.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-6-abd.masalkhi@gmail.com/
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-6-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 2574f60dd771..57813f249578 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1362,22 +1362,17 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
mddev->cluster_ops->area_resyncing(mddev, WRITE,
bio->bi_iter.bi_sector,
bio_end_sector(bio)))) {
- DEFINE_WAIT(w);
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
free_r10bio(r10_bio);
return false;
}
- for (;;) {
- prepare_to_wait(&conf->wait_barrier,
- &w, TASK_IDLE);
- if (!mddev->cluster_ops->area_resyncing(mddev, WRITE,
- bio->bi_iter.bi_sector, bio_end_sector(bio)))
- break;
- schedule();
- }
- finish_wait(&conf->wait_barrier, &w);
+
+ wait_event_idle(conf->wait_barrier,
+ !mddev->cluster_ops->area_resyncing(mddev, WRITE,
+ bio->bi_iter.bi_sector,
+ bio_end_sector(bio)));
}
sectors = r10_bio->sectors;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 4/7] md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
From: Abd-Alrhman Masalkhi @ 2026-07-08 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, Abd-Alrhman Masalkhi
In-Reply-To: <20260708101341.473750-1-abd.masalkhi@gmail.com>
raid10_write_request() drops the barrier before calling
bio_submit_split_bioset() and reacquires it afterwards. This is no
longer necessary because the split bio cannot re-enter
raid10_write_request() while the barrier is held.
The allow_barrier()/wait_barrier() pair was introduced by commit
e820d55cb99d ("md: fix raid10 hang issue caused by barrier") when
submit_flushes() called md_handle_request() directly, allowing re-entry
into raid10_write_request(). Since v5.2, submit_flushes() has instead
gone through submit_bio(), eliminating that recursion. submit_flushes()
was later removed entirely by commit b75197e86e6d ("md: Remove flush
handling").
Currently, raid10_write_request() is only entered from the bio
submission path, so the split bio submitted by bio_submit_split_bioset()
cannot recurse back into wait_barrier().
Remove the redundant allow_barrier()/wait_barrier() pair around
bio_submit_split_bioset().
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-5-abd.masalkhi@gmail.com/
Changes in v2:
- Expand the commit message to explain why the
allow_barrier()/wait_barrier() pair is no longer needed.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-5-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 3480fc7907f0..2574f60dd771 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1487,10 +1487,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (atomic)
goto err_handle;
- allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
- wait_barrier(conf, false);
if (!bio) {
set_bit(R10BIO_Returned, &r10_bio->state);
goto err_handle;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/7] md/raid10: consistently fail atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-07-08 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, Abd-Alrhman Masalkhi
In-Reply-To: <20260708101341.473750-1-abd.masalkhi@gmail.com>
RAID10 currently handles one badblock path explicitly by failing atomic
writes with EIO. However, another badblock path can also reduce the
writable range and force the bio through bio_submit_split_bioset(),
which implicitly completes the bio with EINVAL.
Fix this by handling atomic writes in the common split check. If RAID10
determines that an atomic write would require splitting, complete the
bio with EIO.
Fixes: a1d9b4fd42d9 ("md/raid10: Atomic write support")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-4-abd.masalkhi@gmail.com/
Changes in v2:
- Drop the early atomic write split check from raid10_write_request()
and rely on queue limits instead.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-4-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index bd322eccdc3f..3480fc7907f0 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1356,6 +1356,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
int i, k;
sector_t sectors;
int max_sectors;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
if ((mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1464,16 +1465,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (is_bad) {
int good_sectors;
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC)
- goto err_handle;
-
good_sectors = first_bad - dev_sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1493,6 +1484,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
r10_bio->sectors = max_sectors;
if (r10_bio->sectors < bio_sectors(bio)) {
+ if (atomic)
+ goto err_handle;
+
allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-07-08 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, Abd-Alrhman Masalkhi
In-Reply-To: <20260708101341.473750-1-abd.masalkhi@gmail.com>
Atomic writes in RAID1 must fit within a single barrier unit. Set
chunk_sectors to BARRIER_UNIT_SECTOR_SIZE so that bios which would cross
a barrier-unit boundary are rejected by the block layer before reaching
MD.
A bio that passes block-layer validation may still become unserviceable
within RAID1 due to bad blocks or write-behind constraints. In the former
case, complete the bio with EIO. In the latter case, disable
write-behind rather than failing the bio with EIO.
Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
Fixes: a4c55c902670 ("md/raid1: simplify raid1_write_request() error handling")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v3:
- Set chunk_sectors to BARRIER_UNIT_SECTOR_SIZE instead of setting
atomic_write_hw_unit_max.
- Avoid enabling write-behind when the atomic write exceeds the
write-behind limit.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-3-abd.masalkhi@gmail.com/
Changes in v2:
- Drop the early atomic write split check from raid1_write_request().
- Advertise the atomic write size limit via queue limits.
- Disable write-behind instead of failing atomic writes when the
BIO_MAX_VECS limit is encountered.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/
---
drivers/md/raid1.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index afe2ca96ad8c..6c8beca995e6 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
int first_clone;
bool write_behind = false;
bool nowait = bio->bi_opf & REQ_NOWAIT;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
bool is_discard = op_is_discard(bio->bi_opf);
sector_t sector = bio->bi_iter.bi_sector;
@@ -1577,7 +1578,8 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* write-mostly, which means we could allocate write behind
* bio later.
*/
- if (!is_discard && rdev && test_bit(WriteMostly, &rdev->flags))
+ if (!is_discard && rdev && test_bit(WriteMostly, &rdev->flags) &&
+ (!atomic || max_sectors <= BIO_MAX_VECS * (PAGE_SIZE >> 9)))
write_behind = true;
r1_bio->bios[i] = NULL;
@@ -1603,20 +1605,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
}
if (is_bad) {
int good_sectors;
-
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC) {
- bio->bi_status = BLK_STS_NOTSUPP;
- bio_endio(bio);
- goto err_dec_pending;
- }
-
good_sectors = first_bad - sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1636,7 +1624,13 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
if (write_behind && mddev->bitmap)
max_sectors = min_t(int, max_sectors,
BIO_MAX_VECS * (PAGE_SIZE >> 9));
+
if (max_sectors < bio_sectors(bio)) {
+ if (atomic) {
+ bio_io_error(bio);
+ goto err_dec_pending;
+ }
+
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
if (!bio)
@@ -3228,6 +3222,7 @@ static int raid1_set_limits(struct mddev *mddev)
md_init_stacking_limits(&lim);
lim.max_write_zeroes_sectors = 0;
lim.max_hw_wzeroes_unmap_sectors = 0;
+ lim.chunk_sectors = BARRIER_UNIT_SECTOR_SIZE;
lim.logical_block_size = mddev->logical_block_size;
lim.features |= BLK_FEAT_ATOMIC_WRITES;
lim.features |= BLK_FEAT_PCI_P2PDMA;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
From: Abd-Alrhman Masalkhi @ 2026-07-08 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, Abd-Alrhman Masalkhi, sashiko-bot
In-Reply-To: <20260708101341.473750-1-abd.masalkhi@gmail.com>
When raid10_write_request() fails because REQ_NOWAIT is set, the
allocated r10_bio is not freed before returning, resulting in a memory
leak. Free r10_bio before returning from the REQ_NOWAIT error paths.
Fixes: c9aa889b035f ("md: raid10 add nowait support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-raid/20260613184042.BCEC01F000E9@smtp.kernel.org/
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-2-abd.masalkhi@gmail.com/
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-2-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0a3cfdd3f5df..bd322eccdc3f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1365,6 +1365,7 @@ static bool 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);
+ free_r10bio(r10_bio);
return false;
}
for (;;) {
@@ -1398,6 +1399,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (bio->bi_opf & REQ_NOWAIT) {
allow_barrier(conf);
bio_wouldblock_error(bio);
+ free_r10bio(r10_bio);
return false;
}
mddev_add_trace_msg(conf->mddev,
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/7] md/raid10: fixes, atomic write handling, and error-path cleanup
From: Abd-Alrhman Masalkhi @ 2026-07-08 10:13 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, Abd-Alrhman Masalkhi
Hi,
This v3 of series contains a mix of bug fixes and cleanups for RAID10,
along with a related atomic write fix for RAID1.
Changes in v3:
- Set chunk_sectors to BARRIER_UNIT_SECTOR_SIZE instead of setting
atomic_write_hw_unit_max.
- Avoid enabling write-behind when the atomic write exceeds the
write-behind limit.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-1-abd.masalkhi@gmail.com/
Changes in v2:
- Expand the commit message to explain why the
allow_barrier()/wait_barrier() pair is no longer needed.
- Drop the early atomic write split check from raid1_write_request().
- Advertise the atomic write size limit via queue limits.
- Disable write-behind instead of failing atomic writes when the
BIO_MAX_VECS limit is encountered.
- Drop the early atomic write split check from raid10_write_request()
and rely on queue limits instead.
- Fix a compilation error (bi -> bio).
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-1-abd.masalkhi@gmail.com/
Thanks,
Abd-alrhman,
Abd-Alrhman Masalkhi (7):
md/raid10: fix r10bio leak in raid10_write_request() error paths
md/raid1: advertise atomic write limits and handle runtime constraints
md/raid10: consistently fail atomic writes that require splitting
md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
md/raid10: replace wait loop with wait_event_idle()
md/raid10: simplify write request error handling
md/raid10: simplify read request error handling
drivers/md/raid1.c | 25 ++++------
drivers/md/raid10.c | 118 +++++++++++++++++++++-----------------------
2 files changed, 65 insertions(+), 78 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH 6/8] raid6: defer implementation selection when built-in
From: sashiko-bot @ 2026-07-08 9:23 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, yukuai
In-Reply-To: <20260708090740.1433685-7-hch@lst.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The boot-time RAID6 benchmark runs unconditionally when built-in, ignoring the `CONFIG_RAID6_PQ_BENCHMARK` setting and the single-algorithm optimization.
--
--- Patch [6]: [PATCH 6/8] raid6: defer implementation selection when built-in ---
commit 2c9ec4f0b44515f49715b67c2e70f8628c5abd61
Author: Christoph Hellwig <hch@lst.de>
Subject: raid6: defer implementation selection when built-in
This commit defers the RAID6 implementation selection to avoid holding up
early boot. It follows the pattern used in the RAID5 XOR code, allowing
full use of the timing subsystem for benchmarking.
> diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
> index 6f5c89ab2b17b..e7984bde5157f 100644
> --- a/lib/raid/raid6/algos.c
> +++ b/lib/raid/raid6/algos.c
> @@ -236,16 +236,6 @@ static int __init raid6_select_algo(void)
> int i, cycle;
> int error;
>
> - if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
> - pr_info("raid6: skipped pq benchmark and selected %s\n",
> - raid6_algos[raid6_nr_algos - 1]->name);
> - static_call_update(raid6_gen_syndrome_impl,
> - raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
> - static_call_update(raid6_xor_syndrome_impl,
> - raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
> - return 0;
> - }
> -
> /* prepare the buffer and fill it circularly with gfmul table */
> disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
[ ... ]
> @@ -329,13 +319,40 @@ static int __init raid6_init(void)
> static_call_update(raid6_recov_datap_impl, raid6_recov_algo->datap);
> pr_info("raid6: using %s recovery algorithm\n", raid6_recov_algo->name);
>
> + if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
> + pr_info("raid6: skipped pq benchmark and selected %s\n",
> + raid6_algos[raid6_nr_algos - 1]->name);
> + goto default_algo;
> + }
> +
> +#ifdef MODULE
> return raid6_select_algo();
> +#else
> + /*
> + * Pick the last registered implementation as the temporary default until
> + * calibration happens.
> + */
> +#endif
[ ... ]
> +/*
> + * When built-in we must register the default implementation before md
> + * initializes, but we don't want calibration to run that early as that
> + * would delay the boot process.
> + */
> +#ifndef MODULE
> +device_initcall(raid6_select_algo);
> +#endif
[Severity: Medium]
Will this unconditional device_initcall cause the benchmark to run even when
it is intentionally disabled in the configuration?
While raid6_init() checks CONFIG_RAID6_PQ_BENCHMARK and correctly skips the
early calibration phase, removing that same check from raid6_select_algo()
means this deferred call will unconditionally run the benchmarking loop later
in the boot process. It looks like this bypasses both the config setting and
the single-algorithm optimization entirely.
> subsys_initcall(raid6_init);
> module_exit(raid6_exit);
> MODULE_LICENSE("GPL");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=6
^ permalink raw reply
* Re: [PATCH 4/8] xor/kunit: add a benchmark
From: sashiko-bot @ 2026-07-08 9:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: yukuai, linux-raid
In-Reply-To: <20260708090740.1433685-5-hch@lst.de>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Potential divide-by-zero kernel panic in `xor_benchmark()` when calculating the benchmark speed if the execution time is 0.
- [Medium] The benchmark holds `preempt_disable()` for the entire duration of 1000 XOR iterations, risking soft lockups on slower hardware or emulators.
--
--- Patch [4]: [PATCH 4/8] xor/kunit: add a benchmark ---
commit 4f82691a9522c9ab25bdc94cf0c1ad74dc8c3b7e
Author: Christoph Hellwig <hch@lst.de>
xor/kunit: add a benchmark
This patch adds a benchmark to test the XOR functions for more
representative block sizes and numbers of disks. It measures the execution
time of 1000 iterations of xor_gen() for varying buffer counts and lengths,
reporting the throughput in GB/s.
> diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
> index 659ae3edbc25b..648c6da9464c4 100644
> --- a/lib/raid/xor/tests/xor_kunit.c
> +++ b/lib/raid/xor/tests/xor_kunit.c
[ ... ]
> @@ -125,8 +125,70 @@ static void xor_test(struct kunit *test)
> }
> }
>
> +static void xor_benchmark(struct kunit *test)
> +{
[ ... ]
> + for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
> + unsigned int len = len_to_test[j];
> + const unsigned long num_iters = 1000;
> +
> + KUNIT_ASSERT_GT(test, len, 0);
> + KUNIT_ASSERT_LE(test, len, XOR_KUNIT_MAX_BYTES);
> +
> + preempt_disable();
> + t = ktime_get_ns();
> + for (l = 0; l < num_iters; l++)
> + xor_gen(test_dest, test_buffers, nr, len);
> + t = ktime_get_ns() - t;
> + preempt_enable();
[Severity: Medium]
Does this workload risk triggering the kernel's soft lockup detector?
The test executes a hardcoded 1000 iterations of xor_gen() on up to 32 buffers
of 16KB within a single preempt_disable() block. This amounts to processing
over 500MB of data atomically.
In slower environments like QEMU TCG without KVM, this large computational
workload could take tens of seconds, which might exceed the typical 20-second
soft lockup threshold.
> +
> + speed[j] = div64_u64((u64)len * num_iters * nr, t);
[Severity: High]
Can this result in a divide-by-zero kernel panic?
If the benchmark runs on a system with coarse timekeeping resolution (e.g.,
using the jiffies clocksource), and the CPU completes the 1000 iterations
within a single timer tick, the elapsed time `t` could evaluate to 0.
Since `t` is used directly as the divisor in div64_u64() without being
checked, this might trigger a hardware exception and panic the kernel.
> + }
> +
> + static_assert(ARRAY_SIZE(len_to_test) == 2);
> + kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
> + nr, speed[0], speed[1]);
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=4
^ permalink raw reply
* Re: [PATCH 8/8] raid6/kunit: add a benchmark
From: sashiko-bot @ 2026-07-08 9:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: yukuai, linux-raid
In-Reply-To: <20260708090740.1433685-9-hch@lst.de>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Potential division by zero in benchmark speed calculation if elapsed time `t` evaluates to zero.
- [Medium] Unbounded atomic section (preemption disabled) can cause soft lockups or RCU stalls on slower architectures.
- [Low] Duplicate `RAID6_PQ` Kconfig symbol declaration.
- [Low] Inaccurate commit message referencing XOR functions instead of RAID6 PQ functions.
--
--- Patch [8]: [PATCH 8/8] raid6/kunit: add a benchmark ---
commit 63d135afc471564206ce7921d64339a16ff1484f
Author: Christoph Hellwig <hch@lst.de>
raid6/kunit: add a benchmark
This commit adds a benchmark to the RAID6 KUnit test suite to evaluate P/Q
generation speeds across different disk counts and block sizes. It splits
the max alloc size from the kunit max size and reports the performance in
GB/s.
> Add a benchmark to test the XOR functions for more representative block
> sizes and numbers of disks. This splits the maximum alloc size used for
[Severity: Low]
This isn't a bug, but the commit message states it adds a benchmark for the
"XOR functions", while the patch actually adds a benchmark for the RAID6 P/Q
generation algorithm.
[ ... ]
> diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
> index c4eeb7c716c89..c52d5c2052946 100644
> --- a/lib/raid/Kconfig
> +++ b/lib/raid/Kconfig
> @@ -60,6 +60,15 @@ config RAID6_PQ_KUNIT_TEST
> This is intended to help people writing architecture-specific
> optimized versions. If unsure, say N.
>
> +config RAID6_PQ_KUNIT_BENCHMARK
> + bool "Benchmark for RAID6 PQ"
> + depends on RAID6_PQ_KUNIT_TEST
> + help
> + Include benchmarks in the KUnit test suite for raid P/Q generation.
> +
> +config RAID6_PQ
> + tristate
> +
[Severity: Low]
Is this duplicate declaration of config RAID6_PQ intentional? It appears to
be defined earlier in this same file.
> config RAID6_PQ_BENCHMARK
> bool "Automatically choose fastest RAID6 PQ functions"
> depends on RAID6_PQ
> diff --git a/lib/raid/raid6/tests/raid6_kunit.c b/lib/raid/raid6/tests/raid6_kunit.c
> index 9f3e671a12241..f927e3456d175 100644
> --- a/lib/raid/raid6/tests/raid6_kunit.c
> +++ b/lib/raid/raid6/tests/raid6_kunit.c
[ ... ]
> @@ -229,6 +230,68 @@ static void raid6_test(struct kunit *test)
> raid6_test_one(test);
> }
>
> +static void raid6_benchmark(struct kunit *test)
> +{
[ ... ]
> + for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
> + unsigned int len = len_to_test[j];
> + const unsigned long num_iters = 1000;
> +
> + KUNIT_ASSERT_GT(test, len, 0);
> + KUNIT_ASSERT_LE(test, len, RAID6_KUNIT_ALLOC_BYTES);
> +
> + preempt_disable();
> + t = ktime_get_ns();
> + for (l = 0; l < num_iters; l++)
> + raid6_gen_syndrome(nr_to_test[i],
> + len_to_test[j], test_buffers);
> + t = ktime_get_ns() - t;
> + preempt_enable();
[Severity: Medium]
Could this unbounded atomic section cause soft lockups or RCU stalls on
slower architectures or emulators?
The loop executes 1,000 iterations of raid6_gen_syndrome() with preemption
disabled. On slow hardware or CI emulators, processing this much data might
take multiple seconds and prevent the scheduler from running.
> +
> + speed[j] = div64_u64((u64)len * num_iters * nr, t);
[Severity: Medium]
Is there a risk of a division by zero panic here?
If the KUnit test runs on a system with a coarse-resolution clock (such as
with CONFIG_HIGH_RES_TIMERS disabled) and a sufficiently fast CPU, the
1,000 iterations might complete in less time than a single clock tick.
In this scenario, ktime_get_ns() could return the same value before and
after the loop, causing the elapsed time t to evaluate to exactly 0 before
being passed as the divisor.
> + }
> +
> + static_assert(ARRAY_SIZE(len_to_test) == 2);
> + kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
> + nr, speed[0], speed[1]);
> + }
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=8
^ permalink raw reply
* Re: [PATCH 7/8] raid6: improve the runtime selection benchmark
From: sashiko-bot @ 2026-07-08 9:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, yukuai
In-Reply-To: <20260708090740.1433685-8-hch@lst.de>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Potential division by zero kernel panic when calculating benchmark throughput if the elapsed time `t` evaluates to 0.
- [Medium] The reported throughput for the XOR algorithm is artificially inflated by a factor of 2.
- [Low] The benchmark inner loop variable `int i` redundantly shadows the outer function-scoped variable `unsigned int i`.
--
--- Patch [7]: [PATCH 7/8] raid6: improve the runtime selection benchmark ---
commit ae782577f509b50023ef73d6d96f6fe2f366a500
Author: Christoph Hellwig <hch@lst.de>
raid6: improve the runtime selection benchmark
Use plain ktime_get_ns for the timing, use 8 + 2 disks for a realistic
load, and report the throughput on the data disks as that is what
storage systems are measured on.
> diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
> index e7984bde5157f..c44b986e319e3 100644
> --- a/lib/raid/raid6/algos.c
> +++ b/lib/raid/raid6/algos.c
[ ... ]
> @@ -152,40 +152,39 @@ void raid6_recov_datap(int disks, size_t bytes, int faila, void **ptrs)
> }
> EXPORT_SYMBOL_GPL(raid6_recov_datap);
>
> -#define RAID6_TIME_JIFFIES_LG2 4
> -#define RAID6_TEST_DISKS 8
> +#define BENCH_SIZE SZ_4K
> +#define NR_SRCS 8
> +#define NR_DISKS (NR_SRCS + 2)
> +#define REPS 800U
>
> -static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
> - const int disks)
> +static int raid6_choose_gen(void *dptrs[NR_DISKS], const int disks)
> {
> /* work on the second half of the disks */
> - int start = (disks >> 1) - 1, stop = disks - 3;
> + int start = (disks / 2) - 1, stop = disks - 3;
> const struct raid6_calls *best = NULL;
> unsigned long bestgenperf = 0;
> unsigned int i;
>
> for (i = 0; i < raid6_nr_algos; i++) {
> const struct raid6_calls *algo = raid6_algos[i];
> - unsigned long perf = 0, j0, j1;
> + unsigned long perf = 0;
> + u64 t;
> + int i;
[Severity: Low]
This isn't a bug, but `int i` declared here shadows the function-scoped
`unsigned int i` used for the outer loop.
>
> preempt_disable();
> - j0 = jiffies;
> - while ((j1 = jiffies) == j0)
> - cpu_relax();
> - while (time_before(jiffies,
> - j1 + (1<<RAID6_TIME_JIFFIES_LG2))) {
> - algo->gen_syndrome(disks, PAGE_SIZE, *dptrs);
> - perf++;
> - }
> + t = ktime_get_ns();
> + for (i = 0; i < REPS; i++)
> + algo->gen_syndrome(disks, BENCH_SIZE, dptrs);
> + t = ktime_get_ns() - t;
> preempt_enable();
>
> + /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
> + perf = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
[Severity: High]
Is it possible for `t` to be 0 here, leading to a division by zero panic?
If the system uses a coarse clock source (like jiffies during early boot before
high-resolution timers are initialized) and the benchmark iterations complete
within the same timer tick, `t` might evaluate to 0.
[ ... ]
> @@ -197,28 +196,22 @@ static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
> static_call_update(raid6_xor_syndrome_impl, best->xor_syndrome);
>
> pr_info("raid6: using algorithm %s gen() %ld MB/s\n",
> - best->name,
> - (bestgenperf * HZ * (disks - 2)) >>
> - (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2));
> + best->name, bestgenperf);
>
> if (best->xor_syndrome) {
> - unsigned long perf = 0, j0, j1;
> + u64 t;
> + int i;
[Severity: Low]
This isn't a bug, but `int i` declared here also shadows the function-scoped
`unsigned int i`.
>
> preempt_disable();
> - j0 = jiffies;
> - while ((j1 = jiffies) == j0)
> - cpu_relax();
> - while (time_before(jiffies,
> - j1 + (1 << RAID6_TIME_JIFFIES_LG2))) {
> - best->xor_syndrome(disks, start, stop,
> - PAGE_SIZE, *dptrs);
> - perf++;
> - }
> + t = ktime_get_ns();
> + for (i = 0; i < REPS; i++)
> + best->xor_syndrome(disks, start, stop, BENCH_SIZE,
> + dptrs);
> + t = ktime_get_ns() - t;
> preempt_enable();
>
> - pr_info("raid6: .... xor() %ld MB/s, rmw enabled\n",
> - (perf * HZ * (disks - 2)) >>
> - (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2 + 1));
> + pr_info("raid6: .... xor() %llu MB/s, rmw enabled\n",
> + div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t));
[Severity: Medium]
Does this calculation artificially inflate the reported throughput for the XOR
algorithm by a factor of 2?
The `xor_syndrome` benchmark operates on a subset of disks from `start` to
`stop` (which is `(disks / 2) - 1` through `disks - 3`). For the default
`NR_DISKS` (10), this processes 4 data disks. However, the throughput
calculation multiplies by `NR_SRCS` (8) instead of the actual number of disks
processed. The previous implementation correctly adjusted for this using an
extra bit shift (`+ 1`), which was dropped in this patch.
[Severity: High]
Like the calculation above, could `t` be 0 here leading to a division by zero
panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=7
^ permalink raw reply
* Re: [PATCH 2/8] xor: improve the runtime selection benchmark
From: sashiko-bot @ 2026-07-08 9:18 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, yukuai
In-Reply-To: <20260708090740.1433685-3-hch@lst.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A division by zero kernel panic will occur during boot on systems using a coarse clocksource when calculating the benchmark speed.
--
--- Patch [2]: [PATCH 2/8] xor: improve the runtime selection benchmark ---
commit 69c8bf90e6d6fa234ebb61b5b5b7c4dfd2d8e9c1
Author: Christoph Hellwig <hch@lst.de>
xor: improve the runtime selection benchmark
This commit updates the XOR benchmark to use ktime_get_ns() for timing,
increases the workload to 4 source disks, and calculates throughput based
on the data disks.
> diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
> --- a/lib/raid/xor/xor-core.c
> +++ b/lib/raid/xor/xor-core.c
[ ... ]
> @@ -73,59 +72,56 @@ void __init xor_force(struct xor_block_template *tmpl)
[ ... ]
> -static void __init
> -do_xor_speed(struct xor_block_template *tmpl, void *b1, void *b2)
> +static void __init do_xor_speed(struct xor_block_template *tmpl, void *dest,
> + void *srcs[NR_SRCS])
> {
[ ... ]
> + t = ktime_get_ns();
> + for (i = 0; i < REPS; i++) {
> mb(); /* prevent loop optimization */
> - tmpl->xor_gen(b1, srcs, 1, BENCH_SIZE);
> + tmpl->xor_gen(dest, srcs, NR_SRCS, BENCH_SIZE);
> mb();
> - } while (reps++ < REPS || (t0 = ktime_get()) == start);
> - min = ktime_sub(t0, start);
> -
> + }
> + t = ktime_get_ns() - t;
> preempt_enable();
>
> - // bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
> - speed = (1000 * reps * BENCH_SIZE) / (unsigned int)ktime_to_ns(min);
> - tmpl->speed = speed;
> + /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
> + tmpl->speed = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
[Severity: High]
Could this cause a division by zero panic during boot on systems with a
coarse clocksource?
If the 800 loop iterations execute very quickly (within the same clock tick)
on a platform without high-resolution timers, or during early boot phases
where jiffies are used as the clocksource, the elapsed time t will evaluate
to 0.
The previous code avoided this by looping until the clock advanced before
and after the benchmark:
while ((start = ktime_get()) == t0)
Since t is now used directly as the divisor in div64_u64(), a 0 value will
trigger an unhandled divide-by-zero hardware exception.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=2
^ permalink raw reply
* [PATCH 8/8] raid6/kunit: add a benchmark
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Add a benchmark to test the XOR functions for more representative block
sizes and numbers of disks. This splits the maximum alloc size used for
the benchmark from the max size for the kunit test because recovery
is currently limited to a single page at a time. Hopefully this will be
fixed soon.
The runtime numbers are reported in GB/s as the numbers of modern
implementations are basically unreadable as MB/s. This means
retro-architectures could report 0, but that is an easy tradeoff.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/Kconfig | 9 ++++
lib/raid/raid6/tests/raid6_kunit.c | 66 +++++++++++++++++++++++++++++-
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index c4eeb7c716c8..c52d5c205294 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -60,6 +60,15 @@ config RAID6_PQ_KUNIT_TEST
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
+config RAID6_PQ_KUNIT_BENCHMARK
+ bool "Benchmark for RAID6 PQ"
+ depends on RAID6_PQ_KUNIT_TEST
+ help
+ Include benchmarks in the KUnit test suite for raid P/Q generation.
+
+config RAID6_PQ
+ tristate
+
config RAID6_PQ_BENCHMARK
bool "Automatically choose fastest RAID6 PQ functions"
depends on RAID6_PQ
diff --git a/lib/raid/raid6/tests/raid6_kunit.c b/lib/raid/raid6/tests/raid6_kunit.c
index 9f3e671a1224..f927e3456d17 100644
--- a/lib/raid/raid6/tests/raid6_kunit.c
+++ b/lib/raid/raid6/tests/raid6_kunit.c
@@ -18,6 +18,7 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
#define RAID6_KUNIT_MAX_BUFFERS 64 /* Including P and Q */
#define RAID6_KUNIT_MAX_FAILURES 2
#define RAID6_KUNIT_MAX_BYTES PAGE_SIZE
+#define RAID6_KUNIT_ALLOC_BYTES SZ_16K
static struct rnd_state rng;
static void *test_buffers[RAID6_KUNIT_MAX_BUFFERS];
@@ -229,6 +230,68 @@ static void raid6_test(struct kunit *test)
raid6_test_one(test);
}
+static void raid6_benchmark(struct kunit *test)
+{
+ static const unsigned int nr_to_test[] = {
+ 4, 5, 6, 7, 8, 10, 12, 15, 16, 32,
+ };
+ static const unsigned int len_to_test[] = {
+ SZ_4K, SZ_16K,
+ };
+ unsigned int i, j, l;
+ u64 t;
+
+ if (!IS_ENABLED(CONFIG_RAID6_PQ_KUNIT_BENCHMARK))
+ kunit_skip(test, "not enabled");
+
+ /* warm-up */
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ for (l = 0; l < 10; l++) {
+ raid6_gen_syndrome(nr_to_test[i],
+ len_to_test[j], test_buffers);
+ }
+ }
+ }
+
+ /*
+ * Preferably this would be a loop over len_to_test, but the kunit
+ * logging always adds a newline to each logged format string.
+ */
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, " \t%5u bytes\t%5u bytes\n",
+ len_to_test[0], len_to_test[1]);
+
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ unsigned int nr = nr_to_test[i];
+ u64 speed[ARRAY_SIZE(len_to_test)];
+
+ KUNIT_ASSERT_LE(test, nr, RAID6_KUNIT_MAX_BUFFERS);
+
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ unsigned int len = len_to_test[j];
+ const unsigned long num_iters = 1000;
+
+ KUNIT_ASSERT_GT(test, len, 0);
+ KUNIT_ASSERT_LE(test, len, RAID6_KUNIT_ALLOC_BYTES);
+
+ preempt_disable();
+ t = ktime_get_ns();
+ for (l = 0; l < num_iters; l++)
+ raid6_gen_syndrome(nr_to_test[i],
+ len_to_test[j], test_buffers);
+ t = ktime_get_ns() - t;
+ preempt_enable();
+
+ speed[j] = div64_u64((u64)len * num_iters * nr, t);
+ }
+
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
+ nr, speed[0], speed[1]);
+ }
+}
+
static const void *raid6_gen_params(struct kunit *test, const void *prev,
char *desc)
{
@@ -256,6 +319,7 @@ static const void *raid6_gen_params(struct kunit *test, const void *prev,
static struct kunit_case raid6_test_cases[] = {
KUNIT_CASE_PARAM(raid6_test, raid6_gen_params),
+ KUNIT_CASE(raid6_benchmark),
{},
};
@@ -270,7 +334,7 @@ static int raid6_suite_init(struct kunit_suite *suite)
* so that it is immediately followed by a guard page. This allows
* buffer overreads to be detected, even in assembly code.
*/
- test_buflen = round_up(RAID6_KUNIT_MAX_BYTES, PAGE_SIZE);
+ test_buflen = round_up(RAID6_KUNIT_ALLOC_BYTES, PAGE_SIZE);
for (i = 0; i < RAID6_KUNIT_MAX_FAILURES; i++) {
test_recov_buffers[i] = vmalloc(test_buflen);
if (!test_recov_buffers[i])
--
2.53.0
^ permalink raw reply related
* [PATCH 7/8] raid6: improve the runtime selection benchmark
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Use plain ktime_get_ns for the timing, use 8 + 2 disks for a realistic
load, and report the throughput on the data disks as that is what
storage systems are measured on.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/algos.c | 75 +++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 41 deletions(-)
diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index e7984bde5157..c44b986e319e 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -152,40 +152,39 @@ void raid6_recov_datap(int disks, size_t bytes, int faila, void **ptrs)
}
EXPORT_SYMBOL_GPL(raid6_recov_datap);
-#define RAID6_TIME_JIFFIES_LG2 4
-#define RAID6_TEST_DISKS 8
+#define BENCH_SIZE SZ_4K
+#define NR_SRCS 8
+#define NR_DISKS (NR_SRCS + 2)
+#define REPS 800U
-static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
- const int disks)
+static int raid6_choose_gen(void *dptrs[NR_DISKS], const int disks)
{
/* work on the second half of the disks */
- int start = (disks >> 1) - 1, stop = disks - 3;
+ int start = (disks / 2) - 1, stop = disks - 3;
const struct raid6_calls *best = NULL;
unsigned long bestgenperf = 0;
unsigned int i;
for (i = 0; i < raid6_nr_algos; i++) {
const struct raid6_calls *algo = raid6_algos[i];
- unsigned long perf = 0, j0, j1;
+ unsigned long perf = 0;
+ u64 t;
+ int i;
preempt_disable();
- j0 = jiffies;
- while ((j1 = jiffies) == j0)
- cpu_relax();
- while (time_before(jiffies,
- j1 + (1<<RAID6_TIME_JIFFIES_LG2))) {
- algo->gen_syndrome(disks, PAGE_SIZE, *dptrs);
- perf++;
- }
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++)
+ algo->gen_syndrome(disks, BENCH_SIZE, dptrs);
+ t = ktime_get_ns() - t;
preempt_enable();
+ /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
+ perf = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
if (perf > bestgenperf) {
bestgenperf = perf;
best = algo;
}
- pr_info("raid6: %-8s gen() %5ld MB/s\n", algo->name,
- (perf * HZ * (disks-2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2));
+ pr_info("raid6: %-8s gen() %5lu MB/s\n", algo->name, perf);
}
if (!best) {
@@ -197,28 +196,22 @@ static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
static_call_update(raid6_xor_syndrome_impl, best->xor_syndrome);
pr_info("raid6: using algorithm %s gen() %ld MB/s\n",
- best->name,
- (bestgenperf * HZ * (disks - 2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2));
+ best->name, bestgenperf);
if (best->xor_syndrome) {
- unsigned long perf = 0, j0, j1;
+ u64 t;
+ int i;
preempt_disable();
- j0 = jiffies;
- while ((j1 = jiffies) == j0)
- cpu_relax();
- while (time_before(jiffies,
- j1 + (1 << RAID6_TIME_JIFFIES_LG2))) {
- best->xor_syndrome(disks, start, stop,
- PAGE_SIZE, *dptrs);
- perf++;
- }
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++)
+ best->xor_syndrome(disks, start, stop, BENCH_SIZE,
+ dptrs);
+ t = ktime_get_ns() - t;
preempt_enable();
- pr_info("raid6: .... xor() %ld MB/s, rmw enabled\n",
- (perf * HZ * (disks - 2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2 + 1));
+ pr_info("raid6: .... xor() %llu MB/s, rmw enabled\n",
+ div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t));
}
return 0;
@@ -230,14 +223,14 @@ static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
static int __init raid6_select_algo(void)
{
- const int disks = RAID6_TEST_DISKS;
+ const int disks = NR_DISKS;
+ void *dptrs[NR_DISKS];
char *disk_ptr, *p;
- void *dptrs[RAID6_TEST_DISKS];
int i, cycle;
int error;
/* prepare the buffer and fill it circularly with gfmul table */
- disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
+ disk_ptr = kmalloc_array(NR_DISKS, BENCH_SIZE, GFP_KERNEL);
if (!disk_ptr) {
pr_err("raid6: Yikes! No memory available.\n");
return -ENOMEM;
@@ -245,19 +238,19 @@ static int __init raid6_select_algo(void)
p = disk_ptr;
for (i = 0; i < disks; i++)
- dptrs[i] = p + PAGE_SIZE * i;
+ dptrs[i] = p + BENCH_SIZE * i;
- cycle = ((disks - 2) * PAGE_SIZE) / 65536;
+ cycle = ((disks - 2) * BENCH_SIZE) / 65536;
for (i = 0; i < cycle; i++) {
memcpy(p, raid6_gfmul, 65536);
p += 65536;
}
- if ((disks - 2) * PAGE_SIZE % 65536)
- memcpy(p, raid6_gfmul, (disks - 2) * PAGE_SIZE % 65536);
+ if ((disks - 2) * BENCH_SIZE % 65536)
+ memcpy(p, raid6_gfmul, (disks - 2) * BENCH_SIZE % 65536);
/* select raid gen_syndrome function */
- error = raid6_choose_gen(&dptrs, disks);
+ error = raid6_choose_gen(dptrs, disks);
kfree(disk_ptr);
--
2.53.0
^ permalink raw reply related
* [PATCH 6/8] raid6: defer implementation selection when built-in
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Don't hold up early boot and defer the selection just like we've been
doing for the RAID5 XOR code since commit 524ccdbdfb52 ("crypto: xor -
defer load time benchmark to a later time").
This will also allow full use of the timing subsystem for benchmarking.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/algos.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index 6f5c89ab2b17..e7984bde5157 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -236,16 +236,6 @@ static int __init raid6_select_algo(void)
int i, cycle;
int error;
- if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
- pr_info("raid6: skipped pq benchmark and selected %s\n",
- raid6_algos[raid6_nr_algos - 1]->name);
- static_call_update(raid6_gen_syndrome_impl,
- raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
- static_call_update(raid6_xor_syndrome_impl,
- raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
- return 0;
- }
-
/* prepare the buffer and fill it circularly with gfmul table */
disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
if (!disk_ptr) {
@@ -329,13 +319,40 @@ static int __init raid6_init(void)
static_call_update(raid6_recov_datap_impl, raid6_recov_algo->datap);
pr_info("raid6: using %s recovery algorithm\n", raid6_recov_algo->name);
+ if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
+ pr_info("raid6: skipped pq benchmark and selected %s\n",
+ raid6_algos[raid6_nr_algos - 1]->name);
+ goto default_algo;
+ }
+
+#ifdef MODULE
return raid6_select_algo();
+#else
+ /*
+ * Pick the last registered implementation as the temporary default until
+ * calibration happens.
+ */
+#endif
+default_algo:
+ static_call_update(raid6_gen_syndrome_impl,
+ raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
+ static_call_update(raid6_xor_syndrome_impl,
+ raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
+ return 0;
}
static void __exit raid6_exit(void)
{
}
+/*
+ * When built-in we must register the default implementation before md
+ * initializes, but we don't want calibration to run that early as that
+ * would delay the boot process.
+ */
+#ifndef MODULE
+device_initcall(raid6_select_algo);
+#endif
subsys_initcall(raid6_init);
module_exit(raid6_exit);
MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related
* [PATCH 5/8] raid6: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/raid/raid6/Makefile b/lib/raid/raid6/Makefile
index 038d6c74d1ba..5cdb7223de2a 100644
--- a/lib/raid/raid6/Makefile
+++ b/lib/raid/raid6/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
ifeq ($(CONFIG_RAID6_PQ_ARCH),y)
--
2.53.0
^ permalink raw reply related
* [PATCH 4/8] xor/kunit: add a benchmark
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Add a benchmark to test the XOR functions for more representative block
sizes and numbers of disks. Including 64k would be useful here, but
increasing the test buffer size increases the runtime of the functional
kunit test too much unfortunately.
The runtime numbers are reported in GB/s as the numbers of modern
implementations are basically unreadable as MB/s. This means
retro-architectures could report 0, but that is an easy tradeoff.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/Kconfig | 6 ++++
lib/raid/xor/tests/xor_kunit.c | 62 ++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index 978cd6ba08ac..c4eeb7c716c8 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -29,6 +29,12 @@ config XOR_KUNIT_TEST
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
+config XOR_BENCHMARK
+ bool "Benchmark for xor_gen"
+ depends on XOR_KUNIT_TEST
+ help
+ Include benchmarks in the KUnit test suite for xor_gen.
+
config RAID6_PQ
tristate
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 659ae3edbc25..648c6da9464c 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -125,8 +125,70 @@ static void xor_test(struct kunit *test)
}
}
+static void xor_benchmark(struct kunit *test)
+{
+ static const unsigned int nr_to_test[] = {
+ 4, 5, 6, 7, 8, 10, 12, 15, 16, 32,
+ };
+ static const unsigned int len_to_test[] = {
+ SZ_4K, SZ_16K,
+ };
+ unsigned int i, j, l;
+ u64 t;
+
+ if (!IS_ENABLED(CONFIG_XOR_BENCHMARK))
+ kunit_skip(test, "not enabled");
+
+ /* warm-up */
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ for (l = 0; l < 10; l++) {
+ xor_gen(test_dest, test_buffers, nr_to_test[i],
+ len_to_test[j]);
+ }
+ }
+ }
+
+ /*
+ * Preferably this would be a loop over len_to_test, but the kunit
+ * logging always adds a newline to each logged format string.
+ */
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, " \t%5u bytes\t%5u bytes\n",
+ len_to_test[0], len_to_test[1]);
+
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ unsigned int nr = nr_to_test[i];
+ u64 speed[ARRAY_SIZE(len_to_test)];
+
+ KUNIT_ASSERT_LE(test, nr, XOR_KUNIT_MAX_BUFFERS);
+
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ unsigned int len = len_to_test[j];
+ const unsigned long num_iters = 1000;
+
+ KUNIT_ASSERT_GT(test, len, 0);
+ KUNIT_ASSERT_LE(test, len, XOR_KUNIT_MAX_BYTES);
+
+ preempt_disable();
+ t = ktime_get_ns();
+ for (l = 0; l < num_iters; l++)
+ xor_gen(test_dest, test_buffers, nr, len);
+ t = ktime_get_ns() - t;
+ preempt_enable();
+
+ speed[j] = div64_u64((u64)len * num_iters * nr, t);
+ }
+
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
+ nr, speed[0], speed[1]);
+ }
+}
+
static struct kunit_case xor_test_cases[] = {
KUNIT_CASE(xor_test),
+ KUNIT_CASE(xor_benchmark),
{},
};
--
2.53.0
^ permalink raw reply related
* [PATCH 3/8] xor/kunit: fix a spelling error
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/tests/xor_kunit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 0c2a3a420bf9..659ae3edbc25 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -85,7 +85,7 @@ static void xor_test(struct kunit *test)
xor_generate_random_data();
/*
- * If we're not using the entire buffer size, inject randomize
+ * If we're not using the entire buffer size, inject randomized
* alignment into the buffer.
*/
max_alignment = XOR_KUNIT_MAX_BYTES - len;
--
2.53.0
^ permalink raw reply related
* [PATCH 2/8] xor: improve the runtime selection benchmark
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Use plain ktime_get_ns for the timing, use 4 + 1 disks for a realistic
load, and report the throughput on the data disks instead of the that
on the parity disk, which isn't all that useful.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/xor-core.c | 55 +++++++++++++++++++----------------------
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
index 50931fbf0324..a890cec21bbc 100644
--- a/lib/raid/xor/xor-core.c
+++ b/lib/raid/xor/xor-core.c
@@ -10,7 +10,6 @@
#include <linux/gfp.h>
#include <linux/slab.h>
#include <linux/raid/xor.h>
-#include <linux/jiffies.h>
#include <linux/preempt.h>
#include <linux/static_call.h>
#include "xor_impl.h"
@@ -73,59 +72,56 @@ void __init xor_force(struct xor_block_template *tmpl)
forced_template = tmpl;
}
-#define BENCH_SIZE 4096
+#define BENCH_SIZE SZ_4K
+#define NR_SRCS 4
#define REPS 800U
-static void __init
-do_xor_speed(struct xor_block_template *tmpl, void *b1, void *b2)
+static void __init do_xor_speed(struct xor_block_template *tmpl, void *dest,
+ void *srcs[NR_SRCS])
{
- int speed;
- unsigned long reps;
- ktime_t min, start, t0;
- void *srcs[1] = { b2 };
+ u64 t;
+ int i;
preempt_disable();
-
- reps = 0;
- t0 = ktime_get();
- /* delay start until time has advanced */
- while ((start = ktime_get()) == t0)
- cpu_relax();
- do {
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++) {
mb(); /* prevent loop optimization */
- tmpl->xor_gen(b1, srcs, 1, BENCH_SIZE);
+ tmpl->xor_gen(dest, srcs, NR_SRCS, BENCH_SIZE);
mb();
- } while (reps++ < REPS || (t0 = ktime_get()) == start);
- min = ktime_sub(t0, start);
-
+ }
+ t = ktime_get_ns() - t;
preempt_enable();
- // bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
- speed = (1000 * reps * BENCH_SIZE) / (unsigned int)ktime_to_ns(min);
- tmpl->speed = speed;
+ /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
+ tmpl->speed = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
- pr_info(" %-16s: %5d MB/sec\n", tmpl->name, speed);
+ pr_info(" %-16s: %5d MB/sec\n", tmpl->name, tmpl->speed);
}
static int __init calibrate_xor_blocks(void)
{
- void *b1, *b2;
struct xor_block_template *f, *fastest;
+ void *srcs[NR_SRCS];
+ void *buf, *dest;
+ int i;
if (forced_template)
return 0;
- b1 = kmalloc(PAGE_SIZE * 4, GFP_KERNEL);
- if (!b1) {
+ buf = kmalloc(BENCH_SIZE * (NR_SRCS + 1), GFP_KERNEL);
+ if (!buf) {
pr_warn("xor: Yikes! No memory available.\n");
return -ENOMEM;
}
- b2 = b1 + 2*PAGE_SIZE + BENCH_SIZE;
+ get_random_bytes(buf, BENCH_SIZE * (NR_SRCS + 1));
+ dest = buf;
+ for (i = 0; i < NR_SRCS; i++)
+ srcs[i] = buf + (i + 1) * BENCH_SIZE;
pr_info("xor: measuring software checksum speed\n");
fastest = template_list;
for (f = template_list; f; f = f->next) {
- do_xor_speed(f, b1, b2);
+ do_xor_speed(f, dest, srcs);
if (f->speed > fastest->speed)
fastest = f;
}
@@ -133,9 +129,10 @@ static int __init calibrate_xor_blocks(void)
pr_info("xor: using function: %s (%d MB/sec)\n",
fastest->name, fastest->speed);
- kfree(b1);
+ kfree(buf);
return 0;
}
+#undef NR_SRCS
#ifdef CONFIG_XOR_BLOCKS_ARCH
#include "xor_arch.h" /* $SRCARCH/xor_arch.h */
--
2.53.0
^ permalink raw reply related
* [PATCH 1/8] xor: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/Makefile | 2 ++
lib/raid/xor/tests/Makefile | 2 ++
2 files changed, 4 insertions(+)
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index e8ecec3c09f9..9b0fad459cdb 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
obj-$(CONFIG_XOR_BLOCKS) += xor.o
diff --git a/lib/raid/xor/tests/Makefile b/lib/raid/xor/tests/Makefile
index 661e8f6ffd1f..1cce833cd7fd 100644
--- a/lib/raid/xor/tests/Makefile
+++ b/lib/raid/xor/tests/Makefile
@@ -1,3 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_XOR_KUNIT_TEST) += xor_kunit.o
--
2.53.0
^ permalink raw reply related
* misc lib/raid/ improvements
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
Hi all,
this series improves the benchmark-based algorithm selection and
adds kunit benchmarks for both the XOR and raid6 libraries, and tidies
up a few very minor other bits.
diffstat:
lib/raid/Kconfig | 15 ++++++
lib/raid/raid6/Makefile | 2
lib/raid/raid6/algos.c | 112 +++++++++++++++++++++++++---------------------
lib/raid/raid6/tests/raid6_kunit.c | 66 ++++++++++++++++++++++++++-
lib/raid/xor/Makefile | 2
lib/raid/xor/tests/Makefile | 2
lib/raid/xor/tests/xor_kunit.c | 64 +++++++++++++++++++++++++-
lib/raid/xor/xor-core.c | 55 ++++++++++------------
8 files changed, 236 insertions(+), 82 deletions(-)
^ permalink raw reply
* [PATCH 6/8] raid6: defer implementation selection when built-in
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
Don't hold up early boot and defer the selection just like we've been
doing for the RAID5 XOR code since commit 524ccdbdfb52 ("crypto: xor -
defer load time benchmark to a later time").
This will also allow full use of the timing subsystem for benchmarking.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/algos.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index 6f5c89ab2b17..e7984bde5157 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -236,16 +236,6 @@ static int __init raid6_select_algo(void)
int i, cycle;
int error;
- if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
- pr_info("raid6: skipped pq benchmark and selected %s\n",
- raid6_algos[raid6_nr_algos - 1]->name);
- static_call_update(raid6_gen_syndrome_impl,
- raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
- static_call_update(raid6_xor_syndrome_impl,
- raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
- return 0;
- }
-
/* prepare the buffer and fill it circularly with gfmul table */
disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
if (!disk_ptr) {
@@ -329,13 +319,40 @@ static int __init raid6_init(void)
static_call_update(raid6_recov_datap_impl, raid6_recov_algo->datap);
pr_info("raid6: using %s recovery algorithm\n", raid6_recov_algo->name);
+ if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
+ pr_info("raid6: skipped pq benchmark and selected %s\n",
+ raid6_algos[raid6_nr_algos - 1]->name);
+ goto default_algo;
+ }
+
+#ifdef MODULE
return raid6_select_algo();
+#else
+ /*
+ * Pick the last registered implementation as the temporary default until
+ * calibration happens.
+ */
+#endif
+default_algo:
+ static_call_update(raid6_gen_syndrome_impl,
+ raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
+ static_call_update(raid6_xor_syndrome_impl,
+ raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
+ return 0;
}
static void __exit raid6_exit(void)
{
}
+/*
+ * When built-in we must register the default implementation before md
+ * initializes, but we don't want calibration to run that early as that
+ * would delay the boot process.
+ */
+#ifndef MODULE
+device_initcall(raid6_select_algo);
+#endif
subsys_initcall(raid6_init);
module_exit(raid6_exit);
MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] md/raid10: free r10_bio on write NOWAIT exits
From: sashiko-bot @ 2026-07-08 9:06 UTC (permalink / raw)
To: Guangshuo Li; +Cc: linux-raid, yukuai
In-Reply-To: <20260708084701.737763-1-lgs201920130244@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The MD layer and RAID10 driver sleep in multiple paths while handling a `REQ_NOWAIT` bio, violating the non-blocking semantics of the block layer.
- [High] The driver unconditionally uses `GFP_NOIO` for mempool and bio allocations, which can sleep when memory is low, violating `REQ_NOWAIT` non-blocking semantics.
--
--- Patch [1]: [PATCH] md/raid10: free r10_bio on write NOWAIT exits ---
commit 2e30580bb5bb2ae342c9d924bf8c217f5b6c832f
Author: Guangshuo Li <lgs201920130244@gmail.com>
md/raid10: free r10_bio on write NOWAIT exits
This commit fixes a memory leak in raid10_write_request() where the r10_bio
structure was not returned to the mempool on two REQ_NOWAIT early exit
paths. It ensures free_r10bio() is called before returning false for these
NOWAIT cases.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df8..d480344c5b8f3 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1364,6 +1364,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> DEFINE_WAIT(w);
> /* Bail out if REQ_NOWAIT is set for the bio */
> if (bio->bi_opf & REQ_NOWAIT) {
> + free_r10bio(r10_bio);
> bio_wouldblock_error(bio);
> return false;
> }
[ ... ]
> @@ -1397,6 +1398,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> md_wakeup_thread(mddev->thread);
> if (bio->bi_opf & REQ_NOWAIT) {
> allow_barrier(conf);
> + free_r10bio(r10_bio);
> bio_wouldblock_error(bio);
> return false;
> }
[Severity: High]
This isn't a bug introduced by this patch, but are there other blocking
operations in the NOWAIT path that could still violate the non-blocking
semantics?
If a REQ_NOWAIT bio does not hit these early exits, it proceeds down to
wait_blocked_dev() later in raid10_write_request(), which does not check
for REQ_NOWAIT:
raid10_find_phys(conf, r10_bio);
wait_blocked_dev(mddev, r10_bio);
If a member device is blocked, wait_blocked_dev() drops the barrier and
sleeps in md_wait_for_blocked_rdev() (which has a 5-second timeout) and
then sleeps again in wait_barrier(conf, false).
Additionally, if the bio crosses a chunk boundary, it reacquires the
barrier without checking for REQ_NOWAIT:
if (r10_bio->sectors < bio_sectors(bio)) {
allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
wait_barrier(conf, false);
Could these unconditional sleeps cause the thread submitting the async I/O
to stall, potentially exhausting io_uring worker threads?
[Severity: High]
This is also a pre-existing issue, but does the driver unconditionally use
GFP_NOIO for allocations when handling a REQ_NOWAIT bio?
Before raid10_write_request() is even called, __make_request() allocates
the primary r10_bio using GFP_NOIO:
r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
Later, raid10_write_one_disk() clones the bio unconditionally using
GFP_NOIO:
mbio = bio_alloc_clone(rdev->bdev, bio, GFP_NOIO, &mddev->bio_set);
If the mempool is depleted and memory pressure requires direct reclaim,
won't these GFP_NOIO allocations sleep and stall the IO submission thread?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708084701.737763-1-lgs201920130244@gmail.com?part=1
^ permalink raw reply
* [PATCH 5/8] raid6: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/raid/raid6/Makefile b/lib/raid/raid6/Makefile
index 038d6c74d1ba..5cdb7223de2a 100644
--- a/lib/raid/raid6/Makefile
+++ b/lib/raid/raid6/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
ifeq ($(CONFIG_RAID6_PQ_ARCH),y)
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox