md: fix for divide error in status_resync
authorMariusz Tkaczyk <mariusz.tkaczyk@intel.com>
Thu, 13 Jun 2019 14:11:41 +0000 (16:11 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sun, 21 Jul 2019 07:04:26 +0000 (09:04 +0200)
[ Upstream commit 9642fa73d073527b0cbc337cc17a47d545d82cd2 ]

Stopping external metadata arrays during resync/recovery causes
retries, loop of interrupting and starting reconstruction, until it
hit at good moment to stop completely. While these retries
curr_mark_cnt can be small- especially on HDD drives, so subtraction
result can be smaller than 0. However it is casted to uint without
checking. As a result of it the status bar in /proc/mdstat while stopping
is strange (it jumps between 0% and 99%).

The real problem occurs here after commit 72deb455b5ec ("block: remove
CONFIG_LBDAF"). Sector_div() macro has been changed, now the
divisor is casted to uint32. For db = -8 the divisior(db/32-1) becomes 0.

Check if db value can be really counted and replace these macro by
div64_u64() inline.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
drivers/md/md.c

index b27a69388dcd918287e62cc11cbfdb89e855c3bd..764ed9c4662949f47ea00cc6dfb767b6c50073f9 100644 (file)
@@ -7605,9 +7605,9 @@ static void status_unused(struct seq_file *seq)
 static int status_resync(struct seq_file *seq, struct mddev *mddev)
 {
        sector_t max_sectors, resync, res;
-       unsigned long dt, db;
-       sector_t rt;
-       int scale;
+       unsigned long dt, db = 0;
+       sector_t rt, curr_mark_cnt, resync_mark_cnt;
+       int scale, recovery_active;
        unsigned int per_milli;
 
        if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery) ||
@@ -7677,22 +7677,30 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
         * db: blocks written from mark until now
         * rt: remaining time
         *
-        * rt is a sector_t, so could be 32bit or 64bit.
-        * So we divide before multiply in case it is 32bit and close
-        * to the limit.
-        * We scale the divisor (db) by 32 to avoid losing precision
-        * near the end of resync when the number of remaining sectors
-        * is close to 'db'.
-        * We then divide rt by 32 after multiplying by db to compensate.
-        * The '+1' avoids division by zero if db is very small.
+        * rt is a sector_t, which is always 64bit now. We are keeping
+        * the original algorithm, but it is not really necessary.
+        *
+        * Original algorithm:
+        *   So we divide before multiply in case it is 32bit and close
+        *   to the limit.
+        *   We scale the divisor (db) by 32 to avoid losing precision
+        *   near the end of resync when the number of remaining sectors
+        *   is close to 'db'.
+        *   We then divide rt by 32 after multiplying by db to compensate.
+        *   The '+1' avoids division by zero if db is very small.
         */
        dt = ((jiffies - mddev->resync_mark) / HZ);
        if (!dt) dt++;
-       db = (mddev->curr_mark_cnt - atomic_read(&mddev->recovery_active))
-               - mddev->resync_mark_cnt;
+
+       curr_mark_cnt = mddev->curr_mark_cnt;
+       recovery_active = atomic_read(&mddev->recovery_active);
+       resync_mark_cnt = mddev->resync_mark_cnt;
+
+       if (curr_mark_cnt >= (recovery_active + resync_mark_cnt))
+               db = curr_mark_cnt - (recovery_active + resync_mark_cnt);
 
        rt = max_sectors - resync;    /* number of remaining sectors */
-       sector_div(rt, db/32+1);
+       rt = div64_u64(rt, db/32+1);
        rt *= dt;
        rt >>= 5;