2023-06-02 09:43:16

by Li Nan

[permalink] [raw]
Subject: [PATCH v7 0/2] raid10 bugfix

From: Li Nan <[email protected]>

Changes in v7:
- in patch 1, change "fail" to "fails".

Changes in v6:
- in patch 1, improve commit message summary and comment.

Changes in v5:
- v4 send wrong patch, correct and resend.

Changes in v4:
- improve commit log
- removed applied patches


Li Nan (2):
md/raid10: Do not add spare disk when recovery fails
md/raid10: fix io loss while replacement replace rdev

drivers/md/raid10.c | 42 ++++++++++++++++++++++++++++++++++++------
1 file changed, 36 insertions(+), 6 deletions(-)

--
2.39.2



2023-06-02 09:49:49

by Li Nan

[permalink] [raw]
Subject: [PATCH v7 2/2] md/raid10: fix io loss while replacement replace rdev

From: Li Nan <[email protected]>

When removing a disk with replacement, the replacement will be used to
replace rdev. During this process, there is a brief window in which both
rdev and replacement are read as NULL in raid10_write_request(). This
will result in io not being submitted but it should be.

//remove //write
raid10_remove_disk raid10_write_request
mirror->rdev = NULL
read rdev -> NULL
mirror->rdev = mirror->replacement
mirror->replacement = NULL
read replacement -> NULL

Fix it by reading replacement first and rdev later, meanwhile, use smp_mb()
to prevent memory reordering.

Fixes: 475b0321a4df ("md/raid10: writes should get directed to replacement as well as original.")
Signed-off-by: Li Nan <[email protected]>
Reviewed-by: Yu Kuai <[email protected]>
---
drivers/md/raid10.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 3d52fb618571..c867efbc6197 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -779,8 +779,16 @@ static struct md_rdev *read_balance(struct r10conf *conf,
disk = r10_bio->devs[slot].devnum;
rdev = rcu_dereference(conf->mirrors[disk].replacement);
if (rdev == NULL || test_bit(Faulty, &rdev->flags) ||
- r10_bio->devs[slot].addr + sectors > rdev->recovery_offset)
+ r10_bio->devs[slot].addr + sectors >
+ rdev->recovery_offset) {
+ /*
+ * Read replacement first to prevent reading both rdev
+ * and replacement as NULL during replacement replace
+ * rdev.
+ */
+ smp_mb();
rdev = rcu_dereference(conf->mirrors[disk].rdev);
+ }
if (rdev == NULL ||
test_bit(Faulty, &rdev->flags))
continue;
@@ -1479,9 +1487,15 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,

for (i = 0; i < conf->copies; i++) {
int d = r10_bio->devs[i].devnum;
- struct md_rdev *rdev = rcu_dereference(conf->mirrors[d].rdev);
- struct md_rdev *rrdev = rcu_dereference(
- conf->mirrors[d].replacement);
+ struct md_rdev *rdev, *rrdev;
+
+ rrdev = rcu_dereference(conf->mirrors[d].replacement);
+ /*
+ * Read replacement first to prevent reading both rdev and
+ * replacement as NULL during replacement replace rdev.
+ */
+ smp_mb();
+ rdev = rcu_dereference(conf->mirrors[d].rdev);
if (rdev == rrdev)
rrdev = NULL;
if (rdev && (test_bit(Faulty, &rdev->flags)))
--
2.39.2


2023-06-06 00:45:58

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH v7 0/2] raid10 bugfix

On Fri, Jun 2, 2023 at 2:22 AM <[email protected]> wrote:
>
> From: Li Nan <[email protected]>
>
> Changes in v7:
> - in patch 1, change "fail" to "fails".

Applied v7 to md-next.

Thanks,
Song