[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-raid
Subject:    Re: [PATCH] md: fix for divide error in status_resync
From:       Song Liu <liu.song.a23 () gmail ! com>
Date:       2019-06-18 15:21:31
Message-ID: CAPhsuW5pE1fvMqoOe5e=4o=zV3a3aY=WG0GRVoEV9sOzbo4h3g () mail ! gmail ! com
[Download RAW message or body]

On Fri, Jun 14, 2019 at 2:48 PM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Thu, Jun 13, 2019 at 8:09 AM Mariusz Tkaczyk
> <mariusz.tkaczyk@intel.com> 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@intel.com>
>
> 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).

Never mind, I fixed the comments while applying the patch.

Song
[prev in list] [next in list] [prev in thread] [next in thread]