Re: [PATCH] Recovery speed is wrong

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 7 Aug 2014 09:37:41 -0400 (EDT) Xiao Ni <xni@xxxxxxxxxx> wrote:

> Hi all
> 
>    When we calculate the speed of recovery, the numerator that contains the recovery done sectors.
> It's need to subtract the sectors which don't finish recovery.

Thanks,
I've applied the patch.  However it needed some fixing up first.  If you are
going to be sending more patches, you need to get these details right.

I suggest using "quilt" to create patches.  There are other options, I mostly
use 'git' and 'stg' myself, but quilt is easy to get started with:
> 
> 
> Signed-off-by: Xiao Ni <xni@xxxxxxxxxx>
> 
> diff -urN linux-stable/drivers/md/md.c fix/md.c
> --- linux-stable/drivers/md/md.c 2014-07-30 14:36:37.327535805 +0800
> +++ fix/md.c   2014-08-07 16:07:12.559503942 +0800

This makes it look like the file "fix/md.c" is being patched, which confuses
programs that apply patches (like "git am").
If you really want to do it by hand, then:

  cp drivers/md/md.c drivers/md/md.c.orig
  # edit drivers/md/md.c
  # compile and test
  diff -u drivers/md/md.c.orig drivers/md/md.c > /tmp/diff

and post /tmp/diff.

> @@ -7376,7 +7376,7 @@
>    struct mddev *mddev2;
>    unsigned int currspeed = 0,
>        window;
> -  sector_t max_sectors,j, io_sectors;
> +  sector_t max_sectors,j, io_sectors, recovery_done;
>    unsigned long mark[SYNC_MARKS];
>    unsigned long update_time;
>    sector_t mark_cnt[SYNC_MARKS];
 ^^^^

There are spaces here.  They should be 'tabs'.  Probably your mail program
thinks it knows better and is converting spaces to tabs.
If you cannot get it not to do that, then add the patch as an attachment.
I MUCH prefer patches to be inline in the text, not as an attachment, but an
attachment is MUCH better than converting all the tabs to spaces.

> @@ -7652,7 +7652,8 @@
>        */  
>       cond_resched();
>  
> -     currspeed = ((unsigned long)(io_sectors-mddev->resync_mark_cnt))/2
> +     recovery_done = io_sectors - atomic_read(&mddev->recovery_active);
> +     currspeed = ((unsigned long)recovery_done - mddev->resync_mark_cnt)/2
>          /((jiffies-mddev->resync_mark)/HZ +1) +1;

I also kept the inner brackets, so that reads:

> +     currspeed = ((unsigned long)(recovery_done - mddev->resync_mark_cnt))/2

I think the extra brackets make it clear that it is the difference that needs
to be cast, not just the first value.  The result is the same, but this very
is (to me) more readable.

Thanks,
NeilBrown


>  
>       if (currspeed > speed_min(mddev)) {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux