On Tuesday May 23, [EMAIL PROTECTED] wrote:
> Neil hello.
>
> 1.
> i have applied the common path according to
> http://www.spinics.net/lists/raid/msg11838.html as much as i can.
Great. I look forward to seeing the results.
>
> it looks ok in terms of throughput.
> before i continue to a non common path ( step 3 ) i do not understand
> raid0_mergeable_bvec entirely.
Not too surprising - it is rather subtle unfortunately.
>
> as i understand the code checks alignment . i made a version for this
> purpose which looks like that:
Yes, it checks alignment with the chunks and devices.
However we always have to allow one page to be added to a bio, so
sometimes we have to accept a bio that crosses a chunk/device boundary.
The main (possibly only) user is in __bio_add_page in fs/bio.c
so we basically code the merge_bvec_fn to meet the needs of that code.
>
> 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;
>
> max=(chunk_sectors-((sector&(chunk_sectors-1))+bio_sectors))<<9;
> if (max < 0){
> printk("handle_aligned_read not aligned %d %d %d
> %lld\n",max,chunk_sectors,bio_sectors,sector);
> return -1; // Is bigger than one chunk size
> }
>
> // printk("handle_aligned_read aligned %d %d %d
> %lld\n",max,chunk_sectors,bio_sectors,sector);
> return max;
> }
you cannot return a negative number, because the result is
compared with an 'unsigned int', and the comparison will be unsigned.
So "return -1" is a problem. I think you need to make this code
look a lot more like raid0_mergeable_bvec.
>
> Questions:
> 1.1 why did you drop the max=0 case ?
I'm not sure what you mean by 'drop'.
If bio->bi_size == 0, then we are not allowed to return a number
smaller than biovec->bv_len, otherwise bio_add_page wont be able
to put any pages on the bio, and so wont be able to start any IO.
> 1.2 what these lines mean ? do i need it ?
> if (max <= biovec->bv_len && bio_sectors == 0)
> return biovec->bv_len;
> else
> return max;
> }
Yes, you need this. It basically implements the above restriction.
NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
