On Mon, May 18, 2009 at 06:00:07PM -0700, Dan Williams wrote:
> +/* the struct page *blocks[] parameter passed to async_gen_syndrome()
> + * and async_syndrome_val() contains the 'P' destination address at
> + * blocks[disks-2] and the 'Q' destination address at blocks[disks-1]
> + *
> + * note: these are macros as they are used a lvalues
> + */
> +#define P(b, d) (b[d-2])
> +#define Q(b, d) (b[d-1])
s/a lvalues/as lvalues
> + /* convert source addresses being careful to collapse 'empty'
> + * sources and update the coefficients accordingly
> + */
> + for (i = 0, idx = 0; i < src_cnt; i++) {
> + if (is_raid6_zero_block(blocks[i]))
> + continue;
> + dma_src[idx] = dma_map_page(dma->dev, blocks[i], offset, len,
> + DMA_TO_DEVICE);
> + coefs[idx] = scfs[i];
> + idx++;
> + }
> + src_cnt = idx;
If P is disabled, we could further collapse this loop by also skipping
sources where the coefficient is zero. Not sure if this would be a win
though.
> +
> + while (src_cnt > 0) {
> + submit->flags = flags_orig;
> + pq_src_cnt = min(src_cnt, dma_maxpq(dma, dma_flags));
> + /* if we are submitting additional pqs, leave the chain open,
> + * clear the callback parameters, and leave the destination
> + * buffers mapped
> + */
> + if (src_cnt > pq_src_cnt) {
> + submit->flags &= ~ASYNC_TX_ACK;
> + dma_flags |= DMA_COMPL_SKIP_DEST_UNMAP;
> + submit->cb_fn = NULL;
> + submit->cb_param = NULL;
> + } else {
> + dma_flags &= ~DMA_COMPL_SKIP_DEST_UNMAP;
> + submit->cb_fn = cb_fn_orig;
> + submit->cb_param = cb_param_orig;
> + }
> + if (submit->cb_fn)
> + dma_flags |= DMA_PREP_INTERRUPT;
The last if() can go to the else branch.
> +/**
> + * do_sync_gen_syndrome - synchronously calculate a raid6 syndrome
> + */
> +static void
> +do_sync_gen_syndrome(struct page **blocks, unsigned int offset, int disks,
> + size_t len, struct async_submit_ctl *submit)
> +{
> + void **srcs;
> + int i;
> +
> + if (submit->scribble)
> + srcs = (void **) submit->scribble;
Unnecessary cast.
> + else
> + srcs = (void **) blocks;
> +
> + for (i = 0; i < disks; i++) {
> + if (is_raid6_zero_block(blocks[i])) {
> + BUG_ON(i > disks - 3); /* P or Q can't be zero */
> + srcs[i] = (void *) blocks[i];
Another Unnecessary cast.
Otherwise, this patch looks also very nice.
Thanks
Andre
--
The only person who always got his work done by Friday was Robinson Crusoe
Attachment:
signature.asc
Description: Digital signature
