* a few passthrough request improvements V2
@ 2016-06-14 17:15 Christoph Hellwig
2016-06-14 17:15 ` [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests Christoph Hellwig
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Christoph Hellwig @ 2016-06-14 17:15 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 V1:
- keep blk_rq_set_block_pc for now and leave zeroing ->cmd in it
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests 2016-06-14 17:15 a few passthrough request improvements V2 Christoph Hellwig @ 2016-06-14 17:15 ` Christoph Hellwig 2016-06-14 17:15 ` [PATCH 2/7] virtio_blk: use blk_rq_map_kern Christoph Hellwig ` (5 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-06-14 17:15 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] 19+ messages in thread
* [PATCH 2/7] virtio_blk: use blk_rq_map_kern 2016-06-14 17:15 a few passthrough request improvements V2 Christoph Hellwig 2016-06-14 17:15 ` [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests Christoph Hellwig @ 2016-06-14 17:15 ` Christoph Hellwig 2016-06-14 17:16 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig ` (4 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-06-14 17:15 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] 19+ messages in thread
* [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-14 17:15 a few passthrough request improvements V2 Christoph Hellwig 2016-06-14 17:15 ` [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests Christoph Hellwig 2016-06-14 17:15 ` [PATCH 2/7] virtio_blk: use blk_rq_map_kern Christoph Hellwig @ 2016-06-14 17:16 ` Christoph Hellwig 2016-06-15 9:02 ` Jens Axboe 2016-06-14 17:16 ` [PATCH 4/7] block: simplify and export blk_rq_append_bio Christoph Hellwig ` (3 subsequent siblings) 6 siblings, 1 reply; 19+ messages in thread From: Christoph Hellwig @ 2016-06-14 17:16 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 | 13 ++++++++----- block/blk-mq.c | 5 +++++ drivers/block/virtio_blk.c | 1 - drivers/nvme/host/core.c | 4 ---- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index c94c7ad..37852ec 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1294,10 +1294,16 @@ 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; + memset(rq->__cmd, 0, sizeof(rq->__cmd)); return rq; } @@ -1377,9 +1383,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..80dce26 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -263,6 +263,11 @@ 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; + memset(rq->__cmd, 0, sizeof(rq->__cmd)); 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] 19+ messages in thread
* Re: [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-14 17:16 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig @ 2016-06-15 9:02 ` Jens Axboe 2016-06-15 10:07 ` Christoph Hellwig 0 siblings, 1 reply; 19+ messages in thread From: Jens Axboe @ 2016-06-15 9:02 UTC (permalink / raw) To: Christoph Hellwig; +Cc: mst, ooo, nab, linux-block On 06/14/2016 07:16 PM, Christoph Hellwig wrote: > 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. This still puts the pc related memset() into the normal fast path... -- Jens Axboe ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-15 9:02 ` Jens Axboe @ 2016-06-15 10:07 ` Christoph Hellwig 2016-06-15 10:17 ` Jens Axboe 0 siblings, 1 reply; 19+ messages in thread From: Christoph Hellwig @ 2016-06-15 10:07 UTC (permalink / raw) To: Jens Axboe; +Cc: Christoph Hellwig, mst, ooo, nab, linux-block On Wed, Jun 15, 2016 at 11:02:18AM +0200, Jens Axboe wrote: > On 06/14/2016 07:16 PM, Christoph Hellwig wrote: >> 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. > > This still puts the pc related memset() into the normal fast path... Oops, I missed removing it from the alloc path when adding back blk_rq_set_block_pc, will fix it up. But I have to object to this actually being a fast path - it's not used for any fs request, just BLOCK_PC, NVMe passthrough and various driver specific little hacks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-15 10:07 ` Christoph Hellwig @ 2016-06-15 10:17 ` Jens Axboe 2016-06-15 10:25 ` Christoph Hellwig 0 siblings, 1 reply; 19+ messages in thread From: Jens Axboe @ 2016-06-15 10:17 UTC (permalink / raw) To: Christoph Hellwig; +Cc: mst, ooo, nab, linux-block On 06/15/2016 12:07 PM, Christoph Hellwig wrote: > On Wed, Jun 15, 2016 at 11:02:18AM +0200, Jens Axboe wrote: >> On 06/14/2016 07:16 PM, Christoph Hellwig wrote: >>> 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. >> >> This still puts the pc related memset() into the normal fast path... > > Oops, I missed removing it from the alloc path when adding back > blk_rq_set_block_pc, will fix it up. > > But I have to object to this actually being a fast path - it's not > used for any fs request, just BLOCK_PC, NVMe passthrough and various > driver specific little hacks. I guess you are right, the fs path jumps in via __blk_mq_alloc_request(), so not fs fast path. But it'd still be nice to properly fix this. Can it wait until the rq->pc mess is resolved? -- Jens Axboe ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-15 10:17 ` Jens Axboe @ 2016-06-15 10:25 ` Christoph Hellwig 0 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-06-15 10:25 UTC (permalink / raw) To: Jens Axboe; +Cc: Christoph Hellwig, mst, ooo, nab, linux-block On Wed, Jun 15, 2016 at 12:17:47PM +0200, Jens Axboe wrote: > I guess you are right, the fs path jumps in via __blk_mq_alloc_request(), > so not fs fast path. But it'd still be nice to properly fix this. Can it > wait until the rq->pc mess is resolved? I'll resend the series removing the memset in the blk_get_request path, which I think is very useful for now as the requests currently returned from blk_get_request / blk_mq_alloc_request are inherently dangerous. req->cmd zeroing is really just a workaround broken devices in practice, as spec complicant scsi device never should look at the cmd array beyond the command length. I'll try to expedite the pc_request split, but I'd really like to get all the NVMe over Fabrics work in first, as we'd be in merge hell otherwise. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/7] block: simplify and export blk_rq_append_bio 2016-06-14 17:15 a few passthrough request improvements V2 Christoph Hellwig ` (2 preceding siblings ...) 2016-06-14 17:16 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig @ 2016-06-14 17:16 ` Christoph Hellwig 2016-06-14 17:16 ` [PATCH 5/7] target: stop using blk_make_request Christoph Hellwig ` (2 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-06-14 17:16 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 37852ec..2c4c021 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1364,7 +1364,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] 19+ messages in thread
* [PATCH 5/7] target: stop using blk_make_request 2016-06-14 17:15 a few passthrough request improvements V2 Christoph Hellwig ` (3 preceding siblings ...) 2016-06-14 17:16 ` [PATCH 4/7] block: simplify and export blk_rq_append_bio Christoph Hellwig @ 2016-06-14 17:16 ` Christoph Hellwig 2016-06-14 17:16 ` [PATCH 6/7] scsi/osd: open code blk_make_request Christoph Hellwig 2016-06-14 17:16 ` [PATCH 7/7] block: unexport various bio mapping helpers Christoph Hellwig 6 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-06-14 17:16 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] 19+ messages in thread
* [PATCH 6/7] scsi/osd: open code blk_make_request 2016-06-14 17:15 a few passthrough request improvements V2 Christoph Hellwig ` (4 preceding siblings ...) 2016-06-14 17:16 ` [PATCH 5/7] target: stop using blk_make_request Christoph Hellwig @ 2016-06-14 17:16 ` Christoph Hellwig 2016-06-14 17:16 ` [PATCH 7/7] block: unexport various bio mapping helpers Christoph Hellwig 6 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-06-14 17:16 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> --- 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 2c4c021..ecf7e49 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1319,63 +1319,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] 19+ messages in thread
* [PATCH 7/7] block: unexport various bio mapping helpers 2016-06-14 17:15 a few passthrough request improvements V2 Christoph Hellwig ` (5 preceding siblings ...) 2016-06-14 17:16 ` [PATCH 6/7] scsi/osd: open code blk_make_request Christoph Hellwig @ 2016-06-14 17:16 ` Christoph Hellwig 6 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-06-14 17:16 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] 19+ messages in thread
* resend: passthrough request improvements V3 @ 2016-07-19 9:31 Christoph Hellwig 2016-07-19 9:31 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig 0 siblings, 1 reply; 19+ messages in thread From: Christoph Hellwig @ 2016-07-19 9:31 UTC (permalink / raw) To: axboe; +Cc: 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. Unchanged from the last post on June 16th. 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] 19+ messages in thread
* [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-07-19 9:31 resend: passthrough request improvements V3 Christoph Hellwig @ 2016-07-19 9:31 ` Christoph Hellwig 0 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-07-19 9:31 UTC (permalink / raw) To: axboe; +Cc: 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 d66509f..3d22fa2 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 5e0fe60..3a52b69 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 a85a143..1523e05 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 cf4b750..9f0ec3b 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -222,10 +222,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] 19+ messages in thread
* passthrough request improvements V3 @ 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 0 siblings, 1 reply; 19+ 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] 19+ 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 ` Christoph Hellwig 0 siblings, 0 replies; 19+ 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] 19+ messages in thread
* a few passthrough request improvements @ 2016-06-13 15:21 Christoph Hellwig 2016-06-13 15:21 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig 0 siblings, 1 reply; 19+ messages in thread From: Christoph Hellwig @ 2016-06-13 15:21 UTC (permalink / raw) To: axboe; +Cc: mst, ooo, bhalevy, 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-13 15:21 a few passthrough request improvements Christoph Hellwig @ 2016-06-13 15:21 ` Christoph Hellwig 2016-06-14 2:17 ` Jens Axboe 2016-06-15 9:57 ` Boaz Harrosh 0 siblings, 2 replies; 19+ messages in thread From: Christoph Hellwig @ 2016-06-13 15:21 UTC (permalink / raw) To: axboe; +Cc: mst, ooo, bhalevy, nab, linux-block blk_get_request is used for BLOCK_PC and similar pass through requests. Currently we always need to call blk_rq_set_block_pc or an opencoded 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. blk_rq_set_block_pc now goes away in favor of just setting cmd_type to REQ_TYPE_BLOCK_PC, or nothing in case it was overriden with a different type a little later (or earlier in case of the SCSI OSD code..) Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/blk-core.c | 27 +++++++++------------------ block/blk-mq.c | 5 +++++ block/bsg.c | 2 +- block/scsi_ioctl.c | 6 +++--- drivers/block/pktcdvd.c | 2 +- drivers/block/virtio_blk.c | 1 - drivers/cdrom/cdrom.c | 2 +- drivers/nvme/host/core.c | 4 ---- drivers/scsi/device_handler/scsi_dh_emc.c | 2 +- drivers/scsi/device_handler/scsi_dh_hp_sw.c | 4 ++-- drivers/scsi/device_handler/scsi_dh_rdac.c | 2 +- drivers/scsi/osd/osd_initiator.c | 3 +-- drivers/scsi/osst.c | 2 +- drivers/scsi/scsi_error.c | 2 +- drivers/scsi/scsi_lib.c | 2 +- drivers/scsi/sg.c | 2 +- drivers/scsi/st.c | 2 +- drivers/target/target_core_pscsi.c | 2 +- fs/nfsd/blocklayout.c | 2 +- include/linux/blkdev.h | 1 - 20 files changed, 32 insertions(+), 43 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index c94c7ad..2b179cd 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1294,10 +1294,16 @@ 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; + memset(rq->__cmd, 0, sizeof(rq->__cmd)); return rq; } @@ -1351,7 +1357,7 @@ struct request *blk_make_request(struct request_queue *q, struct bio *bio, if (IS_ERR(rq)) return rq; - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; for_each_bio(bio) { struct bio *bounce_bio = bio; @@ -1370,21 +1376,6 @@ struct request *blk_make_request(struct request_queue *q, struct bio *bio, EXPORT_SYMBOL(blk_make_request); /** - * blk_rq_set_block_pc - initialize a request to type BLOCK_PC - * @rq: request to be initialized - * - */ -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); - -/** * blk_requeue_request - put a request back on queue * @q: request queue where request should be inserted * @rq: request to be inserted diff --git a/block/blk-mq.c b/block/blk-mq.c index bc7166d..80dce26 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -263,6 +263,11 @@ 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; + memset(rq->__cmd, 0, sizeof(rq->__cmd)); return rq; } EXPORT_SYMBOL(blk_mq_alloc_request); diff --git a/block/bsg.c b/block/bsg.c index d214e92..c0199d7 100644 --- a/block/bsg.c +++ b/block/bsg.c @@ -236,7 +236,7 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm, rq = blk_get_request(q, rw, GFP_KERNEL); if (IS_ERR(rq)) return rq; - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; ret = blk_fill_sgv4_hdr_rq(q, rq, hdr, bd, has_write_perm); if (ret) diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c index 0774799..3750ddb 100644 --- a/block/scsi_ioctl.c +++ b/block/scsi_ioctl.c @@ -318,7 +318,7 @@ static int sg_io(struct request_queue *q, struct gendisk *bd_disk, rq = blk_get_request(q, writing ? WRITE : READ, GFP_KERNEL); if (IS_ERR(rq)) return PTR_ERR(rq); - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; if (hdr->cmd_len > BLK_MAX_CDB) { rq->cmd = kzalloc(hdr->cmd_len, GFP_KERNEL); @@ -449,7 +449,7 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk, fmode_t mode, err = PTR_ERR(rq); goto error_free_buffer; } - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; cmdlen = COMMAND_SIZE(opcode); @@ -539,7 +539,7 @@ static int __blk_send_generic(struct request_queue *q, struct gendisk *bd_disk, rq = blk_get_request(q, WRITE, __GFP_RECLAIM); if (IS_ERR(rq)) return PTR_ERR(rq); - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; rq->timeout = BLK_DEFAULT_SG_TIMEOUT; rq->cmd[0] = cmd; rq->cmd[4] = data; diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index 9393bc7..66ad416 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -707,7 +707,7 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command * WRITE : READ, __GFP_RECLAIM); if (IS_ERR(rq)) return PTR_ERR(rq); - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; if (cgc->buflen) { ret = blk_rq_map_kern(q, rq, cgc->buffer, cgc->buflen, 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/cdrom/cdrom.c b/drivers/cdrom/cdrom.c index 1b257ea..6d20659 100644 --- a/drivers/cdrom/cdrom.c +++ b/drivers/cdrom/cdrom.c @@ -2185,7 +2185,7 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf, ret = PTR_ERR(rq); break; } - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; ret = blk_rq_map_user(q, rq, NULL, ubuf, len, GFP_KERNEL); if (ret) { 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); diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c index 375d818..f6ebf6f 100644 --- a/drivers/scsi/device_handler/scsi_dh_emc.c +++ b/drivers/scsi/device_handler/scsi_dh_emc.c @@ -277,7 +277,7 @@ static struct request *get_req(struct scsi_device *sdev, int cmd, return NULL; } - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; rq->cmd_len = COMMAND_SIZE(cmd); rq->cmd[0] = cmd; diff --git a/drivers/scsi/device_handler/scsi_dh_hp_sw.c b/drivers/scsi/device_handler/scsi_dh_hp_sw.c index 9406d5f..4ae6c15 100644 --- a/drivers/scsi/device_handler/scsi_dh_hp_sw.c +++ b/drivers/scsi/device_handler/scsi_dh_hp_sw.c @@ -113,7 +113,7 @@ retry: if (IS_ERR(req)) return SCSI_DH_RES_TEMP_UNAVAIL; - blk_rq_set_block_pc(req); + req->cmd_type = REQ_TYPE_BLOCK_PC; req->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER; req->cmd_len = COMMAND_SIZE(TEST_UNIT_READY); @@ -243,7 +243,7 @@ static int hp_sw_start_stop(struct hp_sw_dh_data *h) if (IS_ERR(req)) return SCSI_DH_RES_TEMP_UNAVAIL; - blk_rq_set_block_pc(req); + req->cmd_type = REQ_TYPE_BLOCK_PC; req->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER; req->cmd_len = COMMAND_SIZE(START_STOP); diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c index 06fbd0b..1b84b4a 100644 --- a/drivers/scsi/device_handler/scsi_dh_rdac.c +++ b/drivers/scsi/device_handler/scsi_dh_rdac.c @@ -275,7 +275,7 @@ static struct request *get_rdac_req(struct scsi_device *sdev, "get_rdac_req: blk_get_request failed.\n"); return NULL; } - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; if (buflen && blk_rq_map_kern(q, rq, buffer, buflen, GFP_NOIO)) { blk_put_request(rq); diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c index daa4dc1..868ae29d 100644 --- a/drivers/scsi/osd/osd_initiator.c +++ b/drivers/scsi/osd/osd_initiator.c @@ -1567,7 +1567,7 @@ static struct request *_make_request(struct request_queue *q, bool has_write, if (IS_ERR(req)) return req; - blk_rq_set_block_pc(req); + req->cmd_type = REQ_TYPE_BLOCK_PC; return req; } } @@ -1605,7 +1605,6 @@ static int _init_blk_request(struct osd_request *or, ret = PTR_ERR(req); goto out; } - blk_rq_set_block_pc(req); or->in.req = or->request->next_rq = req; } } else if (has_in) diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c index 5033223..6816089 100644 --- a/drivers/scsi/osst.c +++ b/drivers/scsi/osst.c @@ -367,7 +367,7 @@ static int osst_execute(struct osst_request *SRpnt, const unsigned char *cmd, if (IS_ERR(req)) return DRIVER_ERROR << 24; - blk_rq_set_block_pc(req); + req->cmd_type = REQ_TYPE_BLOCK_PC; req->cmd_flags |= REQ_QUIET; SRpnt->bio = NULL; diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index a8b610e..b6b6d92 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1978,7 +1978,7 @@ static void scsi_eh_lock_door(struct scsi_device *sdev) if (IS_ERR(req)) return; - blk_rq_set_block_pc(req); + req->cmd_type = REQ_TYPE_BLOCK_PC; req->cmd[0] = ALLOW_MEDIUM_REMOVAL; req->cmd[1] = 0; diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index c71344a..292f713 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -191,7 +191,7 @@ int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, req = blk_get_request(sdev->request_queue, write, __GFP_RECLAIM); if (IS_ERR(req)) return ret; - blk_rq_set_block_pc(req); + req->cmd_type = REQ_TYPE_BLOCK_PC; if (bufflen && blk_rq_map_kern(sdev->request_queue, req, buffer, bufflen, __GFP_RECLAIM)) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index ae7d9bd..398c04e 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -1708,7 +1708,7 @@ sg_start_req(Sg_request *srp, unsigned char *cmd) return PTR_ERR(rq); } - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; if (hp->cmd_len > BLK_MAX_CDB) rq->cmd = long_cmdp; diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c index 7af5226..8179ee3 100644 --- a/drivers/scsi/st.c +++ b/drivers/scsi/st.c @@ -545,7 +545,7 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd, if (IS_ERR(req)) return DRIVER_ERROR << 24; - blk_rq_set_block_pc(req); + req->cmd_type = REQ_TYPE_BLOCK_PC; req->cmd_flags |= REQ_QUIET; mdata->null_mapped = 1; diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c index 81564c8..1f30179 100644 --- a/drivers/target/target_core_pscsi.c +++ b/drivers/target/target_core_pscsi.c @@ -1022,7 +1022,7 @@ pscsi_execute_cmd(struct se_cmd *cmd) goto fail; } - blk_rq_set_block_pc(req); + req->cmd_type = REQ_TYPE_BLOCK_PC; } else { BUG_ON(!cmd->data_length); diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c index e55b524..cdadcf8 100644 --- a/fs/nfsd/blocklayout.c +++ b/fs/nfsd/blocklayout.c @@ -224,7 +224,7 @@ static int nfsd4_scsi_identify_device(struct block_device *bdev, error = -ENOMEM; goto out_free_buf; } - blk_rq_set_block_pc(rq); + rq->cmd_type = REQ_TYPE_BLOCK_PC; error = blk_rq_map_kern(q, rq, buf, bufflen, GFP_KERNEL); if (error) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 9d1e0a4..472b527 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -789,7 +789,6 @@ 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, int offset, unsigned int len); -- 2.1.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-13 15:21 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig @ 2016-06-14 2:17 ` Jens Axboe 2016-06-14 13:43 ` Christoph Hellwig 2016-06-15 9:57 ` Boaz Harrosh 1 sibling, 1 reply; 19+ messages in thread From: Jens Axboe @ 2016-06-14 2:17 UTC (permalink / raw) To: Christoph Hellwig; +Cc: mst, ooo, bhalevy, nab, linux-block On 06/13/2016 09:21 AM, Christoph Hellwig wrote: > blk_get_request is used for BLOCK_PC and similar pass through requests. > Currently we always need to call blk_rq_set_block_pc or an opencoded > 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. blk_rq_set_block_pc now goes away > in favor of just setting cmd_type to REQ_TYPE_BLOCK_PC, or nothing in case > it was overriden with a different type a little later (or earlier in case > of the SCSI OSD code..) It may be awkward, but we have those to avoid doing things like this: + rq->__data_len = 0; + rq->__sector = (sector_t) -1; + rq->bio = rq->biotail = NULL; + memset(rq->__cmd, 0, sizeof(rq->__cmd)); for every request we allocate, when we don't use ->cmd at all. Honest, I'd rather have struct request *blk_get_pc_request(); and similar helpers around this, so we don't have to do extra initialization when we don't need it. -- Jens Axboe ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-14 2:17 ` Jens Axboe @ 2016-06-14 13:43 ` Christoph Hellwig 2016-06-14 15:04 ` Jens Axboe 0 siblings, 1 reply; 19+ messages in thread From: Christoph Hellwig @ 2016-06-14 13:43 UTC (permalink / raw) To: Jens Axboe; +Cc: Christoph Hellwig, mst, ooo, bhalevy, nab, linux-block On Mon, Jun 13, 2016 at 08:17:38PM -0600, Jens Axboe wrote: > It may be awkward, but we have those to avoid doing things like this: > > + rq->__data_len = 0; > + rq->__sector = (sector_t) -1; > + rq->bio = rq->biotail = NULL; > + memset(rq->__cmd, 0, sizeof(rq->__cmd)); > > for every request we allocate, when we don't use ->cmd at all. Honest, I'd > rather have > > struct request *blk_get_pc_request(); > > and similar helpers around this, so we don't have to do extra > initialization when we don't need it. I'm working on resurrecting my patch to remove rq->cmd and friends from the common request structure, but the full patch is something I'd rather do after the initial NVMe over Fabrics merge. If you're fine with the rest of the series I'll respin it to keep blk_set_block_pc for now which will only do the zeroing and setting cmd_type. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-14 13:43 ` Christoph Hellwig @ 2016-06-14 15:04 ` Jens Axboe 0 siblings, 0 replies; 19+ messages in thread From: Jens Axboe @ 2016-06-14 15:04 UTC (permalink / raw) To: Christoph Hellwig; +Cc: mst, ooo, bhalevy, nab, linux-block On 06/14/2016 07:43 AM, Christoph Hellwig wrote: > On Mon, Jun 13, 2016 at 08:17:38PM -0600, Jens Axboe wrote: >> It may be awkward, but we have those to avoid doing things like this: >> >> + rq->__data_len = 0; >> + rq->__sector = (sector_t) -1; >> + rq->bio = rq->biotail = NULL; >> + memset(rq->__cmd, 0, sizeof(rq->__cmd)); >> >> for every request we allocate, when we don't use ->cmd at all. Honest, I'd >> rather have >> >> struct request *blk_get_pc_request(); >> >> and similar helpers around this, so we don't have to do extra >> initialization when we don't need it. > > I'm working on resurrecting my patch to remove rq->cmd and friends from > the common request structure, That'd be great! And then we can unify the request setup after that. > but the full patch is something I'd rather > do after the initial NVMe over Fabrics merge. If you're fine with the > rest of the series I'll respin it to keep blk_set_block_pc for now which > will only do the zeroing and setting cmd_type. Yes that sounds good, the rest looks fine to me. -- Jens Axboe ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized 2016-06-13 15:21 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig 2016-06-14 2:17 ` Jens Axboe @ 2016-06-15 9:57 ` Boaz Harrosh 1 sibling, 0 replies; 19+ messages in thread From: Boaz Harrosh @ 2016-06-15 9:57 UTC (permalink / raw) To: Christoph Hellwig, axboe; +Cc: mst, bhalevy, nab, linux-block On 06/13/2016 06:21 PM, Christoph Hellwig wrote: > blk_get_request is used for BLOCK_PC and similar pass through requests. > Currently we always need to call blk_rq_set_block_pc or an opencoded > 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. blk_rq_set_block_pc now goes away > in favor of just setting cmd_type to REQ_TYPE_BLOCK_PC, or nothing in case > it was overriden with a different type a little later (or earlier in case > of the SCSI OSD code..) > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- Is good thanks. Review-by: Boaz harrosh <ooo@electrozaur.com> <> > diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c > index daa4dc1..868ae29d 100644 > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -1567,7 +1567,7 @@ static struct request *_make_request(struct request_queue *q, bool has_write, > if (IS_ERR(req)) > return req; > > - blk_rq_set_block_pc(req); > + req->cmd_type = REQ_TYPE_BLOCK_PC; > return req; > } > } > @@ -1605,7 +1605,6 @@ static int _init_blk_request(struct osd_request *or, > ret = PTR_ERR(req); > goto out; > } > - blk_rq_set_block_pc(req); > or->in.req = or->request->next_rq = req; Yes this is the bidi part of the request it need not be of any type. Only properly initialized. > } > } else if (has_in) <> ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-07-19 9:31 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-14 17:15 a few passthrough request improvements V2 Christoph Hellwig 2016-06-14 17:15 ` [PATCH 1/7] memstick: don't allow REQ_TYPE_BLOCK_PC requests Christoph Hellwig 2016-06-14 17:15 ` [PATCH 2/7] virtio_blk: use blk_rq_map_kern Christoph Hellwig 2016-06-14 17:16 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig 2016-06-15 9:02 ` Jens Axboe 2016-06-15 10:07 ` Christoph Hellwig 2016-06-15 10:17 ` Jens Axboe 2016-06-15 10:25 ` Christoph Hellwig 2016-06-14 17:16 ` [PATCH 4/7] block: simplify and export blk_rq_append_bio Christoph Hellwig 2016-06-14 17:16 ` [PATCH 5/7] target: stop using blk_make_request Christoph Hellwig 2016-06-14 17:16 ` [PATCH 6/7] scsi/osd: open code blk_make_request Christoph Hellwig 2016-06-14 17:16 ` [PATCH 7/7] block: unexport various bio mapping helpers Christoph Hellwig -- strict thread matches above, loose matches on Subject: below -- 2016-07-19 9:31 resend: passthrough request improvements V3 Christoph Hellwig 2016-07-19 9:31 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig 2016-06-16 9:14 passthrough request improvements V3 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-13 15:21 a few passthrough request improvements Christoph Hellwig 2016-06-13 15:21 ` [PATCH 3/7] block: ensure bios return from blk_get_request are properly initialized Christoph Hellwig 2016-06-14 2:17 ` Jens Axboe 2016-06-14 13:43 ` Christoph Hellwig 2016-06-14 15:04 ` Jens Axboe 2016-06-15 9:57 ` Boaz Harrosh
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.