From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ocean.emcraft.com (ocean.emcraft.com [213.221.7.182]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 1B759DDDFF for ; Thu, 27 Nov 2008 12:26:31 +1100 (EST) Date: Thu, 27 Nov 2008 04:26:29 +0300 From: Yuri Tikhonov Message-ID: <1115054880.20081127042629@emcraft.com> To: "Dan Williams" Subject: Re[2]: [PATCH 02/11] async_tx: add support for asynchronous GF multiplication In-Reply-To: References: <1226589364-5619-1-git-send-email-yanok@emcraft.com> <1226589364-5619-3-git-send-email-yanok@emcraft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Cc: linux-raid@vger.kernel.org, linuxppc-dev@ozlabs.org, dzu@denx.de, wd@denx.de, Ilya Yanok List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , =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