dm: bio split bvec fix

dm: bio split bvec fix

[Alasdair G Kergon]

The code that handles bios that span table target boundaries by breaking
them up into smaller bios will not split an individual struct bio_vec
into more than two pieces.  Sometimes more than that are required.

This patch adds a loop to break the second piece up into as many
pieces as are necessary.

Cc: "Abhishek Gupta" <abhishekgupt@gmail.com>
Cc: Dan Smith <danms@us.ibm.com>
Signed-Off-By: Alasdair G Kergon <agk@redhat.com>

Index: linux-2.6.16-rc5/drivers/md/dm.c
===================================================================
--- linux-2.6.16-rc5.orig/drivers/md/dm.c	2006-03-20 16:00:11.000000000 +0000
+++ linux-2.6.16-rc5/drivers/md/dm.c	2006-03-20 18:38:19.000000000 +0000
@@ -573,30 +573,35 @@ static void __clone_and_map(struct clone
 
 	} else {
 		/*
-		 * Create two copy bios to deal with io that has
-		 * been split across a target.
+		 * Handle a bvec that must be split between two or more targets.
 		 */
 		struct bio_vec *bv = bio->bi_io_vec + ci->idx;
+		sector_t remaining = to_sector(bv->bv_len);
+		unsigned int offset = 0;
 
-		clone = split_bvec(bio, ci->sector, ci->idx,
-				   bv->bv_offset, max);
-		__map_bio(ti, clone, tio);
-
-		ci->sector += max;
-		ci->sector_count -= max;
-		ti = dm_table_find_target(ci->map, ci->sector);
-
-		len = to_sector(bv->bv_len) - max;
-		clone = split_bvec(bio, ci->sector, ci->idx,
-				   bv->bv_offset + to_bytes(max), len);
-		tio = alloc_tio(ci->md);
-		tio->io = ci->io;
-		tio->ti = ti;
-		memset(&tio->info, 0, sizeof(tio->info));
-		__map_bio(ti, clone, tio);
+		do {
+			if (offset) {
+				ti = dm_table_find_target(ci->map, ci->sector);
+				max = max_io_len(ci->md, ci->sector, ti);
+
+				tio = alloc_tio(ci->md);
+				tio->io = ci->io;
+				tio->ti = ti;
+				memset(&tio->info, 0, sizeof(tio->info));
+			}
+
+			len = (max > remaining) ? remaining : max;
+
+			clone = split_bvec(bio, ci->sector, ci->idx,
+					   bv->bv_offset + offset, len);
+
+			__map_bio(ti, clone, tio);
+
+			ci->sector += len;
+			ci->sector_count -= len;
+			offset += to_bytes(len);
+		} while (remaining -= len);
 
-		ci->sector += len;
-		ci->sector_count -= len;
 		ci->idx++;
 	}
 }

Re: dm: bio split bvec fix

[Andrew Morton]

Alasdair G Kergon <agk@redhat.com> wrote:
>
> The code that handles bios that span table target boundaries by breaking
>  them up into smaller bios will not split an individual struct bio_vec
>  into more than two pieces.  Sometimes more than that are required.
> 
>  This patch adds a loop to break the second piece up into as many
>  pieces as are necessary.

Should this go into 2.6.16.1?

Re: dm: bio split bvec fix

[Jens Axboe]

On Mon, Mar 20 2006, Alasdair G Kergon wrote:
> The code that handles bios that span table target boundaries by breaking
> them up into smaller bios will not split an individual struct bio_vec
> into more than two pieces.  Sometimes more than that are required.
> 
> This patch adds a loop to break the second piece up into as many
> pieces as are necessary.

Why isn't this just handled in the merge callback? Can a single page bio
span > 2 targets?

-- 
Jens Axboe

Re: [dm-devel] Re: dm: bio split bvec fix

[Alasdair G Kergon]

On Wed, Mar 22, 2006 at 12:32:35PM +0100, Jens Axboe wrote:
> Why isn't this just handled in the merge callback? Can a single page bio
> span > 2 targets?
 
Yes.  (Unit of size if the sector - and things don't have to
be aligned nicely, just aligned to sector.)

IIRC the merge function assumes the number of bytes that can
be added is only a function of the offset: but in our case
it's also a function of time.  To make this work it should
reserve those bytes with device-mapper, and guarantee either to 
supply them to us subsequently (and preferably quickly) or to 
cancel that reservation.  Device-mapper for its part would
guarantee to accept the bio without needing to split it.
Or dm could have a rejection mechanism that refuses bios
that are too big (because the max number of bytes we accept
got reduced between the initial call and the bio actually being 
presented) and they go back and get processed again.

Alasdair
-- 
agk@redhat.com

Re: [dm-devel] Re: dm: bio split bvec fix

[Jens Axboe]

On Wed, Mar 22 2006, Alasdair G Kergon wrote:
> On Wed, Mar 22, 2006 at 12:32:35PM +0100, Jens Axboe wrote:
> > Why isn't this just handled in the merge callback? Can a single page bio
> > span > 2 targets?
>  
> Yes.  (Unit of size if the sector - and things don't have to
> be aligned nicely, just aligned to sector.)
> 
> IIRC the merge function assumes the number of bytes that can
> be added is only a function of the offset: but in our case
> it's also a function of time.  To make this work it should
> reserve those bytes with device-mapper, and guarantee either to 
> supply them to us subsequently (and preferably quickly) or to 
> cancel that reservation.  Device-mapper for its part would
> guarantee to accept the bio without needing to split it.
> Or dm could have a rejection mechanism that refuses bios
> that are too big (because the max number of bytes we accept
> got reduced between the initial call and the bio actually being 
> presented) and they go back and get processed again.

Ok, thanks for the followup explanation, I guess there's no way around
that then.

-- 
Jens Axboe

Re: [dm-devel] Re: dm: bio split bvec fix

[Jens Axboe]

On Wed, Mar 22 2006, Alasdair G Kergon wrote:
> On Wed, Mar 22, 2006 at 12:32:35PM +0100, Jens Axboe wrote:
> > Why isn't this just handled in the merge callback? Can a single page bio
> > span > 2 targets?
>  
> Yes.  (Unit of size if the sector - and things don't have to
> be aligned nicely, just aligned to sector.)
> 
> IIRC the merge function assumes the number of bytes that can
> be added is only a function of the offset: but in our case
> it's also a function of time.  To make this work it should
> reserve those bytes with device-mapper, and guarantee either to 
> supply them to us subsequently (and preferably quickly) or to 
> cancel that reservation.  Device-mapper for its part would
> guarantee to accept the bio without needing to split it.
> Or dm could have a rejection mechanism that refuses bios
> that are too big (because the max number of bytes we accept
> got reduced between the initial call and the bio actually being 
> presented) and they go back and get processed again.
> 
> Alasdair
> -- 
> agk@redhat.com
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Jens Axboe