[PATCH 0/4] use block-layer's clone interface in request-based dm

[PATCH 0/4] use block-layer's clone interface in request-based dm

[Kiyoshi Ueda]

Hi Alasdair,

This patch-set changes request-based dm to use the block-layer's
clone setup interface, blk_rq_prep_clone().
(Before that, some clean-ups for request-based dm are included.)

This patch-set can be applied on top of your linux-next patches
including request-based dm (v3):
    http://marc.info/?l=dm-devel&m=124392590507004&w=2

Summary of the patch-set
------------------------
  1/4: dm core: use BUG_ON in dm_end_request()
  2/4: dm core: clean up completion handling of clone bio
  3/4: dm core: remove 'md' argument from free_rq_tio()
  4/4: dm core: use generic clone setup interface

 drivers/md/dm.c |  160 ++++++++++++++++++--------------------------------------
 1 file changed, 52 insertions(+), 108 deletions(-)

Thanks,
Kiyoshi Ueda

[PATCH 1/4] dm core: use BUG_ON in dm_end_request()

[Kiyoshi Ueda]

This patch replaces free_bio_clone() in dm_end_request() with BUG_ON().

free_bio_clone() was there to clean up clone.  However it is
definitely a bug if clone still has bios in dm_end_request().
So use BUG_ON() instead.

Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Alasdair G Kergon <agk@redhat.com>
---
 drivers/md/dm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: 2.6.31-rc/drivers/md/dm.c
===================================================================
--- 2.6.31-rc.orig/drivers/md/dm.c
+++ 2.6.31-rc/drivers/md/dm.c
@@ -845,7 +845,7 @@ static void dm_end_request(struct reques
 			rq->sense_len = clone->sense_len;
 	}
 
-	free_bio_clone(md, clone);
+	BUG_ON(clone->bio);
 	free_rq_tio(md, tio);
 
 	blk_end_request_all(rq, error);

[PATCH 2/4] dm core: clean up completion handling of clone bio

[Kiyoshi Ueda]

This patch cleans up completion handling of clone bio.
struct bio has ->bi_destructor to complete itself, so use it for
request-based dm, too.

Since the destructor can be called before the request is fully
cloned, the 'struct request *rq' of struct dm_rq_clone_bio_info
has been replaced with 'struct dm_rq_target_io *tio' so that
the destructor can find 'tio' safely (i.e. not via rq->end_io_data).

Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Alasdair G Kergon <agk@redhat.com>
---
 drivers/md/dm.c |   38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

Index: 2.6.31-rc/drivers/md/dm.c
===================================================================
--- 2.6.31-rc.orig/drivers/md/dm.c
+++ 2.6.31-rc/drivers/md/dm.c
@@ -78,7 +78,7 @@ struct dm_rq_target_io {
  */
 struct dm_rq_clone_bio_info {
 	struct bio *orig;
-	struct request *rq;
+	struct dm_rq_target_io *tio;
 };
 
 union map_info *dm_get_mapinfo(struct bio *bio)
@@ -666,12 +666,10 @@ static void clone_endio(struct bio *bio,
 static void end_clone_bio(struct bio *clone, int error)
 {
 	struct dm_rq_clone_bio_info *info = clone->bi_private;
-	struct dm_rq_target_io *tio = info->rq->end_io_data;
+	struct dm_rq_target_io *tio = info->tio;
 	struct bio *bio = info->orig;
 	unsigned int nr_bytes = info->orig->bi_size;
 
-	free_bio_info(tio->md, info);
-	clone->bi_private = tio->md->bs;
 	bio_put(clone);
 
 	if (tio->error)
@@ -712,18 +710,13 @@ static void end_clone_bio(struct bio *cl
 	blk_update_request(tio->orig, 0, nr_bytes);
 }
 
-static void free_bio_clone(struct mapped_device *md, struct request *clone)
+static void free_bio_clone(struct request *clone)
 {
 	struct bio *bio;
-	struct dm_rq_clone_bio_info *info;
 
 	while ((bio = clone->bio) != NULL) {
 		clone->bio = bio->bi_next;
 
-		info = bio->bi_private;
-		free_bio_info(md, info);
-
-		bio->bi_private = md->bs;
 		bio_put(bio);
 	}
 }
@@ -761,13 +754,12 @@ static void dm_unprep_request(struct req
 {
 	struct request *clone = rq->special;
 	struct dm_rq_target_io *tio = clone->end_io_data;
-	struct mapped_device *md = tio->md;
 
 	rq->special = NULL;
 	rq->cmd_flags &= ~REQ_DONTPREP;
 
-	free_bio_clone(md, clone);
-	free_rq_tio(md, tio);
+	free_bio_clone(clone);
+	free_rq_tio(tio->md, tio);
 }
 
 /*
@@ -1396,6 +1388,15 @@ void dm_dispatch_request(struct request 
 }
 EXPORT_SYMBOL_GPL(dm_dispatch_request);
 
+static void dm_rq_bio_destructor(struct bio *bio)
+{
+	struct dm_rq_clone_bio_info *info = bio->bi_private;
+	struct mapped_device *md = info->tio->md;
+
+	free_bio_info(md, info);
+	bio_free(bio, md->bs);
+}
+
 static void copy_request_info(struct request *clone, struct request *rq)
 {
 	clone->cpu = rq->cpu;
@@ -1414,8 +1415,9 @@ static void copy_request_info(struct req
 }
 
 static int clone_request_bios(struct request *clone, struct request *rq,
-			      struct mapped_device *md)
+			      struct dm_rq_target_io *tio)
 {
+	struct mapped_device *md = tio->md;
 	struct bio *bio, *clone_bio;
 	struct dm_rq_clone_bio_info *info;
 
@@ -1440,9 +1442,9 @@ static int clone_request_bios(struct req
 			goto free_and_out;
 		}
 
-		clone_bio->bi_destructor = dm_bio_destructor;
+		clone_bio->bi_destructor = dm_rq_bio_destructor;
 		clone_bio->bi_end_io = end_clone_bio;
-		info->rq = clone;
+		info->tio = tio;
 		info->orig = bio;
 		clone_bio->bi_private = info;
 
@@ -1456,7 +1458,7 @@ static int clone_request_bios(struct req
 	return 0;
 
 free_and_out:
-	free_bio_clone(md, clone);
+	free_bio_clone(clone);
 
 	return -ENOMEM;
 }
@@ -1468,7 +1470,7 @@ static int setup_clone(struct request *c
 
 	blk_rq_init(NULL, clone);
 
-	r = clone_request_bios(clone, rq, tio->md);
+	r = clone_request_bios(clone, rq, tio);
 	if (r)
 		return r;
 

[PATCH 3/4] dm core: remove 'md' argument from free_rq_tio()

[Kiyoshi Ueda]

This patch removes the unneeded argument 'md' from free_rq_tio().
'md' can be gotten by tio->md in free_rq_tio(), so removed it
from argument.

Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Alasdair G Kergon <agk@redhat.com>
---
 drivers/md/dm.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

Index: 2.6.31-rc/drivers/md/dm.c
===================================================================
--- 2.6.31-rc.orig/drivers/md/dm.c
+++ 2.6.31-rc/drivers/md/dm.c
@@ -434,9 +434,9 @@ static struct dm_rq_target_io *alloc_rq_
 	return mempool_alloc(md->tio_pool, GFP_ATOMIC);
 }
 
-static void free_rq_tio(struct mapped_device *md, struct dm_rq_target_io *tio)
+static void free_rq_tio(struct dm_rq_target_io *tio)
 {
-	mempool_free(tio, md->tio_pool);
+	mempool_free(tio, tio->md->tio_pool);
 }
 
 static struct dm_rq_clone_bio_info *alloc_bio_info(struct mapped_device *md)
@@ -444,10 +444,9 @@ static struct dm_rq_clone_bio_info *allo
 	return mempool_alloc(md->io_pool, GFP_ATOMIC);
 }
 
-static void free_bio_info(struct mapped_device *md,
-			  struct dm_rq_clone_bio_info *info)
+static void free_bio_info(struct dm_rq_clone_bio_info *info)
 {
-	mempool_free(info, md->io_pool);
+	mempool_free(info, info->tio->md->io_pool);
 }
 
 static void start_io_acct(struct dm_io *io)
@@ -759,7 +758,7 @@ static void dm_unprep_request(struct req
 	rq->cmd_flags &= ~REQ_DONTPREP;
 
 	free_bio_clone(clone);
-	free_rq_tio(tio->md, tio);
+	free_rq_tio(tio);
 }
 
 /*
@@ -838,7 +837,7 @@ static void dm_end_request(struct reques
 	}
 
 	BUG_ON(clone->bio);
-	free_rq_tio(md, tio);
+	free_rq_tio(tio);
 
 	blk_end_request_all(rq, error);
 
@@ -1393,7 +1392,7 @@ static void dm_rq_bio_destructor(struct 
 	struct dm_rq_clone_bio_info *info = bio->bi_private;
 	struct mapped_device *md = info->tio->md;
 
-	free_bio_info(md, info);
+	free_bio_info(info);
 	bio_free(bio, md->bs);
 }
 
@@ -1426,10 +1425,13 @@ static int clone_request_bios(struct req
 		if (!info)
 			goto free_and_out;
 
+		info->tio = tio;
+		info->orig = bio;
+
 		clone_bio = bio_alloc_bioset(GFP_ATOMIC, bio->bi_max_vecs,
 					     md->bs);
 		if (!clone_bio) {
-			free_bio_info(md, info);
+			free_bio_info(info);
 			goto free_and_out;
 		}
 
@@ -1438,14 +1440,12 @@ static int clone_request_bios(struct req
 		if (bio_integrity(bio) &&
 		    !bio_integrity_clone(clone_bio, bio, GFP_ATOMIC)) {
 			bio_free(clone_bio, md->bs);
-			free_bio_info(md, info);
+			free_bio_info(info);
 			goto free_and_out;
 		}
 
 		clone_bio->bi_destructor = dm_rq_bio_destructor;
 		clone_bio->bi_end_io = end_clone_bio;
-		info->tio = tio;
-		info->orig = bio;
 		clone_bio->bi_private = info;
 
 		if (clone->bio) {
@@ -1522,7 +1522,7 @@ static int dm_prep_fn(struct request_que
 	clone = &tio->clone;
 	if (setup_clone(clone, rq, tio)) {
 		/* -ENOMEM */
-		free_rq_tio(md, tio);
+		free_rq_tio(tio);
 		return BLKPREP_DEFER;
 	}
 

[PATCH 4/4] dm core: use generic clone setup interface

[Kiyoshi Ueda]

This patch changes to use the block-layer's clone setup interface
instead of setting clone up internally in request-based dm.

As a result, some internal functions aren't needed anymore,
so removed them.

Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Alasdair G Kergon <agk@redhat.com>
---
 drivers/md/dm.c |   94 ++++++++++----------------------------------------------
 1 file changed, 18 insertions(+), 76 deletions(-)

Index: 2.6.31-rc/drivers/md/dm.c
===================================================================
--- 2.6.31-rc.orig/drivers/md/dm.c
+++ 2.6.31-rc/drivers/md/dm.c
@@ -709,17 +709,6 @@ static void end_clone_bio(struct bio *cl
 	blk_update_request(tio->orig, 0, nr_bytes);
 }
 
-static void free_bio_clone(struct request *clone)
-{
-	struct bio *bio;
-
-	while ((bio = clone->bio) != NULL) {
-		clone->bio = bio->bi_next;
-
-		bio_put(bio);
-	}
-}
-
 /*
  * Don't touch any member of the md after calling this function because
  * the md may be freed in dm_put() at the end of this function.
@@ -757,7 +746,7 @@ static void dm_unprep_request(struct req
 	rq->special = NULL;
 	rq->cmd_flags &= ~REQ_DONTPREP;
 
-	free_bio_clone(clone);
+	blk_rq_unprep_clone(clone);
 	free_rq_tio(tio);
 }
 
@@ -1396,85 +1385,38 @@ static void dm_rq_bio_destructor(struct 
 	bio_free(bio, md->bs);
 }
 
-static void copy_request_info(struct request *clone, struct request *rq)
-{
-	clone->cpu = rq->cpu;
-	clone->cmd_flags = (rq_data_dir(rq) | REQ_NOMERGE);
-	clone->cmd_type = rq->cmd_type;
-	clone->__sector = blk_rq_pos(rq);
-	clone->__data_len = blk_rq_bytes(rq);
-	clone->nr_phys_segments = rq->nr_phys_segments;
-	clone->ioprio = rq->ioprio;
-	clone->buffer = rq->buffer;
-	clone->cmd_len = rq->cmd_len;
-	if (rq->cmd_len)
-		clone->cmd = rq->cmd;
-	clone->extra_len = rq->extra_len;
-	clone->sense = rq->sense;
-}
-
-static int clone_request_bios(struct request *clone, struct request *rq,
-			      struct dm_rq_target_io *tio)
+static int dm_rq_bio_constructor(struct bio *bio, struct bio *bio_orig,
+				 void *data)
 {
+	struct dm_rq_target_io *tio = data;
 	struct mapped_device *md = tio->md;
-	struct bio *bio, *clone_bio;
-	struct dm_rq_clone_bio_info *info;
+	struct dm_rq_clone_bio_info *info = alloc_bio_info(md);
 
-	for (bio = rq->bio; bio; bio = bio->bi_next) {
-		info = alloc_bio_info(md);
-		if (!info)
-			goto free_and_out;
-
-		info->tio = tio;
-		info->orig = bio;
-
-		clone_bio = bio_alloc_bioset(GFP_ATOMIC, bio->bi_max_vecs,
-					     md->bs);
-		if (!clone_bio) {
-			free_bio_info(info);
-			goto free_and_out;
-		}
-
-		__bio_clone(clone_bio, bio);
-
-		if (bio_integrity(bio) &&
-		    !bio_integrity_clone(clone_bio, bio, GFP_ATOMIC)) {
-			bio_free(clone_bio, md->bs);
-			free_bio_info(info);
-			goto free_and_out;
-		}
+	if (!info)
+		return -ENOMEM;
 
-		clone_bio->bi_destructor = dm_rq_bio_destructor;
-		clone_bio->bi_end_io = end_clone_bio;
-		clone_bio->bi_private = info;
-
-		if (clone->bio) {
-			clone->biotail->bi_next = clone_bio;
-			clone->biotail = clone_bio;
-		} else
-			clone->bio = clone->biotail = clone_bio;
-	}
+	info->orig = bio_orig;
+	info->tio = tio;
+	bio->bi_end_io = end_clone_bio;
+	bio->bi_private = info;
+	bio->bi_destructor = dm_rq_bio_destructor;
 
 	return 0;
-
-free_and_out:
-	free_bio_clone(clone);
-
-	return -ENOMEM;
 }
 
 static int setup_clone(struct request *clone, struct request *rq,
 		       struct dm_rq_target_io *tio)
 {
-	int r;
-
-	blk_rq_init(NULL, clone);
+	int r = blk_rq_prep_clone(clone, rq, tio->md->bs, GFP_ATOMIC,
+				  dm_rq_bio_constructor, tio);
 
-	r = clone_request_bios(clone, rq, tio);
 	if (r)
 		return r;
 
-	copy_request_info(clone, rq);
+	clone->cmd = rq->cmd;
+	clone->cmd_len = rq->cmd_len;
+	clone->sense = rq->sense;
+	clone->buffer = rq->buffer;
 	clone->end_io = end_clone_request;
 	clone->end_io_data = tio;