Re: RAID6 - RMW logic

[NeilBrown <neilb@suse.de>]

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

On Sun, 10 Aug 2014 17:49:58 +0000 Markus Stockhausen
<stockhausen@collogia.de> wrote:

> > Von: NeilBrown [neilb@suse.de]
> > Gesendet: Montag, 4. August 2014 03:22
> > An: Markus Stockhausen
> > Cc: linux-raid@vger.kernel.org
> > Betreff: Re: RAID6 - RMW logic
> > 
> > > On Thu, 31 Jul 2014 06:43:52 +0000 Markus Stockhausen
> > > <stockhausen@collogia.de> wrote:
> > > 
> > > Hi,
> > >
> > > thanks for the link. Crawling through the modifcation I isolated two steps
> > > that we must achieve in first place to get it on track. I'm far away from
> > > implementing a full patch so I focus on what I understand.
> > >
> > > 1) Implement a generic switch so we can configure rmw/rcw handling
> > > on the fly. Without any RAID6 rmw patches yet it will simply focus on the
> > > current RAID5 implementation. Later on RAID6 can use it too and we
> > > are able to  compare rmw versus rcw performance in all cases.
> > > I would name the parameter enable_rmw and default it to 1. In RAID6 case
> > > it will be ignored.
> > >
> > > -> Ok with that?
> > 
> > No, sorry.  Or not very.
> > 
> > In that email thread I pointed you to I wrote:
> > 
> > - Can you  explain *why* rcw is sometimes better than rmw even on large
> >   arrays? Even a fairly hand-wavy arguement would help.  And it would go in
> >   the comment at the top of the patch that adds enable_rmw.
> > 
> > 
> > I see you've posted a patch, but there is no "why".
> > I don't like adding configuration options.  If there is some clear and easy
> > to understand benefit, like "this trades throughput against latency", then I
> > might be able to live with one, because it would be easy to tell people how
> > to tune it.
> > 
> > Why would I ever disable rmw?  Don't say "choose the option that performs best
> > for your workload", because that is nearly meaningless: workloads change from
> > moment to moment.  If rwm is good in some cases and bad in others, then we
> > should at least make sure we understand why, and then hopefully get the md
> > driver  to auto-detect the different cases.
> > 
> > There might be a case for allowing an option like that to support a
> > "developer only preview" of the code.  i.e.  Add the rmw-for-RAID6 code, find
> > that is slows down some workloads, get confused about why, ask for help,
> > people are only happy to  test if it is in mainline, so use a developer-only
> > config option.
> > Then at least I could tell people when to turn it on: only if you are a
> > developer.
> 
> As you might have seen I posted a complete rmw patch to the mailing list.
> It is the first test version with the "developer switch". Sorry for being very
> defensive in that way. Working only one week with the md raid code I
> wanted to ensure that nothing gets broken. Especially I had hard times to
> figure out the logic of the async layer. Therefore I'm very unsure if a system 
> with hardware assisted P/Q calculation will benefit straight forward from 
> my patches. 
> 
> Additionaly I thought about some corner cases that might work better with
> one special switch option. To detect them automatically in md might be beyond
> the scope of this patch.
> 
> Hopefully you can allay my concerns. If you like I can simply drop that switch
> in the next version. 

Thanks for the patches.  I'll try to have a proper look sometime soon, but it
might not be until next week.

NeilBrown

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