forked from mirrors/linux
		
	md/raid10: add rcu protection to rdev access in raid10_sync_request.
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:
		
							parent
							
								
									d44b0a928f
								
							
						
					
					
						commit
						f90145f317
					
				
					 1 changed files with 74 additions and 48 deletions
				
			
		| 
						 | 
				
			
			@ -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;
 | 
			
		||||
		}
 | 
			
		||||
| 
						 | 
				
			
			@ -2941,14 +2944,19 @@ 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)) &&
 | 
			
		||||
			    (mreplace == NULL ||
 | 
			
		||||
			     test_bit(Faulty, &mreplace->flags))) {
 | 
			
		||||
				rcu_read_unlock();
 | 
			
		||||
				continue;
 | 
			
		||||
			}
 | 
			
		||||
 | 
			
		||||
			still_degraded = 0;
 | 
			
		||||
			/* 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
 | 
			
		||||
				 * try to recover this sector.
 | 
			
		||||
				 */
 | 
			
		||||
				rcu_read_unlock();
 | 
			
		||||
				continue;
 | 
			
		||||
			}
 | 
			
		||||
			/* 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)
 | 
			
		||||
				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;
 | 
			
		||||
| 
						 | 
				
			
			@ -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
 | 
			
		||||
			 * 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);
 | 
			
		||||
| 
						 | 
				
			
			@ -3010,15 +3027,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,
 | 
			
		||||
| 
						 | 
				
			
			@ -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].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;
 | 
			
		||||
| 
						 | 
				
			
			@ -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_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;
 | 
			
		||||
| 
						 | 
				
			
			@ -3077,8 +3093,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.
 | 
			
		||||
| 
						 | 
				
			
			@ -3086,8 +3101,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;
 | 
			
		||||
| 
						 | 
				
			
			@ -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_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 */
 | 
			
		||||
| 
						 | 
				
			
			@ -3113,15 +3129,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;
 | 
			
		||||
| 
						 | 
				
			
			@ -3139,8 +3155,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) {
 | 
			
		||||
| 
						 | 
				
			
			@ -3185,6 +3207,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;
 | 
			
		||||
| 
						 | 
				
			
			@ -3192,12 +3215,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;
 | 
			
		||||
| 
						 | 
				
			
			@ -3205,25 +3230,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;
 | 
			
		||||
| 
						 | 
				
			
			@ -3231,15 +3259,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++;
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue