On Sun, 10 Aug 2014 17:49:58 +0000 Markus Stockhausen
[off-list ref] wrote:

quoted

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

quoted

On Thu, 31 Jul 2014 06:43:52 +0000 Markus Stockhausen
[off-list ref] 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