md: reduce the number of synchronize_rcu() calls when multiple devices fail.

Every time a device is removed with ->hot_remove_disk() a synchronize_rcu() call is made
which can delay several milliseconds in some case.
If lots of devices fail at once - as could happen with a large RAID10 where one set
of devices are removed all at once - these delays can add up to be very inconcenient.

As failure is not reversible we can check for that first, setting a
separate flag if it is found, and then all synchronize_rcu() once for
all the flagged devices.  Then ->hot_remove_disk() function can skip the
synchronize_rcu() step if the flag is set.

fix build error(Shaohua)
Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: Shaohua Li <shli@fb.com>

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0793754eeffdfe61f7f2e439df16de28e46c4117..2ed547f5c3b6dbc1a9f99af99e862cdd22cbea08 100644 (file)
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8180,15 +8180,34 @@ static int remove_and_add_spares(struct mddev *mddev,
        struct md_rdev *rdev;
        int spares = 0;
        int removed = 0;
+       bool remove_some = false;
 
-       rdev_for_each(rdev, mddev)
+       rdev_for_each(rdev, mddev) {
+               if ((this == NULL || rdev == this) &&
+                   rdev->raid_disk >= 0 &&
+                   !test_bit(Blocked, &rdev->flags) &&
+                   test_bit(Faulty, &rdev->flags) &&
+                   atomic_read(&rdev->nr_pending)==0) {
+                       /* Faulty non-Blocked devices with nr_pending == 0
+                        * never get nr_pending incremented,
+                        * never get Faulty cleared, and never get Blocked set.
+                        * So we can synchronize_rcu now rather than once per device
+                        */
+                       remove_some = true;
+                       set_bit(RemoveSynchronized, &rdev->flags);
+               }
+       }
+
+       if (remove_some)
+               synchronize_rcu();
+       rdev_for_each(rdev, mddev) {
                if ((this == NULL || rdev == this) &&
                    rdev->raid_disk >= 0 &&
                    !test_bit(Blocked, &rdev->flags) &&
-                   (test_bit(Faulty, &rdev->flags) ||
+                   ((test_bit(RemoveSynchronized, &rdev->flags) ||
                     (!test_bit(In_sync, &rdev->flags) &&
                      !test_bit(Journal, &rdev->flags))) &&
-                   atomic_read(&rdev->nr_pending)==0) {
+                   atomic_read(&rdev->nr_pending)==0)) {
                        if (mddev->pers->hot_remove_disk(
                                    mddev, rdev) == 0) {
                                sysfs_unlink_rdev(mddev, rdev);
@@ -8196,6 +8215,10 @@ static int remove_and_add_spares(struct mddev *mddev,
                                removed++;
                        }
                }
+               if (remove_some && test_bit(RemoveSynchronized, &rdev->flags))
+                       clear_bit(RemoveSynchronized, &rdev->flags);
+       }
+
        if (removed && mddev->kobj.sd)
                sysfs_notify(&mddev->kobj, NULL, "degraded");
 

diff --git a/drivers/md/md.h b/drivers/md/md.h
index 03b19aad4921fcdaa8a049041b028cd395c3a70a..dc65ca65b26e03744b76ff10750fca6afa4689f7 100644 (file)
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -163,6 +163,11 @@ enum flag_bits {
                                 * than other devices in the array
                                 */
        ClusterRemove,
+       RemoveSynchronized,     /* synchronize_rcu() was called after
+                                * this device was known to be faulty,
+                                * so it is safe to remove without
+                                * another synchronize_rcu() call.
+                                */
 };
 
 static inline int is_badblock(struct md_rdev *rdev, sector_t s, int sectors,

diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index 7eb9972a37e6b7fcfe17b377bf8ef26e95fd1c78..c145a5a114eb4ead6e92e02b169af8b6091a675e 100644 (file)
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -298,12 +298,14 @@ static int multipath_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
                        goto abort;
                }
                p->rdev = NULL;
-               synchronize_rcu();
-               if (atomic_read(&rdev->nr_pending)) {
-                       /* lost the race, try later */
-                       err = -EBUSY;
-                       p->rdev = rdev;
-                       goto abort;
+               if (!test_bit(RemoveSynchronized, &rdev->flags)) {
+                       synchronize_rcu();
+                       if (atomic_read(&rdev->nr_pending)) {
+                               /* lost the race, try later */
+                               err = -EBUSY;
+                               p->rdev = rdev;
+                               goto abort;
+                       }
                }
                err = md_integrity_register(mddev);
        }

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 34f20c03d1f6b7d6f78ff64f552a71d74cd54935..5027ef4752acbc8429d61dd97b197160f4631b93 100644 (file)
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1656,13 +1656,16 @@ static int raid1_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
                        goto abort;
                }
                p->rdev = NULL;
-               synchronize_rcu();
-               if (atomic_read(&rdev->nr_pending)) {
-                       /* lost the race, try later */
-                       err = -EBUSY;
-                       p->rdev = rdev;
-                       goto abort;
-               } else if (conf->mirrors[conf->raid_disks + number].rdev) {
+               if (!test_bit(RemoveSynchronized, &rdev->flags)) {
+                       synchronize_rcu();
+                       if (atomic_read(&rdev->nr_pending)) {
+                               /* lost the race, try later */
+                               err = -EBUSY;
+                               p->rdev = rdev;
+                               goto abort;
+                       }
+               }
+               if (conf->mirrors[conf->raid_disks + number].rdev) {
                        /* We just removed a device that is being replaced.
                         * Move down the replacement.  We drain all IO before
                         * doing this to avoid confusion.

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 34facda18e7250b42809eee761df8249380308fa..8ee5d96e6a2d88155e9a26e5b711ace69c4a82ba 100644 (file)
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1766,7 +1766,7 @@ static int raid10_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
                err = -EBUSY;
                goto abort;
        }
-       /* Only remove faulty devices if recovery
+       /* Only remove non-faulty devices if recovery
         * is not possible.
         */
        if (!test_bit(Faulty, &rdev->flags) &&
@@ -1778,13 +1778,16 @@ static int raid10_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
                goto abort;
        }
        *rdevp = NULL;
-       synchronize_rcu();
-       if (atomic_read(&rdev->nr_pending)) {
-               /* lost the race, try later */
-               err = -EBUSY;
-               *rdevp = rdev;
-               goto abort;
-       } else if (p->replacement) {
+       if (!test_bit(RemoveSynchronized, &rdev->flags)) {
+               synchronize_rcu();
+               if (atomic_read(&rdev->nr_pending)) {
+                       /* lost the race, try later */
+                       err = -EBUSY;
+                       *rdevp = rdev;
+                       goto abort;
+               }
+       }
+       if (p->replacement) {
                /* We must have just cleared 'rdev' */
                p->rdev = p->replacement;
                clear_bit(Replacement, &p->replacement->flags);

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index f6a191aaaa91b51d7b953bc72c1b05e226a1be66..413cc7d847dab1b942a2be41403dc64952da578e 100644 (file)
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7197,12 +7197,15 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
                goto abort;
        }
        *rdevp = NULL;
-       synchronize_rcu();
-       if (atomic_read(&rdev->nr_pending)) {
-               /* lost the race, try later */
-               err = -EBUSY;
-               *rdevp = rdev;
-       } else if (p->replacement) {
+       if (!test_bit(RemoveSynchronized, &rdev->flags)) {
+               synchronize_rcu();
+               if (atomic_read(&rdev->nr_pending)) {
+                       /* lost the race, try later */
+                       err = -EBUSY;
+                       *rdevp = rdev;
+               }
+       }
+       if (p->replacement) {
                /* We must have just cleared 'rdev' */
                p->rdev = p->replacement;
                clear_bit(Replacement, &p->replacement->flags);