nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

md/raid10: add rcu protection to rdev access in raid10_sync_request.

Browse source 
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.

Fix missed check(Shaohua)
Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: Shaohua Li <shli@fb.com>
This commit is contained in:
NeilBrown 2016-06-02 16:19:52 +10:00 committed by Shaohua Li
parent d44b0a928f
commit f90145f317
1 changed files with 74 additions and 48 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

122
drivers/md/raid10.c
View file

@ -2871,11 +2871,14 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
/* Completed a full sync so the replacements /* Completed a full sync so the replacements
* are now fully recovered. * are now fully recovered.
*/ */
for (i = 0; i < conf->geo.raid_disks; i++) rcu_read_lock();
if (conf->mirrors[i].replacement) for (i = 0; i < conf->geo.raid_disks; i++) {
conf->mirrors[i].replacement struct md_rdev *rdev =
->recovery_offset rcu_dereference(conf->mirrors[i].replacement);
= MaxSector; if (rdev)
rdev->recovery_offset = MaxSector;
}
rcu_read_unlock();
} }
conf->fullsync = 0; conf->fullsync = 0;
} }
@ -2941,14 +2944,19 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
int must_sync; int must_sync;
int any_working; int any_working;
struct raid10_info *mirror = &conf->mirrors[i]; struct raid10_info *mirror = &conf->mirrors[i];
struct md_rdev *mrdev, *mreplace;
if ((mirror->rdev == NULL || rcu_read_lock();
test_bit(In_sync, &mirror->rdev->flags)) mrdev = rcu_dereference(mirror->rdev);
&& mreplace = rcu_dereference(mirror->replacement);
(mirror->replacement == NULL ||
test_bit(Faulty, if ((mrdev == NULL ||
&mirror->replacement->flags))) test_bit(In_sync, &mrdev->flags)) &&
(mreplace == NULL ||
test_bit(Faulty, &mreplace->flags))) {
rcu_read_unlock();
continue; continue;
}
still_degraded = 0; still_degraded = 0;
/* want to reconstruct this device */ /* want to reconstruct this device */
@ -2958,6 +2966,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
/* last stripe is not complete - don't /* last stripe is not complete - don't
* try to recover this sector. * try to recover this sector.
*/ */
rcu_read_unlock();
continue; continue;
} }
/* Unless we are doing a full sync, or a replacement /* Unless we are doing a full sync, or a replacement
@ -2969,14 +2978,19 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
if (sync_blocks < max_sync) if (sync_blocks < max_sync)
max_sync = sync_blocks; max_sync = sync_blocks;
if (!must_sync && if (!must_sync &&
mirror->replacement == NULL && mreplace == NULL &&
!conf->fullsync) { !conf->fullsync) {
/* yep, skip the sync_blocks here, but don't assume /* yep, skip the sync_blocks here, but don't assume
* that there will never be anything to do here * that there will never be anything to do here
*/ */
chunks_skipped = -1; chunks_skipped = -1;
rcu_read_unlock();
continue; 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 = mempool_alloc(conf->r10buf_pool, GFP_NOIO);
r10_bio->state = 0; r10_bio->state = 0;
@ -2995,12 +3009,15 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
/* Need to check if the array will still be /* Need to check if the array will still be
* degraded * degraded
*/ */
for (j = 0; j < conf->geo.raid_disks; j++) rcu_read_lock();
if (conf->mirrors[j].rdev == NULL || for (j = 0; j < conf->geo.raid_disks; j++) {
test_bit(Faulty, &conf->mirrors[j].rdev->flags)) { struct md_rdev *rdev = rcu_dereference(
conf->mirrors[j].rdev);
if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
still_degraded = 1; still_degraded = 1;
break; break;
} }
}
must_sync = bitmap_start_sync(mddev->bitmap, sect, must_sync = bitmap_start_sync(mddev->bitmap, sect,
&sync_blocks, still_degraded); &sync_blocks, still_degraded);
@ -3010,15 +3027,15 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
int k; int k;
int d = r10_bio->devs[j].devnum; int d = r10_bio->devs[j].devnum;
sector_t from_addr, to_addr; 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; sector_t sector, first_bad;
int bad_sectors; int bad_sectors;
if (!conf->mirrors[d].rdev || if (!rdev ||
!test_bit(In_sync, &conf->mirrors[d].rdev->flags)) !test_bit(In_sync, &rdev->flags))
continue; continue;
/* This is where we read from */ /* This is where we read from */
any_working = 1; any_working = 1;
rdev = conf->mirrors[d].rdev;
sector = r10_bio->devs[j].addr; sector = r10_bio->devs[j].addr;
if (is_badblock(rdev, sector, max_sync, if (is_badblock(rdev, sector, max_sync,
@ -3057,8 +3074,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
r10_bio->devs[1].devnum = i; r10_bio->devs[1].devnum = i;
r10_bio->devs[1].addr = to_addr; r10_bio->devs[1].addr = to_addr;
rdev = mirror->rdev; if (!test_bit(In_sync, &mrdev->flags)) {
if (!test_bit(In_sync, &rdev->flags)) {
bio = r10_bio->devs[1].bio; bio = r10_bio->devs[1].bio;
bio_reset(bio); bio_reset(bio);
bio->bi_next = biolist; bio->bi_next = biolist;
@ -3067,8 +3083,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio->bi_end_io = end_sync_write; bio->bi_end_io = end_sync_write;
bio->bi_rw = WRITE; bio->bi_rw = WRITE;
bio->bi_iter.bi_sector = to_addr bio->bi_iter.bi_sector = to_addr
+ rdev->data_offset; + mrdev->data_offset;
bio->bi_bdev = rdev->bdev; bio->bi_bdev = mrdev->bdev;
atomic_inc(&r10_bio->remaining); atomic_inc(&r10_bio->remaining);
} else } else
r10_bio->devs[1].bio->bi_end_io = NULL; r10_bio->devs[1].bio->bi_end_io = NULL;
@ -3077,8 +3093,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio = r10_bio->devs[1].repl_bio; bio = r10_bio->devs[1].repl_bio;
if (bio) if (bio)
bio->bi_end_io = NULL; bio->bi_end_io = NULL;
rdev = mirror->replacement; /* Note: if mreplace != NULL, then bio
/* Note: if rdev != NULL, then bio
* cannot be NULL as r10buf_pool_alloc will * cannot be NULL as r10buf_pool_alloc will
* have allocated it. * have allocated it.
* So the second test here is pointless. * So the second test here is pointless.
@ -3086,8 +3101,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
* this comment keeps human reviewers * this comment keeps human reviewers
* happy. * happy.
*/ */
if (rdev == NULL || bio == NULL || if (mreplace == NULL || bio == NULL ||
test_bit(Faulty, &rdev->flags)) test_bit(Faulty, &mreplace->flags))
break; break;
bio_reset(bio); bio_reset(bio);
bio->bi_next = biolist; bio->bi_next = biolist;
@ -3096,11 +3111,12 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio->bi_end_io = end_sync_write; bio->bi_end_io = end_sync_write;
bio->bi_rw = WRITE; bio->bi_rw = WRITE;
bio->bi_iter.bi_sector = to_addr + bio->bi_iter.bi_sector = to_addr +
rdev->data_offset; mreplace->data_offset;
bio->bi_bdev = rdev->bdev; bio->bi_bdev = mreplace->bdev;
atomic_inc(&r10_bio->remaining); atomic_inc(&r10_bio->remaining);
break; break;
} }
rcu_read_unlock();
if (j == conf->copies) { if (j == conf->copies) {
/* Cannot recover, so abort the recovery or /* Cannot recover, so abort the recovery or
* record a bad block */ * record a bad block */
@ -3113,15 +3129,15 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
if (r10_bio->devs[k].devnum == i) if (r10_bio->devs[k].devnum == i)
break; break;
if (!test_bit(In_sync, if (!test_bit(In_sync,
&mirror->rdev->flags) &mrdev->flags)
&& !rdev_set_badblocks( && !rdev_set_badblocks(
mirror->rdev, mrdev,
r10_bio->devs[k].addr, r10_bio->devs[k].addr,
max_sync, 0)) max_sync, 0))
any_working = 0; any_working = 0;
if (mirror->replacement && if (mreplace &&
!rdev_set_badblocks( !rdev_set_badblocks(
mirror->replacement, mreplace,
r10_bio->devs[k].addr, r10_bio->devs[k].addr,
max_sync, 0)) max_sync, 0))
any_working = 0; any_working = 0;
@ -3139,8 +3155,14 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
if (rb2) if (rb2)
atomic_dec(&rb2->remaining); atomic_dec(&rb2->remaining);
r10_bio = rb2; r10_bio = rb2;
rdev_dec_pending(mrdev, mddev);
if (mreplace)
rdev_dec_pending(mreplace, mddev);
break; break;
} }
rdev_dec_pending(mrdev, mddev);
if (mreplace)
rdev_dec_pending(mreplace, mddev);
} }
if (biolist == NULL) { if (biolist == NULL) {
while (r10_bio) { while (r10_bio) {
@ -3185,6 +3207,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
int d = r10_bio->devs[i].devnum; int d = r10_bio->devs[i].devnum;
sector_t first_bad, sector; sector_t first_bad, sector;
int bad_sectors; int bad_sectors;
struct md_rdev *rdev;
if (r10_bio->devs[i].repl_bio) if (r10_bio->devs[i].repl_bio)
r10_bio->devs[i].repl_bio->bi_end_io = NULL; r10_bio->devs[i].repl_bio->bi_end_io = NULL;
@ -3192,12 +3215,14 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio = r10_bio->devs[i].bio; bio = r10_bio->devs[i].bio;
bio_reset(bio); bio_reset(bio);
bio->bi_error = -EIO; bio->bi_error = -EIO;
if (conf->mirrors[d].rdev == NULL || rcu_read_lock();
test_bit(Faulty, &conf->mirrors[d].rdev->flags)) rdev = rcu_dereference(conf->mirrors[d].rdev);
if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
rcu_read_unlock();
continue; continue;
}
sector = r10_bio->devs[i].addr; sector = r10_bio->devs[i].addr;
if (is_badblock(conf->mirrors[d].rdev, if (is_badblock(rdev, sector, max_sync,
sector, max_sync,
&first_bad, &bad_sectors)) { &first_bad, &bad_sectors)) {
if (first_bad > sector) if (first_bad > sector)
max_sync = first_bad - sector; max_sync = first_bad - sector;
@ -3205,25 +3230,28 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bad_sectors -= (sector - first_bad); bad_sectors -= (sector - first_bad);
if (max_sync > bad_sectors) if (max_sync > bad_sectors)
max_sync = bad_sectors; max_sync = bad_sectors;
rcu_read_unlock();
continue; continue;
} }
} }
atomic_inc(&conf->mirrors[d].rdev->nr_pending); atomic_inc(&rdev->nr_pending);
atomic_inc(&r10_bio->remaining); atomic_inc(&r10_bio->remaining);
bio->bi_next = biolist; bio->bi_next = biolist;
biolist = bio; biolist = bio;
bio->bi_private = r10_bio; bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_read; bio->bi_end_io = end_sync_read;
bio->bi_rw = READ; bio->bi_rw = READ;
bio->bi_iter.bi_sector = sector + bio->bi_iter.bi_sector = sector + rdev->data_offset;
conf->mirrors[d].rdev->data_offset; bio->bi_bdev = rdev->bdev;
bio->bi_bdev = conf->mirrors[d].rdev->bdev;
count++; count++;
if (conf->mirrors[d].replacement == NULL || rdev = rcu_dereference(conf->mirrors[d].replacement);
test_bit(Faulty, if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
&conf->mirrors[d].replacement->flags)) rcu_read_unlock();
continue; continue;
}
atomic_inc(&rdev->nr_pending);
rcu_read_unlock();
/* Need to set up for writing to the replacement */ /* Need to set up for writing to the replacement */
bio = r10_bio->devs[i].repl_bio; bio = r10_bio->devs[i].repl_bio;
@ -3231,15 +3259,13 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio->bi_error = -EIO; bio->bi_error = -EIO;
sector = r10_bio->devs[i].addr; sector = r10_bio->devs[i].addr;
atomic_inc(&conf->mirrors[d].replacement->nr_pending);
bio->bi_next = biolist; bio->bi_next = biolist;
biolist = bio; biolist = bio;
bio->bi_private = r10_bio; bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_write; bio->bi_end_io = end_sync_write;
bio->bi_rw = WRITE; bio->bi_rw = WRITE;
bio->bi_iter.bi_sector = sector + bio->bi_iter.bi_sector = sector + rdev->data_offset;
conf->mirrors[d].replacement->data_offset; bio->bi_bdev = rdev->bdev;
bio->bi_bdev = conf->mirrors[d].replacement->bdev;
count++; count++;
} }