Re: [k-ueda@ct.jp.nec.com: Re: request-based dm-multipath]

Re: [k-ueda@ct.jp.nec.com: Re: request-based dm-multipath]

[Mikulas Patocka]

On Fri, 10 Apr 2009, Mike Snitzer wrote:

> Hi Mikulaus,
> 
> Figured I'd give you this heads up on the request-based multipath
> patches too considering your recent "bottom-layer barrier support"
> patchset (where you said multipath support is coming later).
> 
> We likely want to coordinate with the NEC guys so as to make sure things
> are in order for the request-based patches to get merged along with your
> remaining barrier work for 2.6.31.
> 
> Mike
> 
> p.s. below you can see I mistakenly said to Kiyoshi that the recent
> barrier patches that got merged upstream were "the last of the DM
> barrier support"...

Hi

I would say one thing about the request-based patches --- don't do this.

Your patch adds an alternate I/O path to request processing on device 
mapper.

So, with your patch, there will be two I/O request paths. It means that 
any work on generic device-mapper code that will have to be done in the 
future (such as for example barriers that I did) will be twice harder. It 
will take twice the time to understand request processing, twice brain 
capacity to remember it, twice the time for coding, twice the time for 
code review, twice the time for testing.

If the patch goes in, it will make a lot of things twice harder. And once 
the patch is in productive kernels, there'd be very little possibility to 
pull it out.

What is the exact reason for your patch? I suppose that it's some 
performance degradation caused by the fact that dm-multipath doesn't 
distributes requests optimally across both paths. dm-multipath has 
pluggable path selectors, so you could improve dm-round-robin.c (or write 
alternate path selector module) and you don't have to touch generic dm 
code to solve this problem.

The point is that improving dm-multipath target with better path selector 
is much less intrusive than patching device mapper core. If you improve 
dm-multipath target, only people hacking on dm-multipath will have to 
learn about your code. If you modify generic dm.c file, anyone doing 
anything on device mapper must learn about your code --- so human time 
consumed in much worse in this case.

So, try the alternate solution (write new path selector for dm-multipath) 
and then you can compare them and see the result --- and then it can be 
consisdered if the high human time consumed by patching dm.c is worth the 
performance improvement.

Mikulas

> On Fri, Apr 10 2009 at 11:19am -0400,
> Mike Snitzer <snitzer@redhat.com> wrote:
> 
> > FYI
> > 
> > ----- Forwarded message from Kiyoshi Ueda <k-ueda@ct.jp.nec.com> -----
> > 
> > > From: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
> > > Date: Fri, 10 Apr 2009 15:08:06 +0900
> > > To: Mike Snitzer <snitzer@redhat.com>
> > > CC: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> > > Subject: Re: request-based dm-multipath
> > > 
> > > Hi Mike,
> > > 
> > > > 
> > > > Hi Kiyoshi,
> > > > 
> > > > I just wanted to follow-up with you to understand if you'll be posting
> > > > the request-based patches rebased against 2.6.30-rcX.  Please note that
> > > > Alasdair just requested Linus pull the last of the DM barrier support
> > > > into what will be 2.6.30-rc2.
> > > 
> > > Yes.
> > > Current request-based patch does't have barrier support.
> > > So I think I have to implement barrier support to post the next update.
> > > 
> > > > I'd imagine you'd like the merging of the request-based and dynamic load
> > > > balancer patches to be a priority for 2.6.31?
> > > 
> > > Yes.  That's right.
> > > 
> > > Thanks,
> > > Kiyoshi Ueda
> > 
> > ----- End forwarded message -----
> 

Re: [k-ueda@ct.jp.nec.com: Re: request-based dm-multipath]

[Mike Snitzer]

On Wed, Apr 15 2009 at  3:09pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> On Fri, 10 Apr 2009, Mike Snitzer wrote:
> 
> > Hi Mikulaus,
> > 
> > Figured I'd give you this heads up on the request-based multipath
> > patches too considering your recent "bottom-layer barrier support"
> > patchset (where you said multipath support is coming later).
> > 
> > We likely want to coordinate with the NEC guys so as to make sure things
> > are in order for the request-based patches to get merged along with your
> > remaining barrier work for 2.6.31.
> > 
> > Mike
> > 
> > p.s. below you can see I mistakenly said to Kiyoshi that the recent
> > barrier patches that got merged upstream were "the last of the DM
> > barrier support"...
> 
> Hi
> 
> I would say one thing about the request-based patches --- don't do this.
> 
> Your patch adds an alternate I/O path to request processing on device 
> mapper.
> 
> So, with your patch, there will be two I/O request paths. It means that 
> any work on generic device-mapper code that will have to be done in the 
> future (such as for example barriers that I did) will be twice harder. It 
> will take twice the time to understand request processing, twice brain 
> capacity to remember it, twice the time for coding, twice the time for 
> code review, twice the time for testing.
> 
> If the patch goes in, it will make a lot of things twice harder. And once 
> the patch is in productive kernels, there'd be very little possibility to 
> pull it out.
> 
> What is the exact reason for your patch? I suppose that it's some 
> performance degradation caused by the fact that dm-multipath doesn't 
> distributes requests optimally across both paths. dm-multipath has 
> pluggable path selectors, so you could improve dm-round-robin.c (or write 
> alternate path selector module) and you don't have to touch generic dm 
> code to solve this problem.
> 
> The point is that improving dm-multipath target with better path selector 
> is much less intrusive than patching device mapper core. If you improve 
> dm-multipath target, only people hacking on dm-multipath will have to 
> learn about your code. If you modify generic dm.c file, anyone doing 
> anything on device mapper must learn about your code --- so human time 
> consumed in much worse in this case.
> 
> So, try the alternate solution (write new path selector for dm-multipath) 
> and then you can compare them and see the result --- and then it can be 
> consisdered if the high human time consumed by patching dm.c is worth the 
> performance improvement.

Mikulas,

Section 3.1 of the the following 2007 Linux Symposium paper answers the
"why?" on request-based dm-multipath:
http://ols.108.redhat.com/2007/Reprints/ueda-Reprint.pdf

In summary:
With request-based multipath performance and path error handling is
improved.  

Performance:
The I/O scheduler is leveraged to merge bios into requests; and these
requests are then able to be more evenly balanced across the available
paths (no need to starve other paths like the bio-based multipath is
prone to do).

Error handling:
Finer grained error statistics are available when interfacing more
directly with the hardware like the request-based multipath does.

NEC may already have comparative performance data that will help
illustrate the improvement associated with request-based multipath?
They apparently have dynamic load balancing patches that they developed
for use with the current bio-based multipath.

It'd be interesting to understand the performance difference between
that bio-based implementation and the new request-based implementations
(both service-time and queue-length) of dynamic load balancing.

Mike

Re: [k-ueda@ct.jp.nec.com: Re: request-based dm-multipath]

[Mikulas Patocka]

On Wed, 15 Apr 2009, Mike Snitzer wrote:

> On Wed, Apr 15 2009 at  3:09pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > On Fri, 10 Apr 2009, Mike Snitzer wrote:
> > 
> > > Hi Mikulaus,
> > > 
> > > Figured I'd give you this heads up on the request-based multipath
> > > patches too considering your recent "bottom-layer barrier support"
> > > patchset (where you said multipath support is coming later).
> > > 
> > > We likely want to coordinate with the NEC guys so as to make sure things
> > > are in order for the request-based patches to get merged along with your
> > > remaining barrier work for 2.6.31.
> > > 
> > > Mike
> > > 
> > > p.s. below you can see I mistakenly said to Kiyoshi that the recent
> > > barrier patches that got merged upstream were "the last of the DM
> > > barrier support"...
> > 
> > Hi
> > 
> > I would say one thing about the request-based patches --- don't do this.
> > 
> > Your patch adds an alternate I/O path to request processing on device 
> > mapper.
> > 
> > So, with your patch, there will be two I/O request paths. It means that 
> > any work on generic device-mapper code that will have to be done in the 
> > future (such as for example barriers that I did) will be twice harder. It 
> > will take twice the time to understand request processing, twice brain 
> > capacity to remember it, twice the time for coding, twice the time for 
> > code review, twice the time for testing.
> > 
> > If the patch goes in, it will make a lot of things twice harder. And once 
> > the patch is in productive kernels, there'd be very little possibility to 
> > pull it out.
> > 
> > What is the exact reason for your patch? I suppose that it's some 
> > performance degradation caused by the fact that dm-multipath doesn't 
> > distributes requests optimally across both paths. dm-multipath has 
> > pluggable path selectors, so you could improve dm-round-robin.c (or write 
> > alternate path selector module) and you don't have to touch generic dm 
> > code to solve this problem.
> > 
> > The point is that improving dm-multipath target with better path selector 
> > is much less intrusive than patching device mapper core. If you improve 
> > dm-multipath target, only people hacking on dm-multipath will have to 
> > learn about your code. If you modify generic dm.c file, anyone doing 
> > anything on device mapper must learn about your code --- so human time 
> > consumed in much worse in this case.
> > 
> > So, try the alternate solution (write new path selector for dm-multipath) 
> > and then you can compare them and see the result --- and then it can be 
> > consisdered if the high human time consumed by patching dm.c is worth the 
> > performance improvement.
> 
> Mikulas,
> 
> Section 3.1 of the the following 2007 Linux Symposium paper answers the
> "why?" on request-based dm-multipath:
> http://ols.108.redhat.com/2007/Reprints/ueda-Reprint.pdf
>
> In summary:
> With request-based multipath performance and path error handling is
> improved.  
> 
> Performance:
> The I/O scheduler is leveraged to merge bios into requests; and these
> requests are then able to be more evenly balanced across the available
> paths (no need to starve other paths like the bio-based multipath is
> prone to do).

So you can improve the bio-based selector. You can count number&size of 
outstanding requests on each path and select the less loaded path.

You can remember several end positions of last requests and when new 
request matches one of them, send it to the appropriate path, assuming 
that the lower device's scheduler will marge that. Or --- another solution 
is to access queues of the underlying devices and ask them if there's 
anything to merge --- and then send the request down the path that has 
some adjacent request.

I know that the round-robin selector is silly, but you just haven't even 
try to improve it.

If there is non-intrusive solution (improve path selector), it should be 
tried first, before making an intrusive solution (alternate request path 
in dm core).

> Error handling:
> Finer grained error statistics are available when interfacing more
> directly with the hardware like the request-based multipath does.

You can signal it via flags in bios. No need to rewrite dm core.

> NEC may already have comparative performance data that will help
> illustrate the improvement associated with request-based multipath?
> They apparently have dynamic load balancing patches that they developed
> for use with the current bio-based multipath.

So where is it better and why? Does it save CPU time or disk throughtput? 
How? On which workload?

Did they really try to implement some smart path ballancing that takes 
into account merging?

> It'd be interesting to understand the performance difference between
> that bio-based implementation and the new request-based implementations
> (both service-time and queue-length) of dynamic load balancing.
> 
> Mike

There is downside and benchmarks completely ignore it. It makes 
programming harder.

For example, suppose that you committed rq-based multipath before 
barriers. It would make barrier writing much harder and the result is that 
people will get barriers later and meanwhile won't be able to turn on 
write cache. So rq-based multipath patch improves performance for 
multipath devices (you can measure it) and DEGRADES performance for normal 
disks (no barriers, no hardware cache). And you see how many users use 
ATA+SCSI disks and how many use multipath, you may consider that 
performance of multipath is really not that important.

Besides barriers, I'm also developing new snapshot implementation, if 
programming of barriers were harder, I'd be lagged on development of 
snapshots --- so rq-based multipath would degrade performance of snapshots 
(although it doesn't touch snapshot code at all --- the performance 
degradation comes from comsuming excessive human time that could be used 
for other, better things).

And these are things that benchmarks won't show you. Benchmarks just show 
you improvement, but they can't show you the downside. But if you don't 
consider them, you cause damage to Linux.

Mikulas

Re: Re: [k-ueda@ct.jp.nec.com: Re: request-based dm-multipath]

[Hannes Reinecke]

Mikulas Patocka wrote:
> On Wed, 15 Apr 2009, Mike Snitzer wrote:
> 
>> On Wed, Apr 15 2009 at  3:09pm -0400,
>> Mikulas Patocka <mpatocka@redhat.com> wrote:
>>
>>> On Fri, 10 Apr 2009, Mike Snitzer wrote:
>>>
>>>> Hi Mikulaus,
>>>>
>>>> Figured I'd give you this heads up on the request-based multipath
>>>> patches too considering your recent "bottom-layer barrier support"
>>>> patchset (where you said multipath support is coming later).
>>>>
>>>> We likely want to coordinate with the NEC guys so as to make sure things
>>>> are in order for the request-based patches to get merged along with your
>>>> remaining barrier work for 2.6.31.
>>>>
>>>> Mike
>>>>
>>>> p.s. below you can see I mistakenly said to Kiyoshi that the recent
>>>> barrier patches that got merged upstream were "the last of the DM
>>>> barrier support"...
>>> Hi
>>>
>>> I would say one thing about the request-based patches --- don't do this.
>>>
>>> Your patch adds an alternate I/O path to request processing on device 
>>> mapper.
>>>
>>> So, with your patch, there will be two I/O request paths. It means that 
>>> any work on generic device-mapper code that will have to be done in the 
>>> future (such as for example barriers that I did) will be twice harder. It 
>>> will take twice the time to understand request processing, twice brain 
>>> capacity to remember it, twice the time for coding, twice the time for 
>>> code review, twice the time for testing.
>>>
>>> If the patch goes in, it will make a lot of things twice harder. And once 
>>> the patch is in productive kernels, there'd be very little possibility to 
>>> pull it out.
>>>
>>> What is the exact reason for your patch? I suppose that it's some 
>>> performance degradation caused by the fact that dm-multipath doesn't 
>>> distributes requests optimally across both paths. dm-multipath has 
>>> pluggable path selectors, so you could improve dm-round-robin.c (or write 
>>> alternate path selector module) and you don't have to touch generic dm 
>>> code to solve this problem.
>>>
>>> The point is that improving dm-multipath target with better path selector 
>>> is much less intrusive than patching device mapper core. If you improve 
>>> dm-multipath target, only people hacking on dm-multipath will have to 
>>> learn about your code. If you modify generic dm.c file, anyone doing 
>>> anything on device mapper must learn about your code --- so human time 
>>> consumed in much worse in this case.
>>>
>>> So, try the alternate solution (write new path selector for dm-multipath) 
>>> and then you can compare them and see the result --- and then it can be 
>>> consisdered if the high human time consumed by patching dm.c is worth the 
>>> performance improvement.
>> Mikulas,
>>
>> Section 3.1 of the the following 2007 Linux Symposium paper answers the
>> "why?" on request-based dm-multipath:
>> http://ols.108.redhat.com/2007/Reprints/ueda-Reprint.pdf
>>
>> In summary:
>> With request-based multipath performance and path error handling is
>> improved.  
>>
>> Performance:
>> The I/O scheduler is leveraged to merge bios into requests; and these
>> requests are then able to be more evenly balanced across the available
>> paths (no need to starve other paths like the bio-based multipath is
>> prone to do).
> 
> So you can improve the bio-based selector. You can count number&size of 
> outstanding requests on each path and select the less loaded path.
> 
And which is what you _cannot_ do.
You have _no_ idea at all how the bios are merged in to requests.
And as you do scheduling decisions based on the _bios_ you will interfere
with the elevator. Hence you always have to select large scheduling
intervals as to have the disturbance of the elevator as small as possible.

Just decrease 'min_rr_io' setting and watch the performace to drop.

> You can remember several end positions of last requests and when new 
> request matches one of them, send it to the appropriate path, assuming 
> that the lower device's scheduler will marge that. Or --- another solution 
> is to access queues of the underlying devices and ask them if there's 
> anything to merge --- and then send the request down the path that has 
> some adjacent request.
> 
But this would duplicate the elevator merging logic, wouldn't it?
You would have to out-guess the elevator which requests it would merge
next ...

> I know that the round-robin selector is silly, but you just haven't even 
> try to improve it.
> 
> If there is non-intrusive solution (improve path selector), it should be 
> tried first, before making an intrusive solution (alternate request path 
> in dm core).
> 
>> Error handling:
>> Finer grained error statistics are available when interfacing more
>> directly with the hardware like the request-based multipath does.
> 
> You can signal it via flags in bios. No need to rewrite dm core.
> 
But this makes failover costly, as you have to fail over each
individual bio. Hence you cannot (by design) have a real load
balancing as the cost of failover a single path is prohibitively
high.

Using request-based multipathing OTOH the cost for failover becomes
really small and you can do real load-balancing.
Like setting rr_min_io setting to '1' and you won't suffer any
performance drawback.

>> NEC may already have comparative performance data that will help
>> illustrate the improvement associated with request-based multipath?
>> They apparently have dynamic load balancing patches that they developed
>> for use with the current bio-based multipath.
> 
> So where is it better and why? Does it save CPU time or disk throughtput? 
> How? On which workload?
> 
> Did they really try to implement some smart path ballancing that takes 
> into account merging?
> 
No, they didn't, because of the abovementioned points.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

Re: [k-ueda@ct.jp.nec.com: Re: request-based dm-multipath]

[Jun'ichi Nomura]

Mikulas Patocka wrote:
>>> What is the exact reason for your patch? I suppose that it's some 
>>> performance degradation caused by the fact that dm-multipath doesn't 
>>> distributes requests optimally across both paths. dm-multipath has 
>>> pluggable path selectors, so you could improve dm-round-robin.c (or write 
>>> alternate path selector module) and you don't have to touch generic dm 
>>> code to solve this problem.
>>>
>>> The point is that improving dm-multipath target with better path selector 
>>> is much less intrusive than patching device mapper core. If you improve 
>>> dm-multipath target, only people hacking on dm-multipath will have to 
>>> learn about your code. If you modify generic dm.c file, anyone doing 
>>> anything on device mapper must learn about your code --- so human time 
>>> consumed in much worse in this case.
>>>
>>> So, try the alternate solution (write new path selector for dm-multipath) 
>>> and then you can compare them and see the result --- and then it can be 
>>> consisdered if the high human time consumed by patching dm.c is worth the 
>>> performance improvement.

The main purpose of request-based dm patches is to provide a framework
to implement optimal path selectors in non-intrusive way.

As you mentioned, it might be possible to implement a good path
selector at bio-level by checking internals of underlying devices' 
request queues and/or I/O schedulers.

However, requiring knowledge/assumptions of internals of other layer
is not a right approach.
Or, splitting the request-based multipath driver out of dm would trash
the existing userspace tools and libraries, so that's not good either.
Thus we chose the design of 'adding request-based mapping feature to dm'.
It doesn't break existing target drivers and userspace tools.
The feature is enabled only for request-based target drivers.
If you want to add a bio-specific new feature, it's still possible.

The design has been discussed in several mailing lists, Ottawa Linux Symposium
and Linux Storage/Filesystem workshop including maintainers
of related subsystems for these few years,
and I think we got basic agreement on this direction.


As for your barrier works, we are looking into the patches
to make them work on request-based dm as well.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation

Re: [k-ueda@ct.jp.nec.com: Re: request-based dm-multipath]

[Mikulas Patocka]

On Thu, 16 Apr 2009, Jun'ichi Nomura wrote:

> The main purpose of request-based dm patches is to provide a framework
> to implement optimal path selectors in non-intrusive way.
> 
> As you mentioned, it might be possible to implement a good path
> selector at bio-level by checking internals of underlying devices' 
> request queues and/or I/O schedulers.
> 
> However, requiring knowledge/assumptions of internals of other layer
> is not a right approach.

There is also a number of functions that any driver can call on queue to 
access the queue state (see blkdev.h).

So if you add one more function (something like 
blk_queue_can_merge_at_this_point(struct request_queue *, sector_t), 
there's nothing wrong about it, it's much less intrusive than adding an 
alternate i/o path.

> Or, splitting the request-based multipath driver out of dm would trash
> the existing userspace tools and libraries, so that's not good either.
> Thus we chose the design of 'adding request-based mapping feature to dm'.
> It doesn't break existing target drivers and userspace tools.
> The feature is enabled only for request-based target drivers.
> If you want to add a bio-specific new feature, it's still possible.

I don't want to pull multipath out of dm. I want it to use the standard 
i/o path in dm.

I am convinced that the path balacing can be solved without using 
requests.

> The design has been discussed in several mailing lists, Ottawa Linux Symposium
> and Linux Storage/Filesystem workshop including maintainers
> of related subsystems for these few years,
> and I think we got basic agreement on this direction.

That is groupthink (http://en.wikipedia.org/wiki/Groupthink). It is not 
valid argument to support your point with opinions of other people. You 
should either support it with *your own* opinion or not supoprt it at all 
:)

> As for your barrier works, we are looking into the patches
> to make them work on request-based dm as well.

So you'll have to write barriers and maybe you start to see why this 
approach is not so good.

Basically, suppose that you have a feature A and it takes "t(A)" time to 
implement it. You have a feature B and it takes "t(B)" time to implement 
it. Now --- if the software is correctly designed, the time to implement 
both features A and B is equal to t(A)+t(B). This is the ideal case how it 
should be. If the time to implement both A and B t(A and B) is grossly 
greater than t(A)+t(B), there is some trouble. If such disruptive features 
were added frequently, you end up with a system where programming time is 
exponentially dependent on the number of features --- i.e. something 
totally unmaintainable.

And this double-i/o path is exactly the same case. t(barriers) is some 
time. t(rq-based mpath) takes another time.
t(barriers & rq-based mpath) > t(barriers) + t(rq-based mpath) (because 
the barriers basically have to be reimplemented from scratch for 
the request-based i/o path). And any new features that come into the i/o 
path after barriers will also be twice harder.

See for example the thread with bugs here: 
https://www.redhat.com/archives/dm-devel/2009-March/thread.html#00026 It 
shows that that debugging-time-doubling that I talked about is already 
hapenning, you are fixing bugs that were already fixed in generic dm core 
long time ago. And I don't believe that those bugs are the last. If this 
patch goes in, we all will just double our maintenance time.


So I'm saying that if there's an alternate solution for multipath 
request-dispatching that doesn't have this maintenance-time-increasing 
problem, it should be tried.

Mikulas

> Thanks,
> -- 
> Jun'ichi Nomura, NEC Corporation
> 

Re: [k-ueda@ct.jp.nec.com: Re: request-based dm-multipath]

[Jun'ichi Nomura]

Mikulas Patocka wrote:
> On Thu, 16 Apr 2009, Jun'ichi Nomura wrote:
>> The main purpose of request-based dm patches is to provide a framework
>> to implement optimal path selectors in non-intrusive way.
>>
>> As you mentioned, it might be possible to implement a good path
>> selector at bio-level by checking internals of underlying devices' 
>> request queues and/or I/O schedulers.
>>
>> However, requiring knowledge/assumptions of internals of other layer
>> is not a right approach.
> 
> There is also a number of functions that any driver can call on queue to 
> access the queue state (see blkdev.h).
> 
> So if you add one more function (something like 
> blk_queue_can_merge_at_this_point(struct request_queue *, sector_t), 
> there's nothing wrong about it, it's much less intrusive than adding an 
> alternate i/o path.

And it means exposing the I/O scheduler internal to let
the path selector guess its behavior, which is what I would like to avoid.

I think existing block layer interfaces are not doing that.


>> Or, splitting the request-based multipath driver out of dm would trash
>> the existing userspace tools and libraries, so that's not good either.
>> Thus we chose the design of 'adding request-based mapping feature to dm'.
>> It doesn't break existing target drivers and userspace tools.
>> The feature is enabled only for request-based target drivers.
>> If you want to add a bio-specific new feature, it's still possible.
> 
> I don't want to pull multipath out of dm. I want it to use the standard 
> i/o path in dm.
> 
> I am convinced that the path balacing can be solved without using 
> requests.

The point is not about 'can' or 'cannot'.

My point is that by adding the request-based mapping interface,
path selectors can be implemented in a clean and maintainable way.


Thanks,
-- 
Jun'ichi Nomura, NEC Corporation