On Thu, Jun 13, 2019 at 8:09 AM Mariusz Tkaczyk
<mariusz.tkaczyk@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxx>
This looks good! Thanks for the fix.
> ---
> drivers/md/md.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 04f4f131f9d6..9a8b258ce1ef 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7607,9 +7607,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) ||
> @@ -7709,11 +7709,16 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
> */
Could you please also update comments before this section? (sector_t
is always u64 now).
Thanks,
Song
> 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;
>
> --
> 2.16.4
>
