From b0920ede081b3f1659872f80ce552305610675a6 Mon Sep 17 00:00:00 2001 From: Logan Gunthorpe Date: Thu, 7 Apr 2022 10:57:09 -0600 Subject: md/raid5: Add __rcu annotation to struct disk_info rdev and replacement are protected in some circumstances with rcu_dereference and synchronize_rcu (in raid5_remove_disk()). However, they were not annotated with __rcu so a sparse warning is emitted for every rcu_dereference() call. Add the __rcu annotation and fix up the initialization with RCU_INIT_POINTER, all pointer modifications with rcu_assign_pointer(), a few cases where the pointer value is tested with rcu_access_pointer() and one case where READ_ONCE() is used instead of rcu_dereference(), a case in print_raid5_conf() that should have rcu_dereference() and rcu_read_[un]lock() calls. Additional sparse issues will be fixed up in further commits. Signed-off-by: Logan Gunthorpe Reviewed-by: Christoph Hellwig Signed-off-by: Song Liu --- drivers/md/raid5.c | 46 ++++++++++++++++++++++++++-------------------- drivers/md/raid5.h | 3 ++- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index 79b03c79c66f..c4051625d293 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -6285,7 +6285,7 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n */ rcu_read_lock(); for (i = 0; i < conf->raid_disks; i++) { - struct md_rdev *rdev = READ_ONCE(conf->disks[i].rdev); + struct md_rdev *rdev = rcu_dereference(conf->disks[i].rdev); if (rdev == NULL || test_bit(Faulty, &rdev->flags)) still_degraded = 1; @@ -7317,11 +7317,11 @@ static struct r5conf *setup_conf(struct mddev *mddev) if (test_bit(Replacement, &rdev->flags)) { if (disk->replacement) goto abort; - disk->replacement = rdev; + RCU_INIT_POINTER(disk->replacement, rdev); } else { if (disk->rdev) goto abort; - disk->rdev = rdev; + RCU_INIT_POINTER(disk->rdev, rdev); } if (test_bit(In_sync, &rdev->flags)) { @@ -7628,11 +7628,11 @@ static int raid5_run(struct mddev *mddev) rdev = conf->disks[i].replacement; conf->disks[i].replacement = NULL; clear_bit(Replacement, &rdev->flags); - conf->disks[i].rdev = rdev; + rcu_assign_pointer(conf->disks[i].rdev, rdev); } if (!rdev) continue; - if (conf->disks[i].replacement && + if (rcu_access_pointer(conf->disks[i].replacement) && conf->reshape_progress != MaxSector) { /* replacements and reshape simply do not mix. */ pr_warn("md: cannot handle concurrent replacement and reshape.\n"); @@ -7829,8 +7829,8 @@ static void raid5_status(struct seq_file *seq, struct mddev *mddev) static void print_raid5_conf (struct r5conf *conf) { + struct md_rdev *rdev; int i; - struct disk_info *tmp; pr_debug("RAID conf printout:\n"); if (!conf) { @@ -7841,14 +7841,16 @@ static void print_raid5_conf (struct r5conf *conf) conf->raid_disks, conf->raid_disks - conf->mddev->degraded); + rcu_read_lock(); for (i = 0; i < conf->raid_disks; i++) { char b[BDEVNAME_SIZE]; - tmp = conf->disks + i; - if (tmp->rdev) + rdev = rcu_dereference(conf->disks[i].rdev); + if (rdev) pr_debug(" disk %d, o:%d, dev:%s\n", - i, !test_bit(Faulty, &tmp->rdev->flags), - bdevname(tmp->rdev->bdev, b)); + i, !test_bit(Faulty, &rdev->flags), + bdevname(rdev->bdev, b)); } + rcu_read_unlock(); } static int raid5_spare_active(struct mddev *mddev) @@ -7899,8 +7901,9 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev) struct r5conf *conf = mddev->private; int err = 0; int number = rdev->raid_disk; - struct md_rdev **rdevp; + struct md_rdev __rcu **rdevp; struct disk_info *p = conf->disks + number; + struct md_rdev *tmp; print_raid5_conf(conf); if (test_bit(Journal, &rdev->flags) && conf->log) { @@ -7918,9 +7921,9 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev) log_exit(conf); return 0; } - if (rdev == p->rdev) + if (rdev == rcu_access_pointer(p->rdev)) rdevp = &p->rdev; - else if (rdev == p->replacement) + else if (rdev == rcu_access_pointer(p->replacement)) rdevp = &p->replacement; else return 0; @@ -7940,7 +7943,8 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev) if (!test_bit(Faulty, &rdev->flags) && mddev->recovery_disabled != conf->recovery_disabled && !has_failed(conf) && - (!p->replacement || p->replacement == rdev) && + (!rcu_access_pointer(p->replacement) || + rcu_access_pointer(p->replacement) == rdev) && number < conf->raid_disks) { err = -EBUSY; goto abort; @@ -7951,7 +7955,7 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev) if (atomic_read(&rdev->nr_pending)) { /* lost the race, try later */ err = -EBUSY; - *rdevp = rdev; + rcu_assign_pointer(*rdevp, rdev); } } if (!err) { @@ -7959,17 +7963,19 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev) if (err) goto abort; } - if (p->replacement) { + + tmp = rcu_access_pointer(p->replacement); + if (tmp) { /* We must have just cleared 'rdev' */ - p->rdev = p->replacement; - clear_bit(Replacement, &p->replacement->flags); + rcu_assign_pointer(p->rdev, tmp); + clear_bit(Replacement, &tmp->flags); smp_mb(); /* Make sure other CPUs may see both as identical * but will never see neither - if they are careful */ - p->replacement = NULL; + rcu_assign_pointer(p->replacement, NULL); if (!err) - err = log_modify(conf, p->rdev, true); + err = log_modify(conf, tmp, true); } clear_bit(WantReplacement, &rdev->flags); diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h index 61bc2e1f1b4e..638d29863503 100644 --- a/drivers/md/raid5.h +++ b/drivers/md/raid5.h @@ -473,7 +473,8 @@ enum { */ struct disk_info { - struct md_rdev *rdev, *replacement; + struct md_rdev __rcu *rdev; + struct md_rdev __rcu *replacement; struct page *extra_page; /* extra page to use in prexor */ }; -- cgit v1.2.3