Re: [PATCH 07/13] async_tx: add support for asynchronous RAID6 recovery operations

[Andre Noll <maan@systemlinux.org>]

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

On 12:20, Dan Williams wrote:
> +struct dma_async_tx_descriptor *
> +async_r6_dd_recov(int disks, size_t bytes, int faila, int failb,
> +		  struct page **ptrs, enum async_tx_flags flags,
> +		  struct dma_async_tx_descriptor *depend_tx,
> +		  dma_async_tx_callback cb, void *cb_param)
> +{
> +	struct dma_async_tx_descriptor *tx = NULL;
> +	struct page *lptrs[disks];
> +	unsigned char lcoef[disks-4];

This probably needs a BUG_ON(disks < 4).

> +	 * B = (2^(y-x))*((2^(y-x) + {01})^(-1))

Minor optimization suggestion: As B depends only on y-x, there are
255 possible values for B, so a lookup table for all these values
would only occupy 255 bytes.

> +ddr_sync:
> +	{
> +		void **sptrs = (void **)lptrs;

unnecessary cast

> +struct dma_async_tx_descriptor *
> +async_r6_dp_recov(int disks, size_t bytes, int faila, struct page **ptrs,
> +		  enum async_tx_flags flags,
> +		  struct dma_async_tx_descriptor *depend_tx,
> +		  dma_async_tx_callback cb, void *cb_param)
> +{
> +	struct dma_async_tx_descriptor *tx = NULL;

unnecessary initialization.

> +	struct page *lptrs[disks];
> +	unsigned char lcoef[disks-2];
> +	int i = 0, k = 0;

again. I'd suggest to init i and k in the for() loop.

Regards
Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]