Re: [RFC] lib: raid: New RAID library supporting up to six parities

Re: [RFC] lib: raid: New RAID library supporting up to six parities

[NeilBrown]

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

On Mon, 6 Jan 2014 10:45:23 +0100 Andrea Mazzoleni <amadvance@gmail.com>
wrote:

> Hi Neil,
> 
> Thanks for your feedback. In the meantime I went further in developing and
> I've just sent version 2 of the patch, that contains a preliminary btrfs
> modification to use the new interface.
> 
> Please use this one for any kind of review because it contains a modification
> of the interface to match better the btrfs use.
> I'll now try to do something similar to the async_tx layer and to improve the
> code documentation as you recommended.

Thanks.

> 
> Anyway, a good entry point to understand the code is to start from the
> include/linux/raid/raid.h file. It contains the functions that external
> modules should call with a complete description of them.
> 
> There is raid_par() used to compute parity, and raid_rec() to recover damaged
> blocks. These two functions replace all the old xor_blocks and raid6 calls.
> 
> And there is the raid_sort() you mention. It's an helper function that can be
> used to ensure that the blocks indexes are passed at the raid interface in
> proper order. In existing code I saw that the indexes are often sorted before
> calling raid6, with something like:
> 
> 	if (faila > failb) {
> 		int tmp = failb;
> 		failb = faila;
> 		faila = tmp;
> 	}
> 
> To do the same with up to six failures, it's now required some kind of sort
> function.

I'm not totally convinced by this, but then I haven't played with the code
so maybe I'm wrong.
I don't see the above as "sorting" faila and failb, but rather determining
which one is first.  Once you know which one is first, the remainder follow
in order.
So I would probably just make sure we always process the block is the "right"
order.  Then sorting would be irrelevant.
But as I say, I haven't fiddled with the code, so maybe that would end up
being more complex.

Thanks,
NeilBrown


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

Re: [RFC] lib: raid: New RAID library supporting up to six parities

[Andrea Mazzoleni]

Hi Neil,

On 01/07, NeilBrown wrote:
> > To do the same with up to six failures, it's now required some kind of sort
> > function.
> 
> So I would probably just make sure we always process the block is the "right"
> order.  Then sorting would be irrelevant.
> But as I say, I haven't fiddled with the code, so maybe that would end up
> being more complex.

If the the async_tx and btrfs layers can always provide indexes in order,
for sure this sort function can be removed. I agree with you that it would be
a better design.

In fact, the raid library never uses it directly. It's just provided to help the
callers that for whatever reason cannot provide such indexes in order.
And seeing these swap operations between faila/failb in both btrfs and async_tx,
made me to assume that the order is not always correct.

> I don't see the above as "sorting" faila and failb, but rather determining
> which one is first.  Once you know which one is first, the remainder follow
> in order.

The new raid library, like the existing raid6, requires to have the indexes of
the failed blocks in order. With only two indexes faila/failb this means
faila < failb and sorting is equivalent to find the first one.

But with up to six failures, is like to have six fail variables:
faila/failb/failc/faild/faile/failf and the raid library requires them
to be in order as: faila < failb < failc < faild < faile < failf.
In this general case a sort function is the only one that gives the guarantee
to fulfill this requirement whatever is the original order.

Ciao,
Andrea