dm raid: fix rs_get_progress() synchronization state/ratio
authorHeinz Mauelshagen <heinzm@redhat.com>
Sat, 2 Dec 2017 00:03:55 +0000 (01:03 +0100)
committerMike Snitzer <snitzer@redhat.com>
Fri, 8 Dec 2017 15:59:58 +0000 (10:59 -0500)
Fix various sync state issues causing racy/bogus sync ratio,
sync_action ad health chars in dm_status() info output.

Sync ratio could be N/N (i.e. 100%) shortly after raid set
creation, i.e. creating a new RaidLV or upconverting a linear LV to
raid1 thus:
  "0 2097152 raid raid1 2 Aa 2097162/2097152 recover 0 0 -"
instead of:
  "0 2097152 raid raid1 2 Aa 0/2097152 idle 0 0 -"

Sync action could be non-idle, when the MD thread was done with io.

Health chars could be 'A' when they should be 'a' for a short time
before a resynchonization started.

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
drivers/md/dm-raid.c

index 5730b32034aaba4e7a3d5bd8303d5814f2369544..7e7075fb9c28fcf54ecc1aae4e0b509787120c25 100644 (file)
@@ -210,6 +210,7 @@ struct raid_dev {
 #define RT_FLAG_RESHAPE_RS             4
 #define RT_FLAG_RS_SUSPENDED           5
 #define RT_FLAG_RS_IN_SYNC             6
+#define RT_FLAG_RS_RESYNCING           7
 
 /* Array elements of 64 bit needed for rebuild/failed disk bits */
 #define DISKS_ARRAY_ELEMS ((MAX_RAID_DEVICES + (sizeof(uint64_t) * 8 - 1)) / sizeof(uint64_t) / 8)
@@ -3306,8 +3307,10 @@ static const char *decipher_sync_action(struct mddev *mddev, unsigned long recov
        if (test_bit(MD_RECOVERY_FROZEN, &recovery))
                return "frozen";
 
-       if (test_bit(MD_RECOVERY_RUNNING, &recovery) ||
-           (!mddev->ro && test_bit(MD_RECOVERY_NEEDED, &recovery))) {
+       /* The MD sync thread can be done with io but still be running */
+       if (!test_bit(MD_RECOVERY_DONE, &recovery) &&
+           (test_bit(MD_RECOVERY_RUNNING, &recovery) ||
+            (!mddev->ro && test_bit(MD_RECOVERY_NEEDED, &recovery)))) {
                if (test_bit(MD_RECOVERY_RESHAPE, &recovery))
                        return "reshape";
 
@@ -3344,8 +3347,9 @@ static const char *__raid_dev_status(struct raid_set *rs, struct md_rdev *rdev)
                return "D";
        else if (test_bit(Journal, &rdev->flags))
                return (rs->journal_dev.mode == R5C_JOURNAL_MODE_WRITE_THROUGH) ? "A" : "a";
-       else if (!test_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags) &&
-                !test_bit(In_sync, &rdev->flags))
+       else if (test_bit(RT_FLAG_RS_RESYNCING, &rs->runtime_flags) ||
+                (!test_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags) &&
+                 !test_bit(In_sync, &rdev->flags)))
                return "a";
        else
                return "A";
@@ -3355,49 +3359,70 @@ static const char *__raid_dev_status(struct raid_set *rs, struct md_rdev *rdev)
 static sector_t rs_get_progress(struct raid_set *rs, unsigned long recovery,
                                sector_t resync_max_sectors)
 {
-       sector_t r, curr_resync_completed;
+       sector_t r;
        struct mddev *mddev = &rs->md;
 
        clear_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
-
-       curr_resync_completed = mddev->curr_resync_completed ?: mddev->recovery_cp;
+       clear_bit(RT_FLAG_RS_RESYNCING, &rs->runtime_flags);
 
        if (rs_is_raid0(rs)) {
                r = resync_max_sectors;
                set_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
 
        } else {
-               r = mddev->reshape_position;
-
                /* Reshape is relative to the array size */
-               if (test_bit(MD_RECOVERY_RESHAPE, &recovery) ||
-                   r != MaxSector) {
-                       if (r == MaxSector) {
-                               set_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
-                               r = resync_max_sectors;
-                       } else {
+               if (test_bit(MD_RECOVERY_RESHAPE, &recovery)) {
+                       r = mddev->reshape_position;
+                       if (r != MaxSector) {
                                /* Got to reverse on backward reshape */
                                if (mddev->reshape_backwards)
                                        r = mddev->array_sectors - r;
 
-                               /* Devide by # of data stripes */
-                               sector_div(r, mddev_data_stripes(rs));
+                               /* Divide by # of data stripes unless raid1 */
+                               if (!rs_is_raid1(rs))
+                                       sector_div(r, mddev_data_stripes(rs));
                        }
 
-               /* Sync is relative to the component device size */
-               } else if (test_bit(MD_RECOVERY_RUNNING, &recovery))
-                       r = curr_resync_completed;
+               /*
+                * Sync/recover is relative to the component device size.
+                *
+                * MD_RECOVERY_NEEDED for https://bugzilla.redhat.com/show_bug.cgi?id=1508070
+                */
+               } else if (test_bit(MD_RECOVERY_NEEDED, &recovery) ||
+                          test_bit(MD_RECOVERY_RUNNING, &recovery))
+                       r = mddev->curr_resync_completed;
+
                else
                        r = mddev->recovery_cp;
 
-               if ((r == MaxSector) ||
-                   (test_bit(MD_RECOVERY_DONE, &recovery) &&
-                    (mddev->curr_resync_completed == resync_max_sectors))) {
+               if (r >= resync_max_sectors &&
+                   (!test_bit(MD_RECOVERY_REQUESTED, &recovery) ||
+                    (!test_bit(MD_RECOVERY_FROZEN, &recovery) &&
+                     !test_bit(MD_RECOVERY_NEEDED, &recovery) &&
+                     !test_bit(MD_RECOVERY_RUNNING, &recovery)))) {
                        /*
                         * Sync complete.
                         */
-                       set_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
-                       r = resync_max_sectors;
+                       /* In case we have finished recovering, the array is in sync. */
+                       if (test_bit(MD_RECOVERY_RECOVER, &recovery))
+                               set_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
+
+               } else if (test_bit(MD_RECOVERY_RECOVER, &recovery)) {
+                       /*
+                        * In case we are recovering, the array is not in sync
+                        * and health chars should show the recovering legs.
+                        */
+                       ;
+
+               } else if (test_bit(MD_RECOVERY_SYNC, &recovery) &&
+                          !test_bit(MD_RECOVERY_REQUESTED, &recovery)) {
+                       /*
+                        * If "resync" is occurring, the raid set
+                        * is or may be out of sync hence the health
+                        * characters shall be 'a'.
+                        */
+                       set_bit(RT_FLAG_RS_RESYNCING, &rs->runtime_flags);
+
                } else if (test_bit(MD_RECOVERY_REQUESTED, &recovery)) {
                        /*
                         * If "check" or "repair" is occurring, the raid set has
@@ -3405,26 +3430,34 @@ static sector_t rs_get_progress(struct raid_set *rs, unsigned long recovery,
                         * should not be 'a' anymore.
                         */
                        set_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
+
                } else {
                        struct md_rdev *rdev;
 
+                       /*
+                        * We are idle and recovery is needed, prevent 'A' chars race
+                        * caused by components still set to in-sync by constrcuctor.
+                        */
+                       if (test_bit(MD_RECOVERY_NEEDED, &recovery))
+                               set_bit(RT_FLAG_RS_RESYNCING, &rs->runtime_flags);
+
                        /*
                         * The raid set may be doing an initial sync, or it may
                         * be rebuilding individual components.  If all the
                         * devices are In_sync, then it is the raid set that is
                         * being initialized.
                         */
+                       set_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
                        rdev_for_each(rdev, mddev)
                                if (!test_bit(Journal, &rdev->flags) &&
-                                   !test_bit(In_sync, &rdev->flags))
-                                       set_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
-#if 0
-                       r = 0; /* HM FIXME: TESTME: https://bugzilla.redhat.com/show_bug.cgi?id=1210637 ? */
-#endif
+                                   !test_bit(In_sync, &rdev->flags)) {
+                                       clear_bit(RT_FLAG_RS_IN_SYNC, &rs->runtime_flags);
+                                       break;
+                               }
                }
        }
 
-       return r;
+       return min(r, resync_max_sectors);
 }
 
 /* Helper to return @dev name or "-" if !@dev */