Re: [patch 63/87] md: define raid5_mergeable_bvec

Re: [patch 63/87] md: define raid5_mergeable_bvec

[Raz Ben-Jehuda(caro)]

[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]

this is against 2.6.19-git17
hope this correct
raz

On 12/11/06, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Mon, Dec 11 2006, Raz Ben-Jehuda(caro) wrote:
> > On 12/11/06, Jens Axboe <jens.axboe@oracle.com> wrote:
> > >On Sun, Dec 10 2006, akpm@osdl.org wrote:
> > >> From: "Raz Ben-Jehuda(caro)" <raziebe@gmail.com>
> > >>
> > >> This will encourage read request to be on only one device, so we will
> > >often be
> > >> able to bypass the cache for read requests.
> > >>
> > >> Signed-off-by: Neil Brown <neilb@suse.de>
> > >> Cc: Jens Axboe <jens.axboe@oracle.com>
> > >> Signed-off-by: Andrew Morton <akpm@osdl.org>
> > >> ---
> > >>
> > >>  drivers/md/raid5.c |   24 ++++++++++++++++++++++++
> > >>  1 file changed, 24 insertions(+)
> > >>
> > >> diff -puN drivers/md/raid5.c~md-define-raid5_mergeable_bvec
> > >drivers/md/raid5.c
> > >> --- a/drivers/md/raid5.c~md-define-raid5_mergeable_bvec
> > >> +++ a/drivers/md/raid5.c
> > >> @@ -2611,6 +2611,28 @@ static int raid5_congested(void *data, i
> > >>       return 0;
> > >>  }
> > >>
> > >> +/* We want read requests to align with chunks where possible,
> > >> + * but write requests don't need to.
> > >> + */
> > >> +static int raid5_mergeable_bvec(request_queue_t *q, struct bio *bio,
> > >struct bio_vec *biovec)
> > >> +{
> > >> +     mddev_t *mddev = q->queuedata;
> > >> +     sector_t sector = bio->bi_sector + get_start_sect(bio->bi_bdev);
> > >> +     int max;
> > >> +     unsigned int chunk_sectors = mddev->chunk_size >> 9;
> > >> +     unsigned int bio_sectors = bio->bi_size >> 9;
> > >> +
> > >> +     if (bio_data_dir(bio))
> > >> +             return biovec->bv_len; /* always allow writes to be
> > >mergeable */
> > >
> > >Please don't ever do that - you are making assumptions on the value of
> > >READ and WRITE.
> > >
> > >        if (bio_data_dir(bio) == WRITE)
> > >                ...
> > >
> > >If this has already been merged, please submit a patch correcting it.
> > >People end up copying code like this :-)
> > >
> > >--
> > >Jens Axboe
> > >
> > >
> >
> > thanks Jens
> > the attached is a fix.
>
> But the patch is already merged, so the patch needs to be against Linus'
> current tree.
>
> --
> Jens Axboe
>
>


-- 
Raz

[-- Attachment #2: mergeablevecJens.patch --]
[-- Type: text/x-patch, Size: 487 bytes --]

--- linux-2.6.19/drivers/md/raid5.c	2006-12-11 11:33:23.000000000 +0000
+++ linux-2.6.19-git17/drivers/md/raid5.c	2006-12-11 11:31:27.000000000 +0000
@@ -2567,7 +2567,7 @@
 	unsigned int chunk_sectors = mddev->chunk_size >> 9;
 	unsigned int bio_sectors = bio->bi_size >> 9;
 
-	if (bio_data_dir(bio))
+	if (bio_data_dir(bio) == WRITE )
 		return biovec->bv_len; /* always allow writes to be mergeable */
 
 	max =  (chunk_sectors - ((sector & (chunk_sectors - 1)) + bio_sectors)) << 9;

Re: [patch 63/87] md: define raid5_mergeable_bvec

[Jens Axboe]

On Mon, Dec 11 2006, Raz Ben-Jehuda(caro) wrote:
> this is against 2.6.19-git17
> hope this correct

Patch itself looks fine. Some general suggestions for the future:

- Don't top post on lkml
- Inline patches, and always include a description and a Signed-off-by
  line.
- Documentation/SubmittingPatches has a lot of good info.

-- 
Jens Axboe