Re[2]: [PATCH 03/11][v3] async_tx: add support for asynchronous RAID6 recovery operations

[Yuri Tikhonov]

On Thursday, January 15, 2009 Dan Williams wrote:

 On Mon, Jan 12, 2009 at 5:43 PM, Yuri Tikhonov y...@emcraft.com wrote:
 +   /* (2) Calculate Q+Qxy */
 +   lptrs[0] = ptrs[failb];
 +   lptrs[1] = ptrs[disks-1];
 +   lptrs[2] = NULL;
 +   tx = async_pq(lptrs, NULL, 0, 1, bytes, ASYNC_TX_DEP_ACK,
 + tx, NULL, NULL);
 +
 +   /* (3) Calculate P+Pxy */
 +   lptrs[0] = ptrs[faila];
 +   lptrs[1] = ptrs[disks-2];
 +   lptrs[2] = NULL;
 +   tx = async_pq(lptrs, NULL, 0, 1, bytes, ASYNC_TX_DEP_ACK,
 + tx, NULL, NULL);
 +

 These two calls convinced me that ASYNC_TX_PQ_ZERO_{P,Q} need to go.
 A 1-source async_pq operation does not make sense.

 Another source is hidden under not-set ASYNC_TX_PQ_ZERO_{P,Q} :) 
Though, I agree, this looks rather misleading.

   These should be:

/* (2) Calculate Q+Qxy */
lptrs[0] = ptrs[disks-1];
lptrs[1] = ptrs[failb];
tx = async_xor(lptrs[0], lptrs, 0, 2, bytes,
   ASYNC_TX_XOR_DROP_DST|ASYNC_TX_DEP_ACK, tx, NULL, NULL);

 /* (3) Calculate P+Pxy */
lptrs[0] = ptrs[disks-2];
lptrs[1] = ptrs[faila];
tx = async_xor(lptrs[0], lptrs, 0, 2, bytes,
   ASYNC_TX_XOR_DROP_DST|ASYNC_TX_DEP_ACK, tx, NULL, NULL);


 The reason why I preferred to use async_pq() instead of async_xor() 
here is to maximize the chance that the whole D+D recovery operation 
will be handled in one ADMA device, i.e. without channels switch and 
the latency introduced because of that.

 So, if we'll decide to stay with ASYNC_TX_PQ_ZERO_{P,Q}, then this 
should be probably kept unchanged, but if we'll get rid of 
ASYNC_TX_PQ_ZERO_{P,Q}, then, obviously, we'll have to use 
async_xor()s here as you suggest.


 Regards, Yuri

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

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: Re[2]: [PATCH 03/11][v3] async_tx: add support for asynchronous RAID6 recovery operations

[Dan Williams]

On Fri, Jan 16, 2009 at 4:51 AM, Yuri Tikhonov y...@emcraft.com wrote:
  The reason why I preferred to use async_pq() instead of async_xor()
 here is to maximize the chance that the whole D+D recovery operation
 will be handled in one ADMA device, i.e. without channels switch and
 the latency introduced because of that.


This should be a function of the async_tx_find_channel implementation.
 The default version tries to keep a chain of operations on one
channel.

struct dma_chan *
__async_tx_find_channel(struct dma_async_tx_descriptor *depend_tx,
enum dma_transaction_type tx_type)
{
/* see if we can keep the chain on one channel */
if (depend_tx 
dma_has_cap(tx_type, depend_tx-chan-device-cap_mask))
return depend_tx-chan;
return dma_find_channel(tx_type);
}

--
Dan
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev