RE: [PATCH v2 09/11] async_tx: add support for asynchronous RAID6recovery operations

[Sosnowski, Maciej]

Dan Williams wrote:
>  async_raid6_2data_recov() recovers two data disk failures
> 
>  async_raid6_datap_recov() recovers a data disk and the P disk
> 
> These routines are a port of the synchronous versions found in
> drivers/md/raid6recov.c.  The primary difference is breaking out the xor
> operations into separate calls to async_xor.  Two helper routines are
> introduced to perform scalar multiplication where needed.
> async_sum_product() multiplies two sources by scalar coefficients and
> then sums (xor) the result.  async_mult() simply multiplies a single
> source by a scalar.
> 
> [ Impact: asynchronous raid6 recovery routines for 2data and datap cases ]
> 
> Cc: Yuri Tikhonov <yur@xxxxxxxxxxx>
> Cc: Ilya Yanok <yanok@xxxxxxxxxxx>
> Cc: H. Peter Anvin <hpa@xxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
>  crypto/async_tx/Kconfig             |    5 +
>  crypto/async_tx/Makefile            |    1
>  crypto/async_tx/async_raid6_recov.c |  292 +++++++++++++++++++++++++++++++++++
>  include/linux/async_tx.h            |    8 +
>  4 files changed, 306 insertions(+), 0 deletions(-)
>  create mode 100644 crypto/async_tx/async_raid6_recov.c

Acked-by: Maciej Sosnowski <maciej.sosnowski@xxxxxxxxx>
 
With a minor comment:

> +static struct dma_async_tx_descriptor *
> +async_mult(struct page *dest, struct page *src, u8 coef, size_t len,
> +          struct async_submit_ctl *submit)
> +{
> +       struct dma_chan *chan = async_tx_find_channel(submit, DMA_PQ,
> +                                                     &dest, 1, srcs, 2, len);
> +       struct dma_device *dma = chan ? chan->device : NULL;
> +       const u8 *qmul; /* Q multiplier table */
> +       u8 *d, *s;
> +
> +       if (dma) {
> +               dma_addr_t dma_dest[2];
> +               dma_addr_t dma_src[1];
> +               struct device *dev = dma->dev;
> +               struct dma_async_tx_descriptor *tx;
> +               enum dma_ctrl_flags dma_flags = DMA_PREP_PQ_DISABLE_P;
> +
> +               dma_dest[1] = dma_map_page(dev, dest, 0, len,
> DMA_BIDIRECTIONAL);
> +               dma_src[0] = dma_map_page(dev, src, 0, len, DMA_TO_DEVICE);
> +               tx = dma->device_prep_dma_pq(chan, dma_dest, dma_src, 1, &coef,
> +                                            len, dma_flags);
> +               if (tx) {
> +                       async_tx_submit(chan, tx, submit);
> +                       return tx;
> +               }
> +       }

How about adding "run the operation synchronously" comment at this point, just like it is in async_sum_product?

Maciej--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/