Re: [dm-devel] [PATCH 2/3] dm raid1: add interface to set resync speed

Re: [dm-devel] [PATCH 2/3] dm raid1: add interface to set resync speed

[Lars Marowsky-Bree]

On 2012-11-22T14:27:52, Guangliang Zhao <gzhao@suse.com> wrote:

Hi Guangliang,

thanks for adding this. I think this approach is a good direction to
take, just one feedback:

> Add ioctl to control resync speed, userspace tool
> is dmsetup message, message format is:
> 	dmsetup message $device 0 "set $speed"
> e.g.
> 	dmsetup message /dev/dm-2 "set 12345"

I think this should be "set-max-resync-rate" or something; "set" is very
generic and not very extensible going forward, should the need arise.

Regards,
    Lars

-- 
Architect Storage/HA
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 21284 (AG Nürnberg)
"Experience is the name everyone gives to their mistakes." -- Oscar Wilde

Re: [dm-devel] [PATCH 2/3] dm raid1: add interface to set resync speed

[Guangliang Zhao]

On Fri, Nov 30, 2012 at 12:39:28PM +0100, Lars Marowsky-Bree wrote:
> On 2012-11-22T14:27:52, Guangliang Zhao <gzhao@suse.com> wrote:
> 
> Hi Guangliang,
> 
> thanks for adding this. I think this approach is a good direction to
> take, just one feedback:
> 
> > Add ioctl to control resync speed, userspace tool
> > is dmsetup message, message format is:
> > 	dmsetup message $device 0 "set $speed"
> > e.g.
> > 	dmsetup message /dev/dm-2 "set 12345"
> 
> I think this should be "set-max-resync-rate" or something; "set" is very
> generic and not very extensible going forward, should the need arise.
That's would be more clear, thanks.

> 
> Regards,
>     Lars
> 
> -- 
> Architect Storage/HA
> SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 21284 (AG Nürnberg)
> "Experience is the name everyone gives to their mistakes." -- Oscar Wilde
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

-- 
Best regards,
Guangliang

Re: [dm-devel] [PATCH 0/3] add resync speed control for dm-raid1

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

On Thu, 22 Nov 2012 14:27:50 +0800 Guangliang Zhao <gzhao@suse.com> wrote:

> Hi,
> 
> These patches are used to add resync speed control for dm-raid1. The
> second and third patch provide support for user-space tool dmsetup.
> 
> Guangliang Zhao (3):
>   dm raid1: add resync speed control for dm-raid1
>   dm raid1: add interface to set resync speed
>   dm raid1: add interface to get resync speed
> 
>  drivers/md/dm-raid1.c |   93 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 90 insertions(+), 3 deletions(-)
> 

The problem with this approach is that it slows down resync even when there
is no other IO happening.
If that is deemed to be acceptable, then the patch set seems fine, though I
would probably make the default a lot higher so as not to change current
default behaviour for anyone.

If it isn't acceptable, then you either need to monitor the number of
requests going to the underlying devices - like md does - or monitor the
number of requests coming in to the dm-raid1 target - which is probably
easier with dm.

i.e. only impose the rate limit if there have been any requests for the
dm-raid1 target in the last 'RESYNC_JIFFIES'.

What do you think of that?

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [dm-devel] [PATCH 0/3] add resync speed control for dm-raid1

[Guangliang Zhao]

On Mon, Dec 10, 2012 at 01:21:23PM +1100, NeilBrown wrote:
> On Thu, 22 Nov 2012 14:27:50 +0800 Guangliang Zhao <gzhao@suse.com> wrote:
> 
> > Hi,
> > 
> > These patches are used to add resync speed control for dm-raid1. The
> > second and third patch provide support for user-space tool dmsetup.
> > 
> > Guangliang Zhao (3):
> >   dm raid1: add resync speed control for dm-raid1
> >   dm raid1: add interface to set resync speed
> >   dm raid1: add interface to get resync speed
> > 
> >  drivers/md/dm-raid1.c |   93 +++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 90 insertions(+), 3 deletions(-)
> > 
> 
> The problem with this approach is that it slows down resync even when there
> is no other IO happening.
> If that is deemed to be acceptable, then the patch set seems fine, though I
> would probably make the default a lot higher so as not to change current
> default behaviour for anyone.
Yes, the users who don't need this feature will not be affected, fully
accepted.

> 
> If it isn't acceptable, then you either need to monitor the number of
> requests going to the underlying devices - like md does - or monitor the
> number of requests coming in to the dm-raid1 target - which is probably
> easier with dm.
> 
> i.e. only impose the rate limit if there have been any requests for the
> dm-raid1 target in the last 'RESYNC_JIFFIES'.
Good idea, that's would be on my TODO list. But anyway these patches are
still required.

> 
> What do you think of that?
> 
> NeilBrown



> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel


-- 
Best regards,
Guangliang

Re: [dm-devel] [PATCH 0/3] add resync speed control for dm-raid1

[Lars Marowsky-Bree]

On 2012-12-10T13:21:23, NeilBrown <neilb@suse.de> wrote:

> The problem with this approach is that it slows down resync even when there
> is no other IO happening.
> If that is deemed to be acceptable, then the patch set seems fine, though I
> would probably make the default a lot higher so as not to change current
> default behaviour for anyone.

I agree to the latter part.

The difficulty is that our primary use case here is preventing IO
starvation while cluster raid is resyncing; and we don't know the IO
load on other nodes, or what other LVs might inflict on the same backend
store / PV. Hence, a static limit probably is the easiest way to
start.

I agree that a more dynamic approach would be desirable, but that
appears to be very complex to get right.


Thanks,
    Lars

-- 
Architect Storage/HA
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 21284 (AG Nürnberg)
"Experience is the name everyone gives to their mistakes." -- Oscar Wilde