Re: [PATCH][RFC] RAID5/DMA/memcpy: zero copy the bio page whenpossible

From: NeilBrown
Date: Mon Jan 02 2012 - 18:13:49 EST


On Wed, 28 Dec 2011 13:20:08 +0800 <b29237@xxxxxxxxxxxxx> wrote:

> From: Forrest shi <b29237@xxxxxxxxxxxxx>
>
> use bio page directly instead of copying the bio page to stripe
> header page when possible.
>
> Signed-off-by: Forrest Shi<b29237@xxxxxxxxxxxxx>

I'm sorry but if you want anyone to review this and comment on it you need to
give us a bit more help - like telling us what the purpose of the patch is.

There are a couple of hints but what is needed is a 'big picture'
description. What is the problem you are fixing? How are you fixing it?
What are the costs/benefits?

> @@ -754,10 +764,16 @@ ops_run_compute5(struct stripe_head *sh, struct raid5_percpu *percpu)
> __func__, (unsigned long long)sh->sector, target);
> BUG_ON(!test_bit(R5_Wantcompute, &tgt->flags));
>
> - for (i = disks; i--; )
> + for (i = disks; i--; ) {
> + struct r5dev *dev = &sh->dev[i];
> + struct page *pg = dev->page;
> if (i != target)
> - xor_srcs[count++] = sh->dev[i].page;
> -
> +#ifdef CONFIG_OPTIMIZE_FSL_DMA_MEMCPY
> + if (test_bit(R5_DirectAccess, &dev->flags))
> + pg = dev->req.bi_io_vec[0].bv_page;
> +#endif
> + xor_srcs[count++] = pg;
> + }
> atomic_inc(&sh->count);

And something is very wrong here. I think you need { } around the body of
the 'if' statement.

NeilBrown

Attachment: signature.asc
Description: PGP signature