leandrof/linux
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

md: fix for divide error in status_resync

Browse Source 
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 72deb455b5 ("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>
This commit is contained in:
Mariusz Tkaczyk
2019-06-13 16:11:41 +02:00
committed by Song Liu
parent 4569180495
commit 9642fa73d0
1 changed files with 22 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
drivers/md/md.c
View File

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