All of lore.kernel.org
 help / color / mirror / Atom feed
* passthrough request improvements V3
@ 2016-06-16  9:14 Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests Christoph Hellwig
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-16  9:14 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

These days we have a lot more users of struct request for driver internal
use than just REQ_TYPE_BLOCK_PC, so let's polish this infrastructure
up a bit.

Changes since V2:
 - really drop zeroing ->cmd from blk_get_request
 - add ACK from Boaz
Changes since V1:
 - keep blk_rq_set_block_pc for now and leave zeroing ->cmd in it

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests
  2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
@ 2016-06-16  9:14 ` Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 2/7] virtio_blk: use blk_rq_map_kern Christoph Hellwig
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-16  9:14 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

There is no code to issue or handle REQ_TYPE_BLOCK_PC request in the
memstick drivers, so remove the bogus conditional.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/memstick/core/ms_block.c    | 3 +--
 drivers/memstick/core/mspro_block.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 3cd6815..21cc231 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -2002,8 +2002,7 @@ static int msb_bd_getgeo(struct block_device *bdev,
 
 static int msb_prepare_req(struct request_queue *q, struct request *req)
 {
-	if (req->cmd_type != REQ_TYPE_FS &&
-				req->cmd_type != REQ_TYPE_BLOCK_PC) {
+	if (req->cmd_type != REQ_TYPE_FS) {
 		blk_dump_rq_flags(req, "MS unsupported request");
 		return BLKPREP_KILL;
 	}
diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index 0fb27d3..586fb8d 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -829,8 +829,7 @@ static void mspro_block_start(struct memstick_dev *card)
 
 static int mspro_block_prepare_req(struct request_queue *q, struct request *req)
 {
-	if (req->cmd_type != REQ_TYPE_FS &&
-	    req->cmd_type != REQ_TYPE_BLOCK_PC) {
+	if (req->cmd_type != REQ_TYPE_FS) {
 		blk_dump_rq_flags(req, "MSPro unsupported request");
 		return BLKPREP_KILL;
 	}
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/7] virtio_blk: use blk_rq_map_kern
  2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests Christoph Hellwig
@ 2016-06-16  9:14 ` Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-16  9:14 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

Similar to how SCSI and NVMe prepare passthrough requests.  This avoids
poking into request internals too much.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/virtio_blk.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 18e4069..963a130 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -236,25 +236,23 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
 static int virtblk_get_id(struct gendisk *disk, char *id_str)
 {
 	struct virtio_blk *vblk = disk->private_data;
+	struct request_queue *q = vblk->disk->queue;
 	struct request *req;
-	struct bio *bio;
 	int err;
 
-	bio = bio_map_kern(vblk->disk->queue, id_str, VIRTIO_BLK_ID_BYTES,
-			   GFP_KERNEL);
-	if (IS_ERR(bio))
-		return PTR_ERR(bio);
-
-	req = blk_make_request(vblk->disk->queue, bio, GFP_KERNEL);
-	if (IS_ERR(req)) {
-		bio_put(bio);
+	req = blk_get_request(q, READ, GFP_KERNEL);
+	if (IS_ERR(req))
 		return PTR_ERR(req);
-	}
-
+	blk_rq_set_block_pc(req);
 	req->cmd_type = REQ_TYPE_DRV_PRIV;
+
+	err = blk_rq_map_kern(q, req, id_str, VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
+	if (err)
+		goto out;
+
 	err = blk_execute_rq(vblk->disk->queue, vblk->disk, req, false);
+out:
 	blk_put_request(req);
-
 	return err;
 }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized
  2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 2/7] virtio_blk: use blk_rq_map_kern Christoph Hellwig
@ 2016-06-16  9:14 ` Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 4/7] block: simplify and export blk_rq_append_bio Christoph Hellwig
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-16  9:14 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

blk_get_request is used for BLOCK_PC and similar passthrough requests.
Currently we always need to call blk_rq_set_block_pc or an open coded
version of it to allow appending bios using the request mapping helpers
later on, which is a somewhat awkward API.  Instead move the
initialization part of blk_rq_set_block_pc into blk_get_request, so that
we always have a safe to use request.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c           | 12 +++++++-----
 block/blk-mq.c             |  4 ++++
 drivers/block/virtio_blk.c |  1 -
 drivers/nvme/host/core.c   |  4 ----
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index c94c7ad..8416c4a 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1294,10 +1294,15 @@ static struct request *blk_old_get_request(struct request_queue *q, int rw,
 
 	spin_lock_irq(q->queue_lock);
 	rq = get_request(q, rw, 0, NULL, gfp_mask);
-	if (IS_ERR(rq))
+	if (IS_ERR(rq)) {
 		spin_unlock_irq(q->queue_lock);
-	/* q->queue_lock is unlocked at this point */
+		return rq;
+	}
 
+	/* q->queue_lock is unlocked at this point */
+	rq->__data_len = 0;
+	rq->__sector = (sector_t) -1;
+	rq->bio = rq->biotail = NULL;
 	return rq;
 }
 
@@ -1377,9 +1382,6 @@ EXPORT_SYMBOL(blk_make_request);
 void blk_rq_set_block_pc(struct request *rq)
 {
 	rq->cmd_type = REQ_TYPE_BLOCK_PC;
-	rq->__data_len = 0;
-	rq->__sector = (sector_t) -1;
-	rq->bio = rq->biotail = NULL;
 	memset(rq->__cmd, 0, sizeof(rq->__cmd));
 }
 EXPORT_SYMBOL(blk_rq_set_block_pc);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index bc7166d..b5606dc 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -263,6 +263,10 @@ struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
 		blk_queue_exit(q);
 		return ERR_PTR(-EWOULDBLOCK);
 	}
+
+	rq->__data_len = 0;
+	rq->__sector = (sector_t) -1;
+	rq->bio = rq->biotail = NULL;
 	return rq;
 }
 EXPORT_SYMBOL(blk_mq_alloc_request);
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 963a130..0be7dd6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -243,7 +243,6 @@ static int virtblk_get_id(struct gendisk *disk, char *id_str)
 	req = blk_get_request(q, READ, GFP_KERNEL);
 	if (IS_ERR(req))
 		return PTR_ERR(req);
-	blk_rq_set_block_pc(req);
 	req->cmd_type = REQ_TYPE_DRV_PRIV;
 
 	err = blk_rq_map_kern(q, req, id_str, VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9d7cee4..cb8ca1b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -201,10 +201,6 @@ struct request *nvme_alloc_request(struct request_queue *q,
 
 	req->cmd_type = REQ_TYPE_DRV_PRIV;
 	req->cmd_flags |= REQ_FAILFAST_DRIVER;
-	req->__data_len = 0;
-	req->__sector = (sector_t) -1;
-	req->bio = req->biotail = NULL;
-
 	req->cmd = (unsigned char *)cmd;
 	req->cmd_len = sizeof(struct nvme_command);
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/7] block: simplify and export blk_rq_append_bio
  2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2016-06-16  9:14 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig
@ 2016-06-16  9:14 ` Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 5/7] target: stop using blk_make_request Christoph Hellwig
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-16  9:14 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

The target SCSI passthrough backend is much better served with the low-level
blk_rq_append_bio construct then the helpers built on top of it, so export it.

Also use the opportunity to remove the pointless request_queue argument and
make the code flow a little more readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c       |  2 +-
 block/blk-map.c        | 25 +++++++++++++++----------
 block/blk.h            |  2 --
 include/linux/blkdev.h |  1 +
 4 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 8416c4a..1952c52 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1363,7 +1363,7 @@ struct request *blk_make_request(struct request_queue *q, struct bio *bio,
 		int ret;
 
 		blk_queue_bounce(q, &bounce_bio);
-		ret = blk_rq_append_bio(q, rq, bounce_bio);
+		ret = blk_rq_append_bio(rq, bounce_bio);
 		if (unlikely(ret)) {
 			blk_put_request(rq);
 			return ERR_PTR(ret);
diff --git a/block/blk-map.c b/block/blk-map.c
index 61733a6..b8657fa 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -9,21 +9,26 @@
 
 #include "blk.h"
 
-int blk_rq_append_bio(struct request_queue *q, struct request *rq,
-		      struct bio *bio)
+/*
+ * Append a bio to a passthrough request.  Only works can be merged into
+ * the request based on the driver constraints.
+ */
+int blk_rq_append_bio(struct request *rq, struct bio *bio)
 {
-	if (!rq->bio)
-		blk_rq_bio_prep(q, rq, bio);
-	else if (!ll_back_merge_fn(q, rq, bio))
-		return -EINVAL;
-	else {
+	if (!rq->bio) {
+		blk_rq_bio_prep(rq->q, rq, bio);
+	} else {
+		if (!ll_back_merge_fn(rq->q, rq, bio))
+			return -EINVAL;
+
 		rq->biotail->bi_next = bio;
 		rq->biotail = bio;
-
 		rq->__data_len += bio->bi_iter.bi_size;
 	}
+
 	return 0;
 }
+EXPORT_SYMBOL(blk_rq_append_bio);
 
 static int __blk_rq_unmap_user(struct bio *bio)
 {
@@ -71,7 +76,7 @@ static int __blk_rq_map_user_iov(struct request *rq,
 	 */
 	bio_get(bio);
 
-	ret = blk_rq_append_bio(q, rq, bio);
+	ret = blk_rq_append_bio(rq, bio);
 	if (ret) {
 		bio_endio(bio);
 		__blk_rq_unmap_user(orig_bio);
@@ -229,7 +234,7 @@ int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf,
 	if (do_copy)
 		rq->cmd_flags |= REQ_COPY_USER;
 
-	ret = blk_rq_append_bio(q, rq, bio);
+	ret = blk_rq_append_bio(rq, bio);
 	if (unlikely(ret)) {
 		/* request is too big */
 		bio_put(bio);
diff --git a/block/blk.h b/block/blk.h
index 70e4aee..c37492f 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -64,8 +64,6 @@ void blk_exit_rl(struct request_list *rl);
 void init_request_from_bio(struct request *req, struct bio *bio);
 void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
 			struct bio *bio);
-int blk_rq_append_bio(struct request_queue *q, struct request *rq,
-		      struct bio *bio);
 void blk_queue_bypass_start(struct request_queue *q);
 void blk_queue_bypass_end(struct request_queue *q);
 void blk_dequeue_request(struct request *rq);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9d1e0a4..07176f4 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -801,6 +801,7 @@ extern int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
 extern void blk_rq_unprep_clone(struct request *rq);
 extern int blk_insert_cloned_request(struct request_queue *q,
 				     struct request *rq);
+extern int blk_rq_append_bio(struct request *rq, struct bio *bio);
 extern void blk_delay_queue(struct request_queue *, unsigned long);
 extern void blk_queue_split(struct request_queue *, struct bio **,
 			    struct bio_set *);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/7] target: stop using blk_make_request
  2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2016-06-16  9:14 ` [PATCH 4/7] block: simplify and export blk_rq_append_bio Christoph Hellwig
@ 2016-06-16  9:14 ` Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 6/7] scsi/osd: open code blk_make_request Christoph Hellwig
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-16  9:14 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

Using blk_rq_append_bio allows to append the bios to the request
directly instead of having to build up a list first, and also
allows to have a single code path for requests with or without
data attached to them.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/target/target_core_pscsi.c | 87 +++++++++++++++-----------------------
 1 file changed, 34 insertions(+), 53 deletions(-)

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 81564c8..9125d93 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -876,19 +876,19 @@ static inline struct bio *pscsi_get_bio(int nr_vecs)
 
 static sense_reason_t
 pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
-		enum dma_data_direction data_direction, struct bio **hbio)
+		struct request *req)
 {
 	struct pscsi_dev_virt *pdv = PSCSI_DEV(cmd->se_dev);
-	struct bio *bio = NULL, *tbio = NULL;
+	struct bio *bio = NULL;
 	struct page *page;
 	struct scatterlist *sg;
 	u32 data_len = cmd->data_length, i, len, bytes, off;
 	int nr_pages = (cmd->data_length + sgl[0].offset +
 			PAGE_SIZE - 1) >> PAGE_SHIFT;
 	int nr_vecs = 0, rc;
-	int rw = (data_direction == DMA_TO_DEVICE);
+	int rw = (cmd->data_direction == DMA_TO_DEVICE);
 
-	*hbio = NULL;
+	BUG_ON(!cmd->data_length);
 
 	pr_debug("PSCSI: nr_pages: %d\n", nr_pages);
 
@@ -927,16 +927,6 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 				pr_debug("PSCSI: Allocated bio: %p,"
 					" dir: %s nr_vecs: %d\n", bio,
 					(rw) ? "rw" : "r", nr_vecs);
-				/*
-				 * Set *hbio pointer to handle the case:
-				 * nr_pages > BIO_MAX_PAGES, where additional
-				 * bios need to be added to complete a given
-				 * command.
-				 */
-				if (!*hbio)
-					*hbio = tbio = bio;
-				else
-					tbio = tbio->bi_next = bio;
 			}
 
 			pr_debug("PSCSI: Calling bio_add_pc_page() i: %d"
@@ -955,11 +945,16 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 				pr_debug("PSCSI: Reached bio->bi_vcnt max:"
 					" %d i: %d bio: %p, allocating another"
 					" bio\n", bio->bi_vcnt, i, bio);
+
+				rc = blk_rq_append_bio(req, bio);
+				if (rc) {
+					pr_err("pSCSI: failed to append bio\n");
+					goto fail;
+				}
+
 				/*
 				 * Clear the pointer so that another bio will
-				 * be allocated with pscsi_get_bio() above, the
-				 * current bio has already been set *tbio and
-				 * bio->bi_next.
+				 * be allocated with pscsi_get_bio() above.
 				 */
 				bio = NULL;
 			}
@@ -968,13 +963,16 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 		}
 	}
 
+	if (bio) {
+		rc = blk_rq_append_bio(req, bio);
+		if (rc) {
+			pr_err("pSCSI: failed to append bio\n");
+			goto fail;
+		}
+	}
+
 	return 0;
 fail:
-	while (*hbio) {
-		bio = *hbio;
-		*hbio = (*hbio)->bi_next;
-		bio_endio(bio);
-	}
 	return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 }
 
@@ -992,11 +990,9 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 {
 	struct scatterlist *sgl = cmd->t_data_sg;
 	u32 sgl_nents = cmd->t_data_nents;
-	enum dma_data_direction data_direction = cmd->data_direction;
 	struct pscsi_dev_virt *pdv = PSCSI_DEV(cmd->se_dev);
 	struct pscsi_plugin_task *pt;
 	struct request *req;
-	struct bio *hbio;
 	sense_reason_t ret;
 
 	/*
@@ -1012,31 +1008,21 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 	memcpy(pt->pscsi_cdb, cmd->t_task_cdb,
 		scsi_command_size(cmd->t_task_cdb));
 
-	if (!sgl) {
-		req = blk_get_request(pdv->pdv_sd->request_queue,
-				(data_direction == DMA_TO_DEVICE),
-				GFP_KERNEL);
-		if (IS_ERR(req)) {
-			pr_err("PSCSI: blk_get_request() failed\n");
-			ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-			goto fail;
-		}
+	req = blk_get_request(pdv->pdv_sd->request_queue,
+			(cmd->data_direction == DMA_TO_DEVICE),
+			GFP_KERNEL);
+	if (IS_ERR(req)) {
+		pr_err("PSCSI: blk_get_request() failed\n");
+		ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+		goto fail;
+	}
 
-		blk_rq_set_block_pc(req);
-	} else {
-		BUG_ON(!cmd->data_length);
+	blk_rq_set_block_pc(req);
 
-		ret = pscsi_map_sg(cmd, sgl, sgl_nents, data_direction, &hbio);
+	if (sgl) {
+		ret = pscsi_map_sg(cmd, sgl, sgl_nents, req);
 		if (ret)
-			goto fail;
-
-		req = blk_make_request(pdv->pdv_sd->request_queue, hbio,
-				       GFP_KERNEL);
-		if (IS_ERR(req)) {
-			pr_err("pSCSI: blk_make_request() failed\n");
-			ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-			goto fail_free_bio;
-		}
+			goto fail_put_request;
 	}
 
 	req->end_io = pscsi_req_done;
@@ -1057,13 +1043,8 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 
 	return 0;
 
-fail_free_bio:
-	while (hbio) {
-		struct bio *bio = hbio;
-		hbio = hbio->bi_next;
-		bio_endio(bio);
-	}
-	ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+fail_put_request:
+	blk_put_request(req);
 fail:
 	kfree(pt);
 	return ret;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/7] scsi/osd: open code blk_make_request
  2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2016-06-16  9:14 ` [PATCH 5/7] target: stop using blk_make_request Christoph Hellwig
@ 2016-06-16  9:14 ` Christoph Hellwig
  2016-06-16  9:14 ` [PATCH 7/7] block: unexport various bio mapping helpers Christoph Hellwig
  2016-07-08  9:36 ` passthrough request improvements V3 Christoph Hellwig
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-16  9:14 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

I wish the OSD code could simply use blk_rq_map_* helpers like
everyone else, but the complex nature of deciding if we have
DATA IN and/or DATA OUT buffers might make this impossible
(at least for a mere human like me).

But using blk_rq_append_bio at least allows sharing the setup code
between request with or without dat a buffers, and given that this
is the last user of blk_make_request it allows getting rid of that
somewhat awkward interface.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Boaz Harrosh <ooo@electrozaur.com>
---
 block/blk-core.c                 | 57 ----------------------------------------
 drivers/scsi/osd/osd_initiator.c | 25 +++++++++++-------
 include/linux/blkdev.h           |  2 --
 3 files changed, 16 insertions(+), 68 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1952c52..85fdb99 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1318,63 +1318,6 @@ struct request *blk_get_request(struct request_queue *q, int rw, gfp_t gfp_mask)
 EXPORT_SYMBOL(blk_get_request);
 
 /**
- * blk_make_request - given a bio, allocate a corresponding struct request.
- * @q: target request queue
- * @bio:  The bio describing the memory mappings that will be submitted for IO.
- *        It may be a chained-bio properly constructed by block/bio layer.
- * @gfp_mask: gfp flags to be used for memory allocation
- *
- * blk_make_request is the parallel of generic_make_request for BLOCK_PC
- * type commands. Where the struct request needs to be farther initialized by
- * the caller. It is passed a &struct bio, which describes the memory info of
- * the I/O transfer.
- *
- * The caller of blk_make_request must make sure that bi_io_vec
- * are set to describe the memory buffers. That bio_data_dir() will return
- * the needed direction of the request. (And all bio's in the passed bio-chain
- * are properly set accordingly)
- *
- * If called under none-sleepable conditions, mapped bio buffers must not
- * need bouncing, by calling the appropriate masked or flagged allocator,
- * suitable for the target device. Otherwise the call to blk_queue_bounce will
- * BUG.
- *
- * WARNING: When allocating/cloning a bio-chain, careful consideration should be
- * given to how you allocate bios. In particular, you cannot use
- * __GFP_DIRECT_RECLAIM for anything but the first bio in the chain. Otherwise
- * you risk waiting for IO completion of a bio that hasn't been submitted yet,
- * thus resulting in a deadlock. Alternatively bios should be allocated using
- * bio_kmalloc() instead of bio_alloc(), as that avoids the mempool deadlock.
- * If possible a big IO should be split into smaller parts when allocation
- * fails. Partial allocation should not be an error, or you risk a live-lock.
- */
-struct request *blk_make_request(struct request_queue *q, struct bio *bio,
-				 gfp_t gfp_mask)
-{
-	struct request *rq = blk_get_request(q, bio_data_dir(bio), gfp_mask);
-
-	if (IS_ERR(rq))
-		return rq;
-
-	blk_rq_set_block_pc(rq);
-
-	for_each_bio(bio) {
-		struct bio *bounce_bio = bio;
-		int ret;
-
-		blk_queue_bounce(q, &bounce_bio);
-		ret = blk_rq_append_bio(rq, bounce_bio);
-		if (unlikely(ret)) {
-			blk_put_request(rq);
-			return ERR_PTR(ret);
-		}
-	}
-
-	return rq;
-}
-EXPORT_SYMBOL(blk_make_request);
-
-/**
  * blk_rq_set_block_pc - initialize a request to type BLOCK_PC
  * @rq:		request to be initialized
  *
diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index daa4dc1..2f2a991 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -1558,18 +1558,25 @@ static int _osd_req_finalize_data_integrity(struct osd_request *or,
 static struct request *_make_request(struct request_queue *q, bool has_write,
 			      struct _osd_io_info *oii, gfp_t flags)
 {
-	if (oii->bio)
-		return blk_make_request(q, oii->bio, flags);
-	else {
-		struct request *req;
-
-		req = blk_get_request(q, has_write ? WRITE : READ, flags);
-		if (IS_ERR(req))
-			return req;
+	struct request *req;
+	struct bio *bio = oii->bio;
+	int ret;
 
-		blk_rq_set_block_pc(req);
+	req = blk_get_request(q, has_write ? WRITE : READ, flags);
+	if (IS_ERR(req))
 		return req;
+	blk_rq_set_block_pc(req);
+
+	for_each_bio(bio) {
+		struct bio *bounce_bio = bio;
+
+		blk_queue_bounce(req->q, &bounce_bio);
+		ret = blk_rq_append_bio(req, bounce_bio);
+		if (ret)
+			return ERR_PTR(ret);
 	}
+
+	return req;
 }
 
 static int _init_blk_request(struct osd_request *or,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 07176f4..3be95eb 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -787,8 +787,6 @@ extern void blk_rq_init(struct request_queue *q, struct request *rq);
 extern void blk_put_request(struct request *);
 extern void __blk_put_request(struct request_queue *, struct request *);
 extern struct request *blk_get_request(struct request_queue *, int, gfp_t);
-extern struct request *blk_make_request(struct request_queue *, struct bio *,
-					gfp_t);
 extern void blk_rq_set_block_pc(struct request *);
 extern void blk_requeue_request(struct request_queue *, struct request *);
 extern void blk_add_request_payload(struct request *rq, struct page *page,
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 7/7] block: unexport various bio mapping helpers
  2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2016-06-16  9:14 ` [PATCH 6/7] scsi/osd: open code blk_make_request Christoph Hellwig
@ 2016-06-16  9:14 ` Christoph Hellwig
  2016-07-08  9:36 ` passthrough request improvements V3 Christoph Hellwig
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-16  9:14 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

They are unused and potential new users really should use the
blk_rq_map* versions.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 848cd35..b88ed1c 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1097,7 +1097,6 @@ int bio_uncopy_user(struct bio *bio)
 	bio_put(bio);
 	return ret;
 }
-EXPORT_SYMBOL(bio_uncopy_user);
 
 /**
  *	bio_copy_user_iov	-	copy user data to bio
@@ -1392,7 +1391,6 @@ void bio_unmap_user(struct bio *bio)
 	__bio_unmap_user(bio);
 	bio_put(bio);
 }
-EXPORT_SYMBOL(bio_unmap_user);
 
 static void bio_map_kern_endio(struct bio *bio)
 {
@@ -1538,7 +1536,6 @@ cleanup:
 	bio_put(bio);
 	return ERR_PTR(-ENOMEM);
 }
-EXPORT_SYMBOL(bio_copy_kern);
 
 /*
  * bio_set_pages_dirty() and bio_check_pages_dirty() are support functions
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: passthrough request improvements V3
  2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2016-06-16  9:14 ` [PATCH 7/7] block: unexport various bio mapping helpers Christoph Hellwig
@ 2016-07-08  9:36 ` Christoph Hellwig
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-07-08  9:36 UTC (permalink / raw)
  To: axboe; +Cc: mst, ooo, nab, linux-block

ping?  Seems like you were mostly happy with the previous versions,
and I should have addressed all comments.

On Thu, Jun 16, 2016 at 11:14:24AM +0200, Christoph Hellwig wrote:
> These days we have a lot more users of struct request for driver internal
> use than just REQ_TYPE_BLOCK_PC, so let's polish this infrastructure
> up a bit.
> 
> Changes since V2:
>  - really drop zeroing ->cmd from blk_get_request
>  - add ACK from Boaz
> Changes since V1:
>  - keep blk_rq_set_block_pc for now and leave zeroing ->cmd in it
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-07-08  9:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-16  9:14 passthrough request improvements V3 Christoph Hellwig
2016-06-16  9:14 ` [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests Christoph Hellwig
2016-06-16  9:14 ` [PATCH 2/7] virtio_blk: use blk_rq_map_kern Christoph Hellwig
2016-06-16  9:14 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig
2016-06-16  9:14 ` [PATCH 4/7] block: simplify and export blk_rq_append_bio Christoph Hellwig
2016-06-16  9:14 ` [PATCH 5/7] target: stop using blk_make_request Christoph Hellwig
2016-06-16  9:14 ` [PATCH 6/7] scsi/osd: open code blk_make_request Christoph Hellwig
2016-06-16  9:14 ` [PATCH 7/7] block: unexport various bio mapping helpers Christoph Hellwig
2016-07-08  9:36 ` passthrough request improvements V3 Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.