* [md PATCH 16/18] md/multipath: add rcu protection to rdev access in multipath_status.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/multipath.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index dd483bb2e111..69244de2036b 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -141,17 +141,19 @@ static void multipath_make_request(struct mddev *mddev, struct bio * bio)
return;
}
-static void multipath_status (struct seq_file *seq, struct mddev *mddev)
+static void multipath_status(struct seq_file *seq, struct mddev *mddev)
{
struct mpconf *conf = mddev->private;
int i;
seq_printf (seq, " [%d/%d] [", conf->raid_disks,
conf->raid_disks - mddev->degraded);
- for (i = 0; i < conf->raid_disks; i++)
- seq_printf (seq, "%s",
- conf->multipaths[i].rdev &&
- test_bit(In_sync, &conf->multipaths[i].rdev->flags) ? "U" : "_");
+ rcu_read_lock();
+ for (i = 0; i < conf->raid_disks; i++) {
+ struct md_rdev *rdev = rcu_dereference(conf->multipaths[i].rdev);
+ seq_printf (seq, "%s", rdev && test_bit(In_sync, &rdev->flags) ? "U" : "_");
+ }
+ rcu_read_unlock();
seq_printf (seq, "]");
}
^ permalink raw reply related
* [md PATCH 15/18] md/raid5: add rcu protection to rdev accesses in raid5_status.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid5.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 7da89ee22a31..e9beba258f4f 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7069,10 +7069,12 @@ static void raid5_status(struct seq_file *seq, struct mddev *mddev)
seq_printf(seq, " level %d, %dk chunk, algorithm %d", mddev->level,
conf->chunk_sectors / 2, mddev->layout);
seq_printf (seq, " [%d/%d] [", conf->raid_disks, conf->raid_disks - mddev->degraded);
- for (i = 0; i < conf->raid_disks; i++)
- seq_printf (seq, "%s",
- conf->disks[i].rdev &&
- test_bit(In_sync, &conf->disks[i].rdev->flags) ? "U" : "_");
+ rcu_read_lock();
+ for (i = 0; i < conf->raid_disks; i++) {
+ struct md_rdev *rdev = rcu_dereference(conf->disks[i].rdev);
+ seq_printf (seq, "%s", rdev && test_bit(In_sync, &rdev->flags) ? "U" : "_");
+ }
+ rcu_read_unlock();
seq_printf (seq, "]");
}
^ permalink raw reply related
* [md PATCH 14/18] md/raid5: add rcu protection to rdev accesses in want_replace
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
Being in the middle of resync is no longer protection against failed
rdevs disappearing. So add rcu protection.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid5.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index e9ad22d64bd7..7da89ee22a31 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3239,15 +3239,16 @@ static int want_replace(struct stripe_head *sh, int disk_idx)
{
struct md_rdev *rdev;
int rv = 0;
- /* Doing recovery so rcu locking not required */
- rdev = sh->raid_conf->disks[disk_idx].replacement;
+
+ rcu_read_lock();
+ rdev = rcu_dereference(sh->raid_conf->disks[disk_idx].replacement);
if (rdev
&& !test_bit(Faulty, &rdev->flags)
&& !test_bit(In_sync, &rdev->flags)
&& (rdev->recovery_offset <= sh->sector
|| rdev->mddev->recovery_cp <= sh->sector))
rv = 1;
-
+ rcu_read_unlock();
return rv;
}
^ permalink raw reply related
* [md PATCH 11/18] md/raid1: small code cleanup in end_sync_write
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
'mirror' is only used to find 'rdev', several times.
So just find 'rdev' once, and use it instead.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid1.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 9585a6b62123..cc53120e4d68 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1709,11 +1709,9 @@ static void end_sync_write(struct bio *bio)
struct r1bio *r1_bio = bio->bi_private;
struct mddev *mddev = r1_bio->mddev;
struct r1conf *conf = mddev->private;
- int mirror=0;
sector_t first_bad;
int bad_sectors;
-
- mirror = find_bio_disk(r1_bio, bio);
+ struct md_rdev *rdev = conf->mirrors[find_bio_disk(r1_bio, bio)].rdev;
if (!uptodate) {
sector_t sync_blocks = 0;
@@ -1726,16 +1724,12 @@ static void end_sync_write(struct bio *bio)
s += sync_blocks;
sectors_to_go -= sync_blocks;
} while (sectors_to_go > 0);
- set_bit(WriteErrorSeen,
- &conf->mirrors[mirror].rdev->flags);
- if (!test_and_set_bit(WantReplacement,
- &conf->mirrors[mirror].rdev->flags))
+ set_bit(WriteErrorSeen, &rdev->flags);
+ if (!test_and_set_bit(WantReplacement, &rdev->flags))
set_bit(MD_RECOVERY_NEEDED, &
mddev->recovery);
set_bit(R1BIO_WriteError, &r1_bio->state);
- } else if (is_badblock(conf->mirrors[mirror].rdev,
- r1_bio->sector,
- r1_bio->sectors,
+ } else if (is_badblock(rdev, r1_bio->sector, r1_bio->sectors,
&first_bad, &bad_sectors) &&
!is_badblock(conf->mirrors[r1_bio->read_disk].rdev,
r1_bio->sector,
^ permalink raw reply related
* [md PATCH 13/18] md/raid5: add rcu protection to rdev accesses in handle_failed_sync.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
The rdev could be freed while handle_failed_sync is running, so
rcu protection is needed.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid5.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8959e6dd31dd..e9ad22d64bd7 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3210,15 +3210,16 @@ handle_failed_sync(struct r5conf *conf, struct stripe_head *sh,
/* During recovery devices cannot be removed, so
* locking and refcounting of rdevs is not needed
*/
+ rcu_read_lock();
for (i = 0; i < conf->raid_disks; i++) {
- struct md_rdev *rdev = conf->disks[i].rdev;
+ struct md_rdev *rdev = rcu_dereference(conf->disks[i].rdev);
if (rdev
&& !test_bit(Faulty, &rdev->flags)
&& !test_bit(In_sync, &rdev->flags)
&& !rdev_set_badblocks(rdev, sh->sector,
STRIPE_SECTORS, 0))
abort = 1;
- rdev = conf->disks[i].replacement;
+ rdev = rcu_dereference(conf->disks[i].replacement);
if (rdev
&& !test_bit(Faulty, &rdev->flags)
&& !test_bit(In_sync, &rdev->flags)
@@ -3226,6 +3227,7 @@ handle_failed_sync(struct r5conf *conf, struct stripe_head *sh,
STRIPE_SECTORS, 0))
abort = 1;
}
+ rcu_read_unlock();
if (abort)
conf->recovery_disabled =
conf->mddev->recovery_disabled;
^ permalink raw reply related
* [md PATCH 10/18] md/raid1: small cleanup in raid1_end_read/write_request
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
Both functions use conf->mirrors[mirror].rdev several times, so
improve readability by storing this in a local variable.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid1.c | 35 +++++++++++++++--------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index a54edbe741ff..9585a6b62123 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -319,14 +319,13 @@ static void raid1_end_read_request(struct bio *bio)
{
int uptodate = !bio->bi_error;
struct r1bio *r1_bio = bio->bi_private;
- int mirror;
struct r1conf *conf = r1_bio->mddev->private;
+ struct md_rdev *rdev = conf->mirrors[r1_bio->read_disk].rdev;
- mirror = r1_bio->read_disk;
/*
* this branch is our 'one mirror IO has finished' event handler:
*/
- update_head_pos(mirror, r1_bio);
+ update_head_pos(r1_bio->read_disk, r1_bio);
if (uptodate)
set_bit(R1BIO_Uptodate, &r1_bio->state);
@@ -339,14 +338,14 @@ static void raid1_end_read_request(struct bio *bio)
spin_lock_irqsave(&conf->device_lock, flags);
if (r1_bio->mddev->degraded == conf->raid_disks ||
(r1_bio->mddev->degraded == conf->raid_disks-1 &&
- test_bit(In_sync, &conf->mirrors[mirror].rdev->flags)))
+ test_bit(In_sync, &rdev->flags)))
uptodate = 1;
spin_unlock_irqrestore(&conf->device_lock, flags);
}
if (uptodate) {
raid_end_bio_io(r1_bio);
- rdev_dec_pending(conf->mirrors[mirror].rdev, conf->mddev);
+ rdev_dec_pending(rdev, conf->mddev);
} else {
/*
* oops, read error:
@@ -356,7 +355,7 @@ static void raid1_end_read_request(struct bio *bio)
KERN_ERR "md/raid1:%s: %s: "
"rescheduling sector %llu\n",
mdname(conf->mddev),
- bdevname(conf->mirrors[mirror].rdev->bdev,
+ bdevname(rdev->bdev,
b),
(unsigned long long)r1_bio->sector);
set_bit(R1BIO_ReadError, &r1_bio->state);
@@ -403,20 +402,18 @@ static void r1_bio_write_done(struct r1bio *r1_bio)
static void raid1_end_write_request(struct bio *bio)
{
struct r1bio *r1_bio = bio->bi_private;
- int mirror, behind = test_bit(R1BIO_BehindIO, &r1_bio->state);
+ int behind = test_bit(R1BIO_BehindIO, &r1_bio->state);
struct r1conf *conf = r1_bio->mddev->private;
struct bio *to_put = NULL;
-
- mirror = find_bio_disk(r1_bio, bio);
+ int mirror = find_bio_disk(r1_bio, bio);
+ struct md_rdev *rdev = conf->mirrors[mirror].rdev;
/*
* 'one mirror IO has finished' event handler:
*/
if (bio->bi_error) {
- set_bit(WriteErrorSeen,
- &conf->mirrors[mirror].rdev->flags);
- if (!test_and_set_bit(WantReplacement,
- &conf->mirrors[mirror].rdev->flags))
+ set_bit(WriteErrorSeen, &rdev->flags);
+ if (!test_and_set_bit(WantReplacement, &rdev->flags))
set_bit(MD_RECOVERY_NEEDED, &
conf->mddev->recovery);
@@ -445,13 +442,12 @@ static void raid1_end_write_request(struct bio *bio)
* before rdev->recovery_offset, but for simplicity we don't
* check this here.
*/
- if (test_bit(In_sync, &conf->mirrors[mirror].rdev->flags) &&
- !test_bit(Faulty, &conf->mirrors[mirror].rdev->flags))
+ if (test_bit(In_sync, &rdev->flags) &&
+ !test_bit(Faulty, &rdev->flags))
set_bit(R1BIO_Uptodate, &r1_bio->state);
/* Maybe we can clear some bad blocks. */
- if (is_badblock(conf->mirrors[mirror].rdev,
- r1_bio->sector, r1_bio->sectors,
+ if (is_badblock(rdev, r1_bio->sector, r1_bio->sectors,
&first_bad, &bad_sectors)) {
r1_bio->bios[mirror] = IO_MADE_GOOD;
set_bit(R1BIO_MadeGood, &r1_bio->state);
@@ -459,7 +455,7 @@ static void raid1_end_write_request(struct bio *bio)
}
if (behind) {
- if (test_bit(WriteMostly, &conf->mirrors[mirror].rdev->flags))
+ if (test_bit(WriteMostly, &rdev->flags))
atomic_dec(&r1_bio->behind_remaining);
/*
@@ -483,8 +479,7 @@ static void raid1_end_write_request(struct bio *bio)
}
}
if (r1_bio->bios[mirror] == NULL)
- rdev_dec_pending(conf->mirrors[mirror].rdev,
- conf->mddev);
+ rdev_dec_pending(rdev, conf->mddev);
/*
* Let's see if all mirrored write operations have finished
^ permalink raw reply related
* [md PATCH 12/18] md/raid1: add rcu protection to rdev in fix_read_error
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
Since remove_and_add_spares() was added to hot_remove_disk() it has
been possible for an rdev to be hot-removed while fix_read_error()
was running, so we need to be more careful, and take a reference to
the rdev while performing IO.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid1.c | 52 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index cc53120e4d68..0ee901ccc9c5 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2056,29 +2056,30 @@ static void fix_read_error(struct r1conf *conf, int read_disk,
s = PAGE_SIZE >> 9;
do {
- /* Note: no rcu protection needed here
- * as this is synchronous in the raid1d thread
- * which is the thread that might remove
- * a device. If raid1d ever becomes multi-threaded....
- */
sector_t first_bad;
int bad_sectors;
- rdev = conf->mirrors[d].rdev;
+ rcu_read_lock();
+ rdev = rcu_dereference(conf->mirrors[d].rdev);
if (rdev &&
(test_bit(In_sync, &rdev->flags) ||
(!test_bit(Faulty, &rdev->flags) &&
rdev->recovery_offset >= sect + s)) &&
is_badblock(rdev, sect, s,
- &first_bad, &bad_sectors) == 0 &&
- sync_page_io(rdev, sect, s<<9,
- conf->tmppage, READ, false))
- success = 1;
- else {
- d++;
- if (d == conf->raid_disks * 2)
- d = 0;
- }
+ &first_bad, &bad_sectors) == 0) {
+ atomic_inc(&rdev->nr_pending);
+ rcu_read_unlock();
+ if (sync_page_io(rdev, sect, s<<9,
+ conf->tmppage, READ, false))
+ success = 1;
+ rdev_dec_pending(rdev, mddev);
+ if (success)
+ break;
+ } else
+ rcu_read_unlock();
+ d++;
+ if (d == conf->raid_disks * 2)
+ d = 0;
} while (!success && d != read_disk);
if (!success) {
@@ -2094,11 +2095,17 @@ static void fix_read_error(struct r1conf *conf, int read_disk,
if (d==0)
d = conf->raid_disks * 2;
d--;
- rdev = conf->mirrors[d].rdev;
+ rcu_read_lock();
+ rdev = rcu_dereference(conf->mirrors[d].rdev);
if (rdev &&
- !test_bit(Faulty, &rdev->flags))
+ !test_bit(Faulty, &rdev->flags)) {
+ atomic_inc(&rdev->nr_pending);
+ rcu_read_unlock();
r1_sync_page_io(rdev, sect, s,
conf->tmppage, WRITE);
+ rdev_dec_pending(rdev, mddev);
+ } else
+ rcu_read_unlock();
}
d = start;
while (d != read_disk) {
@@ -2106,9 +2113,12 @@ static void fix_read_error(struct r1conf *conf, int read_disk,
if (d==0)
d = conf->raid_disks * 2;
d--;
- rdev = conf->mirrors[d].rdev;
+ rcu_read_lock();
+ rdev = rcu_dereference(conf->mirrors[d].rdev);
if (rdev &&
!test_bit(Faulty, &rdev->flags)) {
+ atomic_inc(&rdev->nr_pending);
+ rcu_read_unlock();
if (r1_sync_page_io(rdev, sect, s,
conf->tmppage, READ)) {
atomic_add(s, &rdev->corrected_errors);
@@ -2117,10 +2127,12 @@ static void fix_read_error(struct r1conf *conf, int read_disk,
"(%d sectors at %llu on %s)\n",
mdname(mddev), s,
(unsigned long long)(sect +
- rdev->data_offset),
+ rdev->data_offset),
bdevname(rdev->bdev, b));
}
- }
+ rdev_dec_pending(rdev, mddev);
+ } else
+ rcu_read_unlock();
}
sectors -= s;
sect += s;
^ permalink raw reply related
* [md PATCH 09/18] md/raid10: stop print_conf from being too verbose.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
raid10 arrays can usefully be very large. When they are, the noise generated by print_conf
can over-whelm logs.
So truncate the listing at 16 devices.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid10.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 61091ab02a4b..5d40612d6219 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1610,7 +1610,7 @@ static void print_conf(struct r10conf *conf)
/* This is only called with ->reconfix_mutex held, so
* rcu protection of rdev is not needed */
- for (i = 0; i < conf->geo.raid_disks; i++) {
+ for (i = 0; i < conf->geo.raid_disks && i < 16; i++) {
char b[BDEVNAME_SIZE];
rdev = conf->mirrors[i].rdev;
if (rdev)
@@ -1619,6 +1619,8 @@ static void print_conf(struct r10conf *conf)
!test_bit(Faulty, &rdev->flags),
bdevname(rdev->bdev,b));
}
+ if (conf->geo.raid_disks > 16)
+ printk(KERN_DEBUG " remaining devices excluded for brevity\n");
}
static void close_sync(struct r10conf *conf)
^ permalink raw reply related
* [md PATCH 08/18] md/raid10: simplify print_conf a little.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
'tmp' is only ever used to extract 'tmp->rdev', so just use 'rdev' directly.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid10.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 819c6d5767a4..61091ab02a4b 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1598,7 +1598,7 @@ static void raid10_error(struct mddev *mddev, struct md_rdev *rdev)
static void print_conf(struct r10conf *conf)
{
int i;
- struct raid10_info *tmp;
+ struct md_rdev *rdev;
printk(KERN_DEBUG "RAID10 conf printout:\n");
if (!conf) {
@@ -1608,14 +1608,16 @@ static void print_conf(struct r10conf *conf)
printk(KERN_DEBUG " --- wd:%d rd:%d\n", conf->geo.raid_disks - conf->mddev->degraded,
conf->geo.raid_disks);
+ /* This is only called with ->reconfix_mutex held, so
+ * rcu protection of rdev is not needed */
for (i = 0; i < conf->geo.raid_disks; i++) {
char b[BDEVNAME_SIZE];
- tmp = conf->mirrors + i;
- if (tmp->rdev)
+ rdev = conf->mirrors[i].rdev;
+ if (rdev)
printk(KERN_DEBUG " disk %d, wo:%d, o:%d, dev:%s\n",
- i, !test_bit(In_sync, &tmp->rdev->flags),
- !test_bit(Faulty, &tmp->rdev->flags),
- bdevname(tmp->rdev->bdev,b));
+ i, !test_bit(In_sync, &rdev->flags),
+ !test_bit(Faulty, &rdev->flags),
+ bdevname(rdev->bdev,b));
}
}
^ permalink raw reply related
* [md PATCH 07/18] md/raid10: minor code improvement in fix_read_error()
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
rdev already holds conf->mirrors[d].rdev, so no need to load it again.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid10.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 588cf6544f07..819c6d5767a4 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2262,7 +2262,7 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
printk(KERN_NOTICE
"md/raid10:%s: %s: Failing raid device\n",
mdname(mddev), b);
- md_error(mddev, conf->mirrors[d].rdev);
+ md_error(mddev, rdev);
r10_bio->devs[r10_bio->read_slot].bio = IO_BLOCKED;
return;
}
^ permalink raw reply related
* [md PATCH 05/18] md/raid10: add rcu protection to rdev access in raid10_sync_request.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
mirrors[].rdev can become NULL at any point unless:
- a counted reference is held
- ->reconfig_mutex is held, or
- rcu_read_lock() is held
Previously they could not become NULL during a resync/recovery/reshape either.
However when remove_and_add_spares() was added to hot_remove_disk(), that
changed.
So raid10_sync_request didn't previously need to protect rdev access,
but now it does.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid10.c | 120 +++++++++++++++++++++++++++++++--------------------
1 file changed, 72 insertions(+), 48 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index e6f3a11f8f70..f6f21a253926 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2871,11 +2871,14 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
/* Completed a full sync so the replacements
* are now fully recovered.
*/
- for (i = 0; i < conf->geo.raid_disks; i++)
- if (conf->mirrors[i].replacement)
- conf->mirrors[i].replacement
- ->recovery_offset
- = MaxSector;
+ rcu_read_lock();
+ for (i = 0; i < conf->geo.raid_disks; i++) {
+ struct md_rdev *rdev =
+ rcu_dereference(conf->mirrors[i].replacement);
+ if (rdev)
+ rdev->recovery_offset = MaxSector;
+ }
+ rcu_read_unlock();
}
conf->fullsync = 0;
}
@@ -2934,14 +2937,17 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
int must_sync;
int any_working;
struct raid10_info *mirror = &conf->mirrors[i];
+ struct md_rdev *mrdev, *mreplace;
- if ((mirror->rdev == NULL ||
- test_bit(In_sync, &mirror->rdev->flags))
- &&
- (mirror->replacement == NULL ||
- test_bit(Faulty,
- &mirror->replacement->flags)))
+ rcu_read_lock();
+ mrdev = rcu_dereference(mirror->rdev);
+ mreplace = rcu_dereference(mirror->replacement);
+
+ if ((mrdev == NULL ||
+ test_bit(In_sync, &mrdev->flags))) {
+ rcu_read_unlock();
continue;
+ }
still_degraded = 0;
/* want to reconstruct this device */
@@ -2951,6 +2957,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
/* last stripe is not complete - don't
* try to recover this sector.
*/
+ rcu_read_unlock();
continue;
}
/* Unless we are doing a full sync, or a replacement
@@ -2962,14 +2969,19 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
if (sync_blocks < max_sync)
max_sync = sync_blocks;
if (!must_sync &&
- mirror->replacement == NULL &&
+ mreplace == NULL &&
!conf->fullsync) {
/* yep, skip the sync_blocks here, but don't assume
* that there will never be anything to do here
*/
chunks_skipped = -1;
+ rcu_read_unlock();
continue;
}
+ atomic_inc(&mrdev->nr_pending);
+ if (mreplace)
+ atomic_inc(&mreplace->nr_pending);
+ rcu_read_unlock();
r10_bio = mempool_alloc(conf->r10buf_pool, GFP_NOIO);
r10_bio->state = 0;
@@ -2988,12 +3000,15 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
/* Need to check if the array will still be
* degraded
*/
- for (j = 0; j < conf->geo.raid_disks; j++)
- if (conf->mirrors[j].rdev == NULL ||
- test_bit(Faulty, &conf->mirrors[j].rdev->flags)) {
+ rcu_read_lock();
+ for (j = 0; j < conf->geo.raid_disks; j++) {
+ struct md_rdev *rdev = rcu_dereference(
+ conf->mirrors[j].rdev);
+ if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
still_degraded = 1;
break;
}
+ }
must_sync = bitmap_start_sync(mddev->bitmap, sect,
&sync_blocks, still_degraded);
@@ -3003,15 +3018,15 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
int k;
int d = r10_bio->devs[j].devnum;
sector_t from_addr, to_addr;
- struct md_rdev *rdev;
+ struct md_rdev *rdev =
+ rcu_dereference(conf->mirrors[d].rdev);
sector_t sector, first_bad;
int bad_sectors;
- if (!conf->mirrors[d].rdev ||
- !test_bit(In_sync, &conf->mirrors[d].rdev->flags))
+ if (!rdev ||
+ !test_bit(In_sync, &rdev->flags))
continue;
/* This is where we read from */
any_working = 1;
- rdev = conf->mirrors[d].rdev;
sector = r10_bio->devs[j].addr;
if (is_badblock(rdev, sector, max_sync,
@@ -3050,8 +3065,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
r10_bio->devs[1].devnum = i;
r10_bio->devs[1].addr = to_addr;
- rdev = mirror->rdev;
- if (!test_bit(In_sync, &rdev->flags)) {
+ if (!test_bit(In_sync, &mrdev->flags)) {
bio = r10_bio->devs[1].bio;
bio_reset(bio);
bio->bi_next = biolist;
@@ -3060,8 +3074,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio->bi_end_io = end_sync_write;
bio->bi_rw = WRITE;
bio->bi_iter.bi_sector = to_addr
- + rdev->data_offset;
- bio->bi_bdev = rdev->bdev;
+ + mrdev->data_offset;
+ bio->bi_bdev = mrdev->bdev;
atomic_inc(&r10_bio->remaining);
} else
r10_bio->devs[1].bio->bi_end_io = NULL;
@@ -3070,8 +3084,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio = r10_bio->devs[1].repl_bio;
if (bio)
bio->bi_end_io = NULL;
- rdev = mirror->replacement;
- /* Note: if rdev != NULL, then bio
+ /* Note: if mreplace != NULL, then bio
* cannot be NULL as r10buf_pool_alloc will
* have allocated it.
* So the second test here is pointless.
@@ -3079,8 +3092,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
* this comment keeps human reviewers
* happy.
*/
- if (rdev == NULL || bio == NULL ||
- test_bit(Faulty, &rdev->flags))
+ if (mreplace == NULL || bio == NULL ||
+ test_bit(Faulty, &mreplace->flags))
break;
bio_reset(bio);
bio->bi_next = biolist;
@@ -3089,11 +3102,12 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio->bi_end_io = end_sync_write;
bio->bi_rw = WRITE;
bio->bi_iter.bi_sector = to_addr +
- rdev->data_offset;
- bio->bi_bdev = rdev->bdev;
+ mreplace->data_offset;
+ bio->bi_bdev = mreplace->bdev;
atomic_inc(&r10_bio->remaining);
break;
}
+ rcu_read_unlock();
if (j == conf->copies) {
/* Cannot recover, so abort the recovery or
* record a bad block */
@@ -3106,15 +3120,15 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
if (r10_bio->devs[k].devnum == i)
break;
if (!test_bit(In_sync,
- &mirror->rdev->flags)
+ &mrdev->flags)
&& !rdev_set_badblocks(
- mirror->rdev,
+ mrdev,
r10_bio->devs[k].addr,
max_sync, 0))
any_working = 0;
- if (mirror->replacement &&
+ if (mreplace &&
!rdev_set_badblocks(
- mirror->replacement,
+ mreplace,
r10_bio->devs[k].addr,
max_sync, 0))
any_working = 0;
@@ -3132,8 +3146,14 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
if (rb2)
atomic_dec(&rb2->remaining);
r10_bio = rb2;
+ rdev_dec_pending(mrdev, mddev);
+ if (mreplace)
+ rdev_dec_pending(mreplace, mddev);
break;
}
+ rdev_dec_pending(mrdev, mddev);
+ if (mreplace)
+ rdev_dec_pending(mreplace, mddev);
}
if (biolist == NULL) {
while (r10_bio) {
@@ -3178,6 +3198,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
int d = r10_bio->devs[i].devnum;
sector_t first_bad, sector;
int bad_sectors;
+ struct md_rdev *rdev;
if (r10_bio->devs[i].repl_bio)
r10_bio->devs[i].repl_bio->bi_end_io = NULL;
@@ -3185,12 +3206,14 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio = r10_bio->devs[i].bio;
bio_reset(bio);
bio->bi_error = -EIO;
- if (conf->mirrors[d].rdev == NULL ||
- test_bit(Faulty, &conf->mirrors[d].rdev->flags))
+ rcu_read_lock();
+ rdev = rcu_dereference(conf->mirrors[d].rdev);
+ if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
+ rcu_read_unlock();
continue;
+ }
sector = r10_bio->devs[i].addr;
- if (is_badblock(conf->mirrors[d].rdev,
- sector, max_sync,
+ if (is_badblock(rdev, sector, max_sync,
&first_bad, &bad_sectors)) {
if (first_bad > sector)
max_sync = first_bad - sector;
@@ -3198,25 +3221,28 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bad_sectors -= (sector - first_bad);
if (max_sync > bad_sectors)
max_sync = bad_sectors;
+ rcu_read_unlock();
continue;
}
}
- atomic_inc(&conf->mirrors[d].rdev->nr_pending);
+ atomic_inc(&rdev->nr_pending);
atomic_inc(&r10_bio->remaining);
bio->bi_next = biolist;
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_read;
bio->bi_rw = READ;
- bio->bi_iter.bi_sector = sector +
- conf->mirrors[d].rdev->data_offset;
- bio->bi_bdev = conf->mirrors[d].rdev->bdev;
+ bio->bi_iter.bi_sector = sector + rdev->data_offset;
+ bio->bi_bdev = rdev->bdev;
count++;
- if (conf->mirrors[d].replacement == NULL ||
- test_bit(Faulty,
- &conf->mirrors[d].replacement->flags))
+ rdev = rcu_dereference(conf->mirrors[d].replacement);
+ if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
+ rcu_read_unlock();
continue;
+ }
+ atomic_inc(&rdev->nr_pending);
+ rcu_read_unlock();
/* Need to set up for writing to the replacement */
bio = r10_bio->devs[i].repl_bio;
@@ -3224,15 +3250,13 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio->bi_error = -EIO;
sector = r10_bio->devs[i].addr;
- atomic_inc(&conf->mirrors[d].replacement->nr_pending);
bio->bi_next = biolist;
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_write;
bio->bi_rw = WRITE;
- bio->bi_iter.bi_sector = sector +
- conf->mirrors[d].replacement->data_offset;
- bio->bi_bdev = conf->mirrors[d].replacement->bdev;
+ bio->bi_iter.bi_sector = sector + rdev->data_offset;
+ bio->bi_bdev = rdev->bdev;
count++;
}
^ permalink raw reply related
* [md PATCH 06/18] md/raid10: add rcu protection to rdev access during reshape.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
mirrors[].rdev can become NULL at any point unless:
- a counted reference is held
- ->reconfig_mutex is held, or
- rcu_read_lock() is held
Reshape isn't always suitably careful as in the past rdev couldn't be
removed during reshape. It can now, so add protection.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid10.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index f6f21a253926..588cf6544f07 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4352,15 +4352,16 @@ read_more:
blist = read_bio;
read_bio->bi_next = NULL;
+ rcu_read_lock();
for (s = 0; s < conf->copies*2; s++) {
struct bio *b;
int d = r10_bio->devs[s/2].devnum;
struct md_rdev *rdev2;
if (s&1) {
- rdev2 = conf->mirrors[d].replacement;
+ rdev2 = rcu_dereference(conf->mirrors[d].replacement);
b = r10_bio->devs[s/2].repl_bio;
} else {
- rdev2 = conf->mirrors[d].rdev;
+ rdev2 = rcu_dereference(conf->mirrors[d].rdev);
b = r10_bio->devs[s/2].bio;
}
if (!rdev2 || test_bit(Faulty, &rdev2->flags))
@@ -4405,6 +4406,7 @@ read_more:
nr_sectors += len >> 9;
}
bio_full:
+ rcu_read_unlock();
r10_bio->sectors = nr_sectors;
/* Now submit the read */
@@ -4456,16 +4458,20 @@ static void reshape_request_write(struct mddev *mddev, struct r10bio *r10_bio)
struct bio *b;
int d = r10_bio->devs[s/2].devnum;
struct md_rdev *rdev;
+ rcu_read_lock();
if (s&1) {
- rdev = conf->mirrors[d].replacement;
+ rdev = rcu_dereference(conf->mirrors[d].replacement);
b = r10_bio->devs[s/2].repl_bio;
} else {
- rdev = conf->mirrors[d].rdev;
+ rdev = rcu_dereference(conf->mirrors[d].rdev);
b = r10_bio->devs[s/2].bio;
}
- if (!rdev || test_bit(Faulty, &rdev->flags))
+ if (!rdev || test_bit(Faulty, &rdev->flags)) {
+ rcu_read_unlock();
continue;
+ }
atomic_inc(&rdev->nr_pending);
+ rcu_read_unlock();
md_sync_acct(b->bi_bdev, r10_bio->sectors);
atomic_inc(&r10_bio->remaining);
b->bi_next = NULL;
@@ -4526,9 +4532,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
if (s > (PAGE_SIZE >> 9))
s = PAGE_SIZE >> 9;
+ rcu_read_lock();
while (!success) {
int d = r10b->devs[slot].devnum;
- struct md_rdev *rdev = conf->mirrors[d].rdev;
+ struct md_rdev *rdev = rcu_dereference(conf->mirrors[d].rdev);
sector_t addr;
if (rdev == NULL ||
test_bit(Faulty, &rdev->flags) ||
@@ -4536,11 +4543,15 @@ static int handle_reshape_read_error(struct mddev *mddev,
goto failed;
addr = r10b->devs[slot].addr + idx * PAGE_SIZE;
+ atomic_inc(&rdev->nr_pending);
+ rcu_read_unlock();
success = sync_page_io(rdev,
addr,
s << 9,
bvec[idx].bv_page,
READ, false);
+ rdev_dec_pending(rdev, mddev);
+ rcu_read_lock();
if (success)
break;
failed:
@@ -4550,6 +4561,7 @@ static int handle_reshape_read_error(struct mddev *mddev,
if (slot == first_slot)
break;
}
+ rcu_read_unlock();
if (!success) {
/* couldn't read this block, must give up */
set_bit(MD_RECOVERY_INTR,
@@ -4619,16 +4631,18 @@ static void raid10_finish_reshape(struct mddev *mddev)
}
} else {
int d;
+ rcu_read_lock();
for (d = conf->geo.raid_disks ;
d < conf->geo.raid_disks - mddev->delta_disks;
d++) {
- struct md_rdev *rdev = conf->mirrors[d].rdev;
+ struct md_rdev *rdev = rcu_dereference(conf->mirrors[d].rdev);
if (rdev)
clear_bit(In_sync, &rdev->flags);
- rdev = conf->mirrors[d].replacement;
+ rdev = rcu_dereference(conf->mirrors[d].replacement);
if (rdev)
clear_bit(In_sync, &rdev->flags);
}
+ rcu_read_unlock();
}
mddev->layout = mddev->new_layout;
mddev->chunk_sectors = 1 << conf->geo.chunk_shift;
^ permalink raw reply related
* [md PATCH 03/18] md/raid10: fix refounct imbalance when resyncing an array with a replacement device.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
If you have a raid10 with a replacement device that is resyncing -
e.g. after a crash before the replacement was complete - the write to
the replacement will increment nr_pending on the wrong device, which
will lead to strangeness.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid10.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index cc9e3813e1a4..f371d45f786a 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3222,7 +3222,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio->bi_error = -EIO;
sector = r10_bio->devs[i].addr;
- atomic_inc(&conf->mirrors[d].rdev->nr_pending);
+ atomic_inc(&conf->mirrors[d].replacement->nr_pending);
bio->bi_next = biolist;
biolist = bio;
bio->bi_private = r10_bio;
^ permalink raw reply related
* [md PATCH 04/18] md/raid10: add rcu protection in raid10_status.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
mirrors[].rdev can become NULL at any point unless:
- a counted reference is held
- ->reconfig_mutex is held, or
- rcu_read_lock() is held
raid10_status holds none of these. So add rcu_read_lock()
protection.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid10.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index f371d45f786a..e6f3a11f8f70 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1495,10 +1495,12 @@ static void raid10_status(struct seq_file *seq, struct mddev *mddev)
}
seq_printf(seq, " [%d/%d] [", conf->geo.raid_disks,
conf->geo.raid_disks - mddev->degraded);
- for (i = 0; i < conf->geo.raid_disks; i++)
- seq_printf(seq, "%s",
- conf->mirrors[i].rdev &&
- test_bit(In_sync, &conf->mirrors[i].rdev->flags) ? "U" : "_");
+ rcu_read_lock();
+ for (i = 0; i < conf->geo.raid_disks; i++) {
+ struct md_rdev *rdev = rcu_dereference(conf->mirrors[i].rdev);
+ seq_printf(seq, "%s", rdev && test_bit(In_sync, &rdev->flags) ? "U" : "_");
+ }
+ rcu_read_unlock();
seq_printf(seq, "]");
}
^ permalink raw reply related
* [md PATCH 02/18] md/raid1, raid10: don't recheck "Faulty" flag in read-balance.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
Re-checking the faulty flag here brings no value.
The comment about "risk" refers to the risk that the device could
be in the process of being removed by ->hot_remove_disk().
However providing that the ->nr_pending count is incremented inside
an rcu_read_locked() region, there is no risk of that happening.
This is because the rdev pointer (in the personalities array) is set
to NULL before synchronize_rcu(), and ->nr_pending is tested
afterwards. If the rcu_read_locked region happens before the
synchronize_rcu(), the test will see that nr_pending has been incremented.
If it happens afterwards, the rdev pointer will be NULL so there is nothing
to increment.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid1.c | 7 -------
drivers/md/raid10.c | 8 --------
2 files changed, 15 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index c7c8cde0ab21..a54edbe741ff 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -689,13 +689,6 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio, int *max_sect
if (!rdev)
goto retry;
atomic_inc(&rdev->nr_pending);
- if (test_bit(Faulty, &rdev->flags)) {
- /* cannot risk returning a device that failed
- * before we inc'ed nr_pending
- */
- rdev_dec_pending(rdev, conf->mddev);
- goto retry;
- }
sectors = best_good_sectors;
if (conf->mirrors[best_disk].next_seq_sect != this_sector)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index c7de2a53e625..cc9e3813e1a4 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -707,7 +707,6 @@ static struct md_rdev *read_balance(struct r10conf *conf,
raid10_find_phys(conf, r10_bio);
rcu_read_lock();
-retry:
sectors = r10_bio->sectors;
best_slot = -1;
best_rdev = NULL;
@@ -804,13 +803,6 @@ retry:
if (slot >= 0) {
atomic_inc(&rdev->nr_pending);
- if (test_bit(Faulty, &rdev->flags)) {
- /* Cannot risk returning a device that failed
- * before we inc'ed nr_pending
- */
- rdev_dec_pending(rdev, conf->mddev);
- goto retry;
- }
r10_bio->read_slot = slot;
} else
rdev = NULL;
^ permalink raw reply related
* [md PATCH 01/18] md: disconnect device from personality before trying to remove it.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
When the HOT_REMOVE_DISK ioctl is used to remove a device, we
call remove_and_add_spares() which will remove it from the personality
if possible. This improves the chances that the removal will succeed.
When writing "remove" to dev-XX/state, we don't. So that can fail more easily.
So add the remove_and_add_spares() into "remove" handling.
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/md.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 866825f10b4c..2d26099e1160 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2596,6 +2596,8 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len)
else
err = -EBUSY;
} else if (cmd_match(buf, "remove")) {
+ clear_bit(Blocked, &rdev->flags);
+ remove_and_add_spares(rdev->mddev, rdev);
if (rdev->raid_disk >= 0)
err = -EBUSY;
else {
^ permalink raw reply related
* [md PATCH 00/18] Assorted minor fixes, particularly RCU protection.
From: NeilBrown @ 2016-06-02 6:19 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
The 'rdev' fields in each personality's config data are often accessed
under rcu_read_lock() protection to avoid races with
->hot_remove_disk() removing the rdev.
Originally this was not necessary during resync/recovery etc because
->hot_remove_disk() was only called from md_check_recovery(), and it
would only make the call if there was no resync etc happening.
However we now call ->hot_remove_disk() (from
remove_and_add_spares()) from other contexts, so there could be a race
in the resync code.
So this patch set adds a lot of extra rcu_read_lock protection and
clean up some other bits and pieces on the way.
My goal was the final patch. If you have a large raid10 array and
fail half of the devices at once (e.g. unplug a rack with half of the
mirrors) then synchronize_rcu() will be called once for each device,
which can add up to a big delay. A single call should suffice.
The final patch makes that change.
Thanks,
NeilBrown
---
NeilBrown (18):
md: disconnect device from personality before trying to remove it.
md/raid1,raid10: don't recheck "Faulty" flag in read-balance.
md/raid10: fix refounct imbalance when resyncing an array with a replacement device.
md/raid10: add rcu protection in raid10_status.
md/raid10: add rcu protection to rdev access in raid10_sync_request.
md/raid10: add rcu protection to rdev access during reshape.
md/raid10: minor code improvement in fix_read_error()
md/raid10: simplify print_conf a little.
md/raid10: stop print_conf from being too verbose.
md/raid1: small cleanup in raid1_end_read/write_request
md/raid1: small code cleanup in end_sync_write
md/raid1: add rcu protection to rdev in fix_read_error
md/raid5: add rcu protection to rdev accesses in handle_failed_sync.
md/raid5: add rcu protection to rdev accesses in want_replace
md/raid5: add rcu protection to rdev accesses in raid5_status.
md/multipath: add rcu protection to rdev access in multipath_status.
md: be extra careful not to take a reference to a Faulty device.
md: reduce the number of synchronize_rcu() calls when multiple devices fail.
drivers/md/md.c | 29 ++++++-
drivers/md/md.h | 5 +
drivers/md/multipath.c | 29 ++++---
drivers/md/raid1.c | 125 ++++++++++++++--------------
drivers/md/raid10.c | 213 +++++++++++++++++++++++++++++-------------------
drivers/md/raid5.c | 41 ++++++---
6 files changed, 264 insertions(+), 178 deletions(-)
--
Signature
^ permalink raw reply
* Re: raid 5 crashed
From: Mikael Abrahamsson @ 2016-06-02 5:52 UTC (permalink / raw)
To: Brad Campbell; +Cc: linux-raid
In-Reply-To: <2f3e2fe6-1810-e7bd-7dc0-483ed4f0d46b@fnarfbargle.com>
On Thu, 2 Jun 2016, Brad Campbell wrote:
> People keep saying that. I've never encountered it. I suspect it's just not
Well, I have had drives that would occasionally throw a read error, but MD
requires that read error to happen three times before re-writing the
sector, and that never happened. See earlier discussions I had with Neil
on the topic. But you're correct, I don't see this on normally functioning
drives. Swapped out that drive (it didn't have any specific SMART errors
either) and everything was fine. I don't know what was wrong with it,
might have been something flying around in there causing spurious
problems.
> the problem that the hysterical ranting makes it out to be (either that or
> the pile of cheap and nasty drives I have here are model citizens).
> I've *never* seen a read error unless the drive was in trouble, and that
> includes running dd reads in a loop over multiple days continuously.
> If it were that bad I'd see drives failing SMART long tests routinely also,
> and that does not happen either.
I've seen enough read errors that I nowadays only run RAID6, never RAID5.
I'd also venture to say that considering the amount of people who come on
the list and who come on the #linux-raid IRC channel with "raid5,
one-drive-failed, and now I have read error on another drive so my array
doesn't resync, what should I do?", I'd say this is a real problem. It's
not however like "if you have a good drive, reading it 5 times will yield
a read error". The vendor bit error rate specification doesn't work like
that, so totally agree with you there.
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: Why does raid0 set max_hw_sectors as chunk size but the other raid types doesn't?
From: NeilBrown @ 2016-06-02 4:38 UTC (permalink / raw)
To: Joey Liao, linux-raid
In-Reply-To: <CAHvN=in80JrXO=tDyxNhq9d_wW1ZCCioENyTLu9tYuYYDof20w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]
On Mon, May 30 2016, Joey Liao wrote:
> Hi,
>
> I have no idea why does raid0_run() in raid0.c use
> blk_queue_max_hw_sectors() to set max_hw_sectors as the chunk size,
> but the other raid types doesn't?
git is your friend.... admittedly you need the 'history' git tree to go
back before 2.6.12, but it is available.
http://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=f556ef000efc90a45a285f4f0b4fd70bb70f
>
> What's the purpose to limit the max_hw_sectors in raid0?
unfortunately the commit doesn't answer that question. I think it was
to ensure requests larger than one chunk were not created. If they were
they would just have to be split, so there is no much to gain.
>
> Is it related to the source code logic issue or the performance issue?
>
> Besides, I have an interesting observation. If I remove all the
> following queue limitation codes in raid0_run() of raid0.c, the block
> size in iostat is still the same as chunk size even the input block
> size is much larger than the chunk size. Why???
Because when you write to a RAID0 you *must* divide each request up into
chunk-sizes sub-requests, and send them to different devices.
NeilBrown
>
> - blk_queue_max_hw_sectors(mddev->queue, mddev->chunk_sectors);
> - blk_queue_max_write_same_sectors(mddev->queue, mddev->chunk_sectors);
> + //blk_queue_max_hw_sectors(mddev->queue, mddev->chunk_sectors);
> + //blk_queue_max_write_same_sectors(mddev->queue, mddev->chunk_sectors);
>
> - blk_queue_io_min(mddev->queue, mddev->chunk_sectors << 9);
> - blk_queue_io_opt(mddev->queue,
> - (mddev->chunk_sectors << 9) * mddev->raid_disks);
> + //blk_queue_io_min(mddev->queue, mddev->chunk_sectors << 9);
> + /*blk_queue_io_opt(mddev->queue,
> + (mddev->chunk_sectors << 9) * mddev->raid_disks);*/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] Fixes for lots of arrays
From: Jes Sorensen @ 2016-06-02 1:00 UTC (permalink / raw)
To: Mike Lovell; +Cc: linux-raid
In-Reply-To: <CAK9wOaA0RaH7yPH83D2BxOLq41d3rC9Ck5E5nNauekSYkOZAng@mail.gmail.com>
Mike Lovell <mlovell@bluehost.com> writes:
> ping. its been about 2 week since i posted these. just following up on it.
Hi,
I think you sent it while I was traveling - I'll try to get to it within
the next couple of days.
Thanks for the reminder.
Jes
>
> mike
>
> On Wed, May 18, 2016 at 12:23 PM, Mike Lovell <mlovell@bluehost.com> wrote:
>> This patch series fixes two issues around having more than 127 arrays on a
>> system. The first one fixes an issue with using a dev_t as int and the
>> number going negative when the array number would be larger than 2<<19. This
>> would happen when more than 128 arrays were created on the system without
>> creating the arrays by name. Manually specifying the large number would also
>> fail.
>>
>> The second patch changes find_free_devnm in mdopen.c to use go to (2<<9)-1
>> after 128 arrays have been created. Newer versions of the kernel don't allow
>> the user to specify an array number than 511 so mdadm shouldn't automatically
>> choose a bigger number. There was discussion about checking for new_array
>> in /sys/module/md_mod/parameters on the list but that parameter has been in
>> the kernel since 2.6.29. Any kernel from the last 7 years would still be
>> limited by the check so it probably isn't worth a special case.
>>
>> Mike Lovell (2):
>> Use dev_t for devnm2devid and devid2devnm
>> Change behavior in find_free_devnm when wrapping around.
>>
>> Detail.c | 4 ++--
>> Grow.c | 2 +-
>> lib.c | 2 +-
>> mapfile.c | 2 +-
>> mdadm.h | 4 ++--
>> mdopen.c | 6 +++---
>> util.c | 6 +++---
>> 7 files changed, 13 insertions(+), 13 deletions(-)
>>
>> --
>> 1.9.1
>>
^ permalink raw reply
* Re: raid 5 crashed
From: Brad Campbell @ 2016-06-01 23:15 UTC (permalink / raw)
To: Wols Lists, Edward Kuns
Cc: Phil Turmel, bobzer, linux-raid, Mikael Abrahamsson
In-Reply-To: <574F00F5.80801@youngman.org.uk>
On 01/06/16 23:36, Wols Lists wrote:
> On 01/06/16 05:07, Brad Campbell wrote:
>> I have however done a *lot* of data recovery on single drives over the
>> years and can absolutely vouch that dd will leave you in tears.
> Good to know! I've regularly used dd on drives, but not on ones that
> were in trouble (maybe once ...)
>
> But now drives are at the point that you cannot guarantee an error-free
> scan even on just one pass,
People keep saying that. I've never encountered it. I suspect it's just
not the problem that the hysterical ranting makes it out to be (either
that or the pile of cheap and nasty drives I have here are model citizens).
I've *never* seen a read error unless the drive was in trouble, and that
includes running dd reads in a loop over multiple days continuously.
If it were that bad I'd see drives failing SMART long tests routinely
also, and that does not happen either.
--
Dolphins are so intelligent that within a few weeks they can
train Americans to stand at the edge of the pool and throw them
fish.
^ permalink raw reply
* Re: [RFC 4/5] r5cache: write part of r5cache
From: NeilBrown @ 2016-06-01 22:37 UTC (permalink / raw)
To: Song Liu, linux-raid@vger.kernel.org
Cc: Shaohua Li, dan.j.williams@intel.com, hch@infradead.org,
Kernel Team
In-Reply-To: <8FB9AC1E-AB6D-41B2-9411-C924B23FA7AF@fb.com>
[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]
On Wed, Jun 01 2016, Song Liu wrote:
>>
>>It isn't at all clear to me why you need the extra page. Could you
>>explain when and why the ->page and the ->orig_page would contain
>>different content and both of the content would be needed?
>>
>
> In prexor, we need read old data from the disk to perform the
> substract-xor. In the meanwhile, we need a buffer for the new data.
> When there is no write cache, the new data is in the page of bio.
> With write cache, however, the bio might be already released.
> Therefore, we will need an extra page here: orig_page for new
> Cached data, while newly allocated page just hold old data for prexor.
>
Ahh, I get it now - thanks. The fact that you said "page" rather than
"pages" threw me. I assumed it was a page for the parity block only,
which is wrong.
So I would write:
For RMW, the code allocates an extra page for each data block being
updated. This is stored in r5dev->page and the old data is read into
it. Then the prexor calculation subtracts ->page from the parity
block, and the reconstruct calculation adds the ->orig_page data back
into the parity block.
Or something like that.
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* RE: [PATCH] dm-log-writes: fix bug with too large bios
From: Mikulas Patocka @ 2016-06-01 17:32 UTC (permalink / raw)
To: James Johnston
Cc: 'Alasdair G. Kergon', 'Mike Snitzer',
'Josef Bacik', dm-devel, 'Eric Wheeler',
'Tim Small', 'Kent Overstreet', linux-bcache,
dm-crypt, 'Neil Brown', linux-raid
In-Reply-To: <07bf01d1bbbd$53991750$facb45f0$@codenest.com>
On Wed, 1 Jun 2016, James Johnston wrote:
> Hi Mikulas,
>
> > bio_alloc can allocate a bio with at most BIO_MAX_PAGES (256) vector
> > entries. However, the incoming bio may have more vector entries if it was
> > allocated by other means. For example, bcache submits bios with more than
> > BIO_MAX_PAGES entries. This results in bio_alloc failure.
> >
> > To avoid the failure, change the code so that it allocates bio with at
> > most BIO_MAX_PAGES entries. If the incoming bio has more entries,
> > bio_add_page will fail and a new bio will be allocated - the code that
> > handles bio_add_page failure already exists in the dm-log-writes target.
> >
> > Also, move atomic_inc(&lc->io_blocks) before bio_alloc to fix a bug that
> > the target hangs if bio_alloc fails. The error path does put_io_block(lc),
> > so we must do atomic_inc(&lc->io_blocks) before invoking the error path to
> > avoid underflow of lc->io_blocks.
> >
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > Cc: stable@vger.kernel.org # v4.1+
>
> How does this relate to the previous patch you made to dm-crypt? How best
> should I test this? It looks like the dm-crypt patch fixed the problem.
>
> Should I test by applying this patch ONLY and reverting the dm-crypt patch?
> (i.e. does this patch also fix the problem.) Or should I just test with
> both patches applied simultaneously?
>
> James
When I found a bug in dm-crypt, I searched the other targets for the same
problem and found out that the same bug is also present in dm-log-writes.
The bug in dm-log-writes can be reproduced if you make a bcache device
with 2MiB bucket size and place in on the dm-log-writes target.
When the bug is present, the dm-log-writes target reports "device-mapper:
log-writes: Couldn't alloc log bio".
This is a script that triggers the bug for dm-log-writes. Before running
the script, create devices /dev/mapper/loop-test1, loop-test2 and
loop-test3.
Mikulas
wipefs -a /dev/mapper/loop-test1
./make-bcache --wipe-bcache --bucket 2M -C /dev/mapper/loop-test1
dmsetup create backCrypt --table "0 `blockdev --getsize /dev/mapper/loop-test2` log-writes /dev/mapper/loop-test2 /dev/mapper/loop-test3"
wipefs -a /dev/mapper/backCrypt
./make-bcache --wipe-bcache -B /dev/mapper/backCrypt
modprobe bcache
echo /dev/mapper/loop-test1 > /sys/fs/bcache/register
echo /dev/mapper/backCrypt > /sys/fs/bcache/register
./bcache-super-show /dev/mapper/loop-test1 | grep cset.uuid | cut -f 3 >/sys/block/bcache0/bcache/attach
echo writeback > /sys/block/bcache0/bcache/cache_mode
cd /sys/block/bcache0/bcache
echo 0 > sequential_cutoff
# Verify that the cache is attached (i.e. does not say "no cache")
cat state
dd if=/dev/urandom of=/dev/bcache0 bs=1M count=250
cat dirty_data
cat state
echo 1 > detach
cat dirty_data
cat state
^ permalink raw reply
* Re: raid 5 crashed
From: Phil Turmel @ 2016-06-01 17:28 UTC (permalink / raw)
To: Wols Lists, Brad Campbell, bobzer; +Cc: linux-raid, Mikael Abrahamsson
In-Reply-To: <574F0258.5000108@youngman.org.uk>
On 06/01/2016 11:42 AM, Wols Lists wrote:
> Okay - so would this be better (a lot slower, possibly, but safe ...)
>
> Use dd - so it DOES bomb on error! - and only replace the drive once
> you've got a clean read off it. With 2TB drives, that should work so
> long as they're not faulty. And if it's - JUST - a timeout issue,
> this'll work fine?
If there's errors, you'll never get a clean read. (Short of the moon
and stars aligning for a near-miracle.) ddrescue and similar replace
those errors with zeros to successfully retrieve less than 100% of your
data.
The whole point of keeping it in the array is to get the correct data
from the array's redundancy wherever the disk has unfixed read errors.
And with correct timeouts, to *FIX* that read error. Please read *all*
of the links I posted on why and how this is.
Side note: In these situations, you should *not* use overlays, as that
prevents the *FIX* part from happening.
Temporarily setting the timeouts for non-raid drives is a one-liner:
for x in /sys/block/*/device/timeout ; do echo 180 > $x ; done
Phil
^ permalink raw reply
* Re: raid 5 crashed
From: Wols Lists @ 2016-06-01 15:42 UTC (permalink / raw)
To: Brad Campbell, Phil Turmel, bobzer; +Cc: linux-raid, Mikael Abrahamsson
In-Reply-To: <95079572-f319-ca57-a3e9-e8d00ef40248@fnarfbargle.com>
On 01/06/16 02:48, Brad Campbell wrote:
> Now, having said that :
>
> Much better to try and get the array running in a read-only state with
> all disks in place and clone the data from the array rather than the
> disks after they've been ddrescued. In the case of a running array, a
> read error on one of the array members will see the RAID attempt to get
> the data from elsewhere (a reconstruction), whereas a read from a disc
> cloned with ddrescue will happily just report what was a faulty sector
> as a big pile of zeros, and *poof* your data is gone.
>
> Set the timeouts appropriately (and conservatively) to give the disks
> time to actually report they can't read the sector. This will allow md
> to try and get it elsewhere rather than kicking the disc out because the
> storage stack timed it out as faulty.
Okay - so would this be better (a lot slower, possibly, but safe ...)
Use dd - so it DOES bomb on error! - and only replace the drive once
you've got a clean read off it. With 2TB drives, that should work so
long as they're not faulty. And if it's - JUST - a timeout issue,
this'll work fine?
Cheers,
Wol
^ 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