Dear Zheng,
Thank you for the patch.
Am 12.06.25 um 15:21 schrieb Zheng Qixing:
From: Zheng Qixing <zhengqixing@xxxxxxxxxx>
IO with REQ_NOWAIT should not set R1BIO_Uptodate when it fails,
and bad blocks should also be cleared when REQ_NOWAIT IO succeeds.
It’d be great if you could add an explanation for the *should*. Why
should it not be done?
Do you have a reproducer for this?
Fixes: 9f346f7d4ea7 ("md/raid1,raid10: don't handle IO error for REQ_RAHEAD and REQ_NOWAIT")
Signed-off-by: Zheng Qixing <zhengqixing@xxxxxxxxxx>
---
drivers/md/raid1.c | 11 ++++++-----
drivers/md/raid10.c | 9 +++++----
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 19c5a0ce5a40..a1cddd24b178 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -455,13 +455,13 @@ static void raid1_end_write_request(struct bio *bio)
struct md_rdev *rdev = conf->mirrors[mirror].rdev;
sector_t lo = r1_bio->sector;
sector_t hi = r1_bio->sector + r1_bio->sectors;
- bool ignore_error = !raid1_should_handle_error(bio) ||
- (bio->bi_status && bio_op(bio) == REQ_OP_DISCARD);
+ bool discard_error = bio->bi_status && bio_op(bio) == REQ_OP_DISCARD;
Excuse my ignorance. What is the difference between ignore and discard?
/*
* 'one mirror IO has finished' event handler:
*/
- if (bio->bi_status && !ignore_error) {
+ if (bio->bi_status && !discard_error &&
+ raid1_should_handle_error(bio)) {
set_bit(WriteErrorSeen, &rdev->flags);
if (!test_and_set_bit(WantReplacement, &rdev->flags))
set_bit(MD_RECOVERY_NEEDED, &
@@ -507,12 +507,13 @@ static void raid1_end_write_request(struct bio *bio)
* check this here.
*/
if (test_bit(In_sync, &rdev->flags) &&
- !test_bit(Faulty, &rdev->flags))
+ !test_bit(Faulty, &rdev->flags) &&
+ (!bio->bi_status || discard_error))
set_bit(R1BIO_Uptodate, &r1_bio->state);
/* Maybe we can clear some bad blocks. */
if (rdev_has_badblock(rdev, r1_bio->sector, r1_bio->sectors) &&
- !ignore_error) {
+ !bio->bi_status) {
r1_bio->bios[mirror] = IO_MADE_GOOD;
set_bit(R1BIO_MadeGood, &r1_bio->state);
}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index b74780af4c22..1848947b0a6d 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -458,8 +458,8 @@ static void raid10_end_write_request(struct bio *bio)
int slot, repl;
struct md_rdev *rdev = NULL;
struct bio *to_put = NULL;
- bool ignore_error = !raid1_should_handle_error(bio) ||
- (bio->bi_status && bio_op(bio) == REQ_OP_DISCARD);
+ bool discard_error = bio->bi_status && bio_op(bio) == REQ_OP_DISCARD;
+ bool ignore_error = !raid1_should_handle_error(bio) || discard_error;
dev = find_bio_disk(conf, r10_bio, bio, &slot, &repl);
@@ -522,13 +522,14 @@ static void raid10_end_write_request(struct bio *bio)
* check this here.
*/
if (test_bit(In_sync, &rdev->flags) &&
- !test_bit(Faulty, &rdev->flags))
+ !test_bit(Faulty, &rdev->flags) &&
+ (!bio->bi_status || discard_error))
set_bit(R10BIO_Uptodate, &r10_bio->state);
/* Maybe we can clear some bad blocks. */
if (rdev_has_badblock(rdev, r10_bio->devs[slot].addr,
r10_bio->sectors) &&
- !ignore_error) {
+ !bio->bi_status) {
bio_put(bio);
if (repl)
r10_bio->devs[slot].repl_bio = IO_MADE_GOOD;
Kind regards,
Paul
