Re[2]: [PATCH 02/11] async_tx: add support for asynchronous GF multiplication

[Yuri Tikhonov <yur@emcraft.com>]

=0D=0A Hello Dan,

On Saturday, November 15, 2008 you wrote:

> A few comments

 Thanks.

> 1/ I don't see code for handling cases where the src_cnt exceeds the
> hardware maximum.

 Right, actually the ADMA devices we used (ppc440spe DMA engines) has=20
no limitations on the src_cnt (well, actually there is the limit - the=20
size of descriptors FIFO, but it's more than the number of drives=20
which may be handled with the current RAID-6 driver, i.e. > 256), but=20
I agree - the ASYNC_TX functions should not assume that any ADMA=20
device will have such a feature. So we'll implement this, and then=20
re-post the patches.

> 2/ dmaengine.h defines DMA_PQ_XOR but these patches should really
> change that to DMA_PQ and do s/pqxor/pq/ across the rest of the code
> base.

 OK.

> 3/ In my implementation (unfinished) of async_pq I decided to make the
> prototype:

 May I ask do you have in plans to finish and release your=20
implementation?


> +/**
> + * async_pq - attempt to generate p (xor) and q (Reed-Solomon code) with=
 a
> + *     dma engine for a given set of blocks.  This routine assumes a fie=
ld of
> + *     GF(2^8) with a primitive polynomial of 0x11d and a generator of {=
02}.
> + *     In the synchronous case the p and q blocks are used as temporary
> + *     storage whereas dma engines have their own internal buffers.  The
> + *     ASYNC_TX_PQ_ZERO_P and ASYNC_TX_PQ_ZERO_Q flags clear the
> + *     destination(s) before they are used.
> + * @blocks: source block array ordered from 0..src_cnt with the p destin=
ation
> + *     at blocks[src_cnt] and q at blocks[src_cnt + 1]
> + *     NOTE: client code must assume the contents of this array are dest=
royed
> + * @offset: offset in pages to start transaction
> + * @src_cnt: number of source pages: 2 < src_cnt <=3D 255
> + * @len: length in bytes
> + * @flags: ASYNC_TX_ACK, ASYNC_TX_DEP_ACK
> + * @depend_tx: p+q operation depends on the result of this transaction.
> + * @cb_fn: function to call when p+q generation completes
> + * @cb_param: parameter to pass to the callback routine
> + */
> +struct dma_async_tx_descriptor *
> +async_pq(struct page **blocks, unsigned int offset, int src_cnt, size_t =
len,
> +        enum async_tx_flags flags, struct dma_async_tx_descriptor *depen=
d_tx,
> +        dma_async_tx_callback cb_fn, void *cb_param)

> Where p and q are not specified separately.  This matches more closely
> how the current gen_syndrome is specified with the goal of not
> requiring any changes to existing software raid6 interface.
> Thoughts?

 Understood. Our goal was to be more close to the ASYNC_TX interfaces,=20
so we specified the destinations separately. Though I'm fine with your=20
prototype, since doubling the same address is no good, so, we'll=20
change this.=20

 Any comments regarding the drivers/md/raid5.c part ?

 Regards, Yuri

 --
 Yuri Tikhonov, Senior Software Engineer
 Emcraft Systems, www.emcraft.com