[patch] 2.5.48-bk, md raid0 fix

[patch] 2.5.48-bk, md raid0 fix

[Thorbjørn Lind]

Fixes the 'BUG at drivers/block/ll_rw_blk.c:19xx' when using raid0 md devices since 2.5.45...

/tul

--- a/drivers/md/raid0.c	2002-11-18 05:29:46.000000000 +0100
+++ b/drivers/md/raid0.c	2002-11-20 01:12:08.000000000 +0100
@@ -173,15 +173,14 @@
  static int raid0_mergeable_bvec(request_queue_t *q, struct bio *bio, struct bio_vec *biovec)
  {
  	mddev_t *mddev = q->queuedata;
-	sector_t block;
-	unsigned int chunk_size;
-	unsigned int bio_sz;
-
-	chunk_size = mddev->chunk_size >> 10;
-	block = bio->bi_sector >> 1;
-	bio_sz = (bio->bi_size + biovec->bv_len) >> 10;
-
-	return (chunk_size - ((block & (chunk_size - 1)) + bio_sz)) << 10;
+	unsigned int max_size;
+
+	max_size = mddev->chunk_size - ((bio->bi_sector % (mddev->chunk_size >> 9)) << 9);
+
+	if(biovec->bv_len <= (max_size - bio->bi_size))
+	    return biovec->bv_len;
+
+	return max_size - bio->bi_size;
  }

  static int raid0_run (mddev_t *mddev)

Re: [patch] 2.5.48-bk, md raid0 fix

[Andrew Morton]

Thorbjørn Lind wrote:
> 
> Fixes the 'BUG at drivers/block/ll_rw_blk.c:19xx' when using raid0 md devices since 2.5.45...
> 
> ...
> +       max_size = mddev->chunk_size - ((bio->bi_sector % (mddev->chunk_size >> 9)) << 9);

The mod operator on a 64-bit quantity won't work with
CONFIG_LBD=y, will it?

Re: [patch] 2.5.48-bk, md raid0 fix

[Thorbjørn Lind]

> The mod operator on a 64-bit quantity won't work with
> CONFIG_LBD=y, will it?

Ohh.. there is such a thing.. let's use & :)

Re: [patch] 2.5.48-bk, md raid0 fix

[Neil Brown]

On Wednesday November 20, mtl@slowbone.net wrote:
> > The mod operator on a 64-bit quantity won't work with
> > CONFIG_LBD=y, will it?
> 
> Ohh.. there is such a thing.. let's use & :)
> 

Here is my version which does the 'right' thing w.r.t. large sector
addresses etc.

I just sent it to Linus for the third time.  Hopefully it wont be
dropped again :-(

NeilBrown
--------------------------------------------------------
Fix *_mergeable_bvec routines for linear/raid0.

They take the length of the passed bvec into account, which is wrong.



 ----------- Diffstat output ------------
 ./drivers/md/linear.c |    7 ++-----
 ./drivers/md/raid0.c  |   16 ++++++++--------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff ./drivers/md/linear.c~current~ ./drivers/md/linear.c
--- ./drivers/md/linear.c~current~	2002-11-21 09:41:47.000000000 +1100
+++ ./drivers/md/linear.c	2002-11-21 09:41:47.000000000 +1100
@@ -58,15 +58,12 @@ static int linear_mergeable_bvec(request
 {
 	mddev_t *mddev = q->queuedata;
 	dev_info_t *dev0;
-	int maxsectors, bio_sectors = (bio->bi_size + biovec->bv_len) >> 9;
+	unsigned long maxsectors, bio_sectors = bio->bi_size >> 9;
 
 	dev0 = which_dev(mddev, bio->bi_sector);
 	maxsectors = (dev0->size << 1) - (bio->bi_sector - (dev0->offset<<1));
 
-	if (bio_sectors <= maxsectors)
-		return biovec->bv_len;
-
-	return (maxsectors << 9) - bio->bi_size;
+	return (maxsectors - bio_sectors) << 9;
 }
 
 static int linear_run (mddev_t *mddev)

diff ./drivers/md/raid0.c~current~ ./drivers/md/raid0.c
--- ./drivers/md/raid0.c~current~	2002-11-21 09:41:47.000000000 +1100
+++ ./drivers/md/raid0.c	2002-11-21 09:41:47.000000000 +1100
@@ -173,15 +173,15 @@ static int create_strip_zones (mddev_t *
 static int raid0_mergeable_bvec(request_queue_t *q, struct bio *bio, struct bio_vec *biovec)
 {
 	mddev_t *mddev = q->queuedata;
-	sector_t block;
-	unsigned int chunk_size;
-	unsigned int bio_sz;
+	sector_t sector;
+	unsigned int chunk_sectors;
+	unsigned int bio_sectors;
+
+	chunk_sectors = mddev->chunk_size >> 9;
+	sector = bio->bi_sector;
+	bio_sectors = bio->bi_size >> 9;
 
-	chunk_size = mddev->chunk_size >> 10;
-	block = bio->bi_sector >> 1;
-	bio_sz = (bio->bi_size + biovec->bv_len) >> 10;
-
-	return (chunk_size - ((block & (chunk_size - 1)) + bio_sz)) << 10;
+	return (chunk_sectors - ((sector & (chunk_sectors - 1)) + bio_sectors)) << 9;
 }
 
 static int raid0_run (mddev_t *mddev)

[patch] 2.5.48-bk, md raid0 fix

[Peter Chubb]

>>>>> "Thorbjørn" == Thorbjørn Lind <mtl@slowbone.net> writes:

Thorbjørn> Fixes the 'BUG at drivers/block/ll_rw_blk.c:19xx' when
Thorbjørn> using raid0 md devices since 2.5.45...  /tul

Your patch won't work if CONFIG_LBD is on.  I haven't seen this bug
reported since the raid0_mergeable_bvec() function went in.

Are you not using power-of-two sized chunks?  If not, use the
sector_div() macro to do the remainder, not a direct remainder
operation (which will fail to link if CONFIG_LBD on IA32)


You could try ... (untested)
--- /tmp/geta22599	2002-11-20 15:48:17.000000000 +1100
+++ raid0.c		2002-11-20 15:46:11.000000000 +1100
@@ -163,7 +163,7 @@
 }
 
 /**
- *	raid0_mergeable_bvec -- tell bio layer if a two requests can be merged
+ *	raid0_mergeable_bvec -- tell bio layer if two requests can be merged
  *	@q: request queue
  *	@bio: the buffer head that's been built up so far
  *	@biovec: the request that could be merged to it.
@@ -181,7 +181,7 @@
	block = bio->bi_sector >> 1;
	bio_sz = (bio->bi_size + biovec->bv_len) >> 10;
 
-	return (chunk_size - ((block & (chunk_size - 1)) + bio_sz)) << 10;
+	return (chunk_size - (sector_div(block, chunk_size) + bio_sz))
<< 10;
 }
 

--
Dr Peter Chubb				    peterc@gelato.unsw.edu.au
You are lost in a maze of BitKeeper repositories, all almost the same.

Re: [patch] 2.5.48-bk, md raid0 fix

[Thorbjørn Lind]

> Are you not using power-of-two sized chunks?  If not, use the
Yes

> You could try ... (untested)
> - return (chunk_size - ((block & (chunk_size - 1)) + bio_sz)) << 10;
> + return (chunk_size - (sector_div(block, chunk_size) + bio_sz)) << 10;

No good either.. not many using raid0 I guess :)

... here is what happenes with either of the above.

raid0_mergeable: chunk_size 32768 bi_sector 524288, bi_size 0, bv_len 4096, ret 28672
raid0_mergeable: chunk_size 32768 bi_sector 100401152, bi_size 0, bv_len 4096, ret 28672
raid0_mergeable: chunk_size 32768 bi_sector 96, bi_size 0, bv_len 4096, ret 12288
raid0_mergeable: chunk_size 32768 bi_sector 4216, bi_size 0, bv_len 4096, ret 0
kernel BUG at drivers/block/ll_rw_blk.c:1995!
invalid operand: 0000

Should it really return 0 when we are actually ready to recieve that 4k request? That is.. should it
return how much can be merged or how much it can take after the merge. The first seems to work.
That would be something like:

--- a/drivers/md/raid0.c        2002-11-18 05:29:46.000000000 +0100
+++ b/drivers/md/raid0.c        2002-11-20 07:19:15.000000000 +0100
@@ -173,15 +173,10 @@
 static int raid0_mergeable_bvec(request_queue_t *q, struct bio *bio, struct bio_vec *biovec)
 {
        mddev_t *mddev = q->queuedata;
-       sector_t block;
-       unsigned int chunk_size;
-       unsigned int bio_sz;
+       unsigned int chunk_sects = mddev->chunk_size >> 9;
+       unsigned int max_sectors = chunk_sects - (bio->bi_sector % chunk_sects);

-       chunk_size = mddev->chunk_size >> 10;
-       block = bio->bi_sector >> 1;
-       bio_sz = (bio->bi_size + biovec->bv_len) >> 10;
-
-       return (chunk_size - ((block & (chunk_size - 1)) + bio_sz)) << 10;
+       return (max_sectors - (bio->bi_size >> 9)) << 9;
 }

 static int raid0_run (mddev_t *mddev)