* fix data transfer size caculation for special payload requests
@ 2017-01-13 11:29 ` Christoph Hellwig
0 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
Hi all,
the first two fixes fix a regression in 4.10-rc1 where the data transfer
size for discard commands wasn't set properly, leading to hangs with
Hyper-V VMs. The use the new data transfer size helper added in patch 1
more widely.
^ permalink raw reply [flat|nested] 39+ messages in thread
* fix data transfer size caculation for special payload requests
@ 2017-01-13 11:29 ` Christoph Hellwig
0 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
Hi all,
the first two fixes fix a regression in 4.10-rc1 where the data transfer
size for discard commands wasn't set properly, leading to hangs with
Hyper-V VMs. The use the new data transfer size helper added in patch 1
more widely.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/4] block: add blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 11:29 ` Christoph Hellwig
-1 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
Add a helper to calculate the actual data transfer size for special
payload requests.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/blkdev.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ff3d774..1ca8e8f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
return blk_rq_cur_bytes(rq) >> 9;
}
+/*
+ * Some commands like WRITE SAME have a payload or data transfer size which
+ * is different from the size of the request. Any driver that supports such
+ * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
+ * calculate the data transfer size.
+ */
+static inline unsigned int blk_rq_payload_bytes(struct request *rq)
+{
+ if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
+ return rq->special_vec.bv_len;
+ return blk_rq_bytes(rq);
+}
+
static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
int op)
{
--
2.1.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 1/4] block: add blk_rq_payload_bytes
@ 2017-01-13 11:29 ` Christoph Hellwig
0 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
Add a helper to calculate the actual data transfer size for special
payload requests.
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
include/linux/blkdev.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ff3d774..1ca8e8f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
return blk_rq_cur_bytes(rq) >> 9;
}
+/*
+ * Some commands like WRITE SAME have a payload or data transfer size which
+ * is different from the size of the request. Any driver that supports such
+ * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
+ * calculate the data transfer size.
+ */
+static inline unsigned int blk_rq_payload_bytes(struct request *rq)
+{
+ if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
+ return rq->special_vec.bv_len;
+ return blk_rq_bytes(rq);
+}
+
static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
int op)
{
--
2.1.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 2/4] scsi: use blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 11:29 ` Christoph Hellwig
-1 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
Without that we'll pass a wrong payload size in cmd->sdb, which
can lead to hangs with drivers that need the total transfer size.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Chris Valean <v-chvale@microsoft.com>
Reported-by: Dexuan Cui <decui@microsoft.com>
Fixes: f9d03f96 ("block: improve handling of the magic discard payload")
---
drivers/scsi/scsi_lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c35b6de..ad4ff8f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1018,7 +1018,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
BUG_ON(count > sdb->table.nents);
sdb->table.nents = count;
- sdb->length = blk_rq_bytes(req);
+ sdb->length = blk_rq_payload_bytes(req);
return BLKPREP_OK;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 2/4] scsi: use blk_rq_payload_bytes
@ 2017-01-13 11:29 ` Christoph Hellwig
0 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
Without that we'll pass a wrong payload size in cmd->sdb, which
can lead to hangs with drivers that need the total transfer size.
Signed-off-by: Christoph Hellwig <hch at lst.de>
Reported-by: Chris Valean <v-chvale at microsoft.com>
Reported-by: Dexuan Cui <decui at microsoft.com>
Fixes: f9d03f96 ("block: improve handling of the magic discard payload")
---
drivers/scsi/scsi_lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c35b6de..ad4ff8f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1018,7 +1018,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
BUG_ON(count > sdb->table.nents);
sdb->table.nents = count;
- sdb->length = blk_rq_bytes(req);
+ sdb->length = blk_rq_payload_bytes(req);
return BLKPREP_OK;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 3/4] nvme: use blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 11:29 ` Christoph Hellwig
-1 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
The new blk_rq_payload_bytes generalizes the payload length hacks
that nvme_map_len did before.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/fc.c | 5 ++---
drivers/nvme/host/nvme.h | 8 --------
drivers/nvme/host/pci.c | 19 ++++++++-----------
drivers/nvme/host/rdma.c | 13 +++++--------
4 files changed, 15 insertions(+), 30 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index aa0bc60..fcc9dcf 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1654,13 +1654,12 @@ nvme_fc_map_data(struct nvme_fc_ctrl *ctrl, struct request *rq,
struct nvme_fc_fcp_op *op)
{
struct nvmefc_fcp_req *freq = &op->fcp_req;
- u32 map_len = nvme_map_len(rq);
enum dma_data_direction dir;
int ret;
freq->sg_cnt = 0;
- if (!map_len)
+ if (!blk_rq_payload_bytes(rq))
return 0;
freq->sg_table.sgl = freq->first_sgl;
@@ -1854,7 +1853,7 @@ nvme_fc_queue_rq(struct blk_mq_hw_ctx *hctx,
if (ret)
return ret;
- data_len = nvme_map_len(rq);
+ data_len = blk_rq_payload_bytes(rq);
if (data_len)
io_dir = ((rq_data_dir(rq) == WRITE) ?
NVMEFC_FCP_WRITE : NVMEFC_FCP_READ);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 6377e14..aead6d0 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -225,14 +225,6 @@ static inline u64 nvme_block_nr(struct nvme_ns *ns, sector_t sector)
return (sector >> (ns->lba_shift - 9));
}
-static inline unsigned nvme_map_len(struct request *rq)
-{
- if (req_op(rq) == REQ_OP_DISCARD)
- return sizeof(struct nvme_dsm_range);
- else
- return blk_rq_bytes(rq);
-}
-
static inline void nvme_cleanup_cmd(struct request *req)
{
if (req->rq_flags & RQF_SPECIAL_PAYLOAD) {
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 19beeb7..3faefab 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -306,11 +306,11 @@ static __le64 **iod_list(struct request *req)
return (__le64 **)(iod->sg + blk_rq_nr_phys_segments(req));
}
-static int nvme_init_iod(struct request *rq, unsigned size,
- struct nvme_dev *dev)
+static int nvme_init_iod(struct request *rq, struct nvme_dev *dev)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(rq);
int nseg = blk_rq_nr_phys_segments(rq);
+ unsigned int size = blk_rq_payload_bytes(rq);
if (nseg > NVME_INT_PAGES || size > NVME_INT_BYTES(dev)) {
iod->sg = kmalloc(nvme_iod_alloc_size(dev, size, nseg), GFP_ATOMIC);
@@ -420,12 +420,11 @@ static void nvme_dif_complete(u32 p, u32 v, struct t10_pi_tuple *pi)
}
#endif
-static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req,
- int total_len)
+static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct dma_pool *pool;
- int length = total_len;
+ int length = blk_rq_payload_bytes(req);
struct scatterlist *sg = iod->sg;
int dma_len = sg_dma_len(sg);
u64 dma_addr = sg_dma_address(sg);
@@ -501,7 +500,7 @@ static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req,
}
static int nvme_map_data(struct nvme_dev *dev, struct request *req,
- unsigned size, struct nvme_command *cmnd)
+ struct nvme_command *cmnd)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct request_queue *q = req->q;
@@ -519,7 +518,7 @@ static int nvme_map_data(struct nvme_dev *dev, struct request *req,
DMA_ATTR_NO_WARN))
goto out;
- if (!nvme_setup_prps(dev, req, size))
+ if (!nvme_setup_prps(dev, req))
goto out_unmap;
ret = BLK_MQ_RQ_QUEUE_ERROR;
@@ -580,7 +579,6 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
struct nvme_dev *dev = nvmeq->dev;
struct request *req = bd->rq;
struct nvme_command cmnd;
- unsigned map_len;
int ret = BLK_MQ_RQ_QUEUE_OK;
/*
@@ -600,13 +598,12 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
if (ret != BLK_MQ_RQ_QUEUE_OK)
return ret;
- map_len = nvme_map_len(req);
- ret = nvme_init_iod(req, map_len, dev);
+ ret = nvme_init_iod(req, dev);
if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out_free_cmd;
if (blk_rq_nr_phys_segments(req))
- ret = nvme_map_data(dev, req, map_len, &cmnd);
+ ret = nvme_map_data(dev, req, &cmnd);
if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out_cleanup_iod;
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 34e5648..557f29b 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -981,8 +981,7 @@ static int nvme_rdma_map_sg_fr(struct nvme_rdma_queue *queue,
}
static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
- struct request *rq, unsigned int map_len,
- struct nvme_command *c)
+ struct request *rq, struct nvme_command *c)
{
struct nvme_rdma_request *req = blk_mq_rq_to_pdu(rq);
struct nvme_rdma_device *dev = queue->device;
@@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
}
if (count == 1) {
- if (rq_data_dir(rq) == WRITE &&
- map_len <= nvme_rdma_inline_data_size(queue) &&
- nvme_rdma_queue_idx(queue))
+ if (rq_data_dir(rq) == WRITE && nvme_rdma_queue_idx(queue) &&
+ blk_rq_payload_bytes(rq) <=
+ nvme_rdma_inline_data_size(queue))
return nvme_rdma_map_sg_inline(queue, req, c);
if (dev->pd->flags & IB_PD_UNSAFE_GLOBAL_RKEY)
@@ -1444,7 +1443,6 @@ static int nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
struct nvme_command *c = sqe->data;
bool flush = false;
struct ib_device *dev;
- unsigned int map_len;
int ret;
WARN_ON_ONCE(rq->tag < 0);
@@ -1462,8 +1460,7 @@ static int nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
blk_mq_start_request(rq);
- map_len = nvme_map_len(rq);
- ret = nvme_rdma_map_data(queue, rq, map_len, c);
+ ret = nvme_rdma_map_data(queue, rq, c);
if (ret < 0) {
dev_err(queue->ctrl->ctrl.device,
"Failed to map data (%d)\n", ret);
--
2.1.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 3/4] nvme: use blk_rq_payload_bytes
@ 2017-01-13 11:29 ` Christoph Hellwig
0 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
The new blk_rq_payload_bytes generalizes the payload length hacks
that nvme_map_len did before.
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
drivers/nvme/host/fc.c | 5 ++---
drivers/nvme/host/nvme.h | 8 --------
drivers/nvme/host/pci.c | 19 ++++++++-----------
drivers/nvme/host/rdma.c | 13 +++++--------
4 files changed, 15 insertions(+), 30 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index aa0bc60..fcc9dcf 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1654,13 +1654,12 @@ nvme_fc_map_data(struct nvme_fc_ctrl *ctrl, struct request *rq,
struct nvme_fc_fcp_op *op)
{
struct nvmefc_fcp_req *freq = &op->fcp_req;
- u32 map_len = nvme_map_len(rq);
enum dma_data_direction dir;
int ret;
freq->sg_cnt = 0;
- if (!map_len)
+ if (!blk_rq_payload_bytes(rq))
return 0;
freq->sg_table.sgl = freq->first_sgl;
@@ -1854,7 +1853,7 @@ nvme_fc_queue_rq(struct blk_mq_hw_ctx *hctx,
if (ret)
return ret;
- data_len = nvme_map_len(rq);
+ data_len = blk_rq_payload_bytes(rq);
if (data_len)
io_dir = ((rq_data_dir(rq) == WRITE) ?
NVMEFC_FCP_WRITE : NVMEFC_FCP_READ);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 6377e14..aead6d0 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -225,14 +225,6 @@ static inline u64 nvme_block_nr(struct nvme_ns *ns, sector_t sector)
return (sector >> (ns->lba_shift - 9));
}
-static inline unsigned nvme_map_len(struct request *rq)
-{
- if (req_op(rq) == REQ_OP_DISCARD)
- return sizeof(struct nvme_dsm_range);
- else
- return blk_rq_bytes(rq);
-}
-
static inline void nvme_cleanup_cmd(struct request *req)
{
if (req->rq_flags & RQF_SPECIAL_PAYLOAD) {
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 19beeb7..3faefab 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -306,11 +306,11 @@ static __le64 **iod_list(struct request *req)
return (__le64 **)(iod->sg + blk_rq_nr_phys_segments(req));
}
-static int nvme_init_iod(struct request *rq, unsigned size,
- struct nvme_dev *dev)
+static int nvme_init_iod(struct request *rq, struct nvme_dev *dev)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(rq);
int nseg = blk_rq_nr_phys_segments(rq);
+ unsigned int size = blk_rq_payload_bytes(rq);
if (nseg > NVME_INT_PAGES || size > NVME_INT_BYTES(dev)) {
iod->sg = kmalloc(nvme_iod_alloc_size(dev, size, nseg), GFP_ATOMIC);
@@ -420,12 +420,11 @@ static void nvme_dif_complete(u32 p, u32 v, struct t10_pi_tuple *pi)
}
#endif
-static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req,
- int total_len)
+static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct dma_pool *pool;
- int length = total_len;
+ int length = blk_rq_payload_bytes(req);
struct scatterlist *sg = iod->sg;
int dma_len = sg_dma_len(sg);
u64 dma_addr = sg_dma_address(sg);
@@ -501,7 +500,7 @@ static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req,
}
static int nvme_map_data(struct nvme_dev *dev, struct request *req,
- unsigned size, struct nvme_command *cmnd)
+ struct nvme_command *cmnd)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct request_queue *q = req->q;
@@ -519,7 +518,7 @@ static int nvme_map_data(struct nvme_dev *dev, struct request *req,
DMA_ATTR_NO_WARN))
goto out;
- if (!nvme_setup_prps(dev, req, size))
+ if (!nvme_setup_prps(dev, req))
goto out_unmap;
ret = BLK_MQ_RQ_QUEUE_ERROR;
@@ -580,7 +579,6 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
struct nvme_dev *dev = nvmeq->dev;
struct request *req = bd->rq;
struct nvme_command cmnd;
- unsigned map_len;
int ret = BLK_MQ_RQ_QUEUE_OK;
/*
@@ -600,13 +598,12 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
if (ret != BLK_MQ_RQ_QUEUE_OK)
return ret;
- map_len = nvme_map_len(req);
- ret = nvme_init_iod(req, map_len, dev);
+ ret = nvme_init_iod(req, dev);
if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out_free_cmd;
if (blk_rq_nr_phys_segments(req))
- ret = nvme_map_data(dev, req, map_len, &cmnd);
+ ret = nvme_map_data(dev, req, &cmnd);
if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out_cleanup_iod;
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 34e5648..557f29b 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -981,8 +981,7 @@ static int nvme_rdma_map_sg_fr(struct nvme_rdma_queue *queue,
}
static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
- struct request *rq, unsigned int map_len,
- struct nvme_command *c)
+ struct request *rq, struct nvme_command *c)
{
struct nvme_rdma_request *req = blk_mq_rq_to_pdu(rq);
struct nvme_rdma_device *dev = queue->device;
@@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
}
if (count == 1) {
- if (rq_data_dir(rq) == WRITE &&
- map_len <= nvme_rdma_inline_data_size(queue) &&
- nvme_rdma_queue_idx(queue))
+ if (rq_data_dir(rq) == WRITE && nvme_rdma_queue_idx(queue) &&
+ blk_rq_payload_bytes(rq) <=
+ nvme_rdma_inline_data_size(queue))
return nvme_rdma_map_sg_inline(queue, req, c);
if (dev->pd->flags & IB_PD_UNSAFE_GLOBAL_RKEY)
@@ -1444,7 +1443,6 @@ static int nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
struct nvme_command *c = sqe->data;
bool flush = false;
struct ib_device *dev;
- unsigned int map_len;
int ret;
WARN_ON_ONCE(rq->tag < 0);
@@ -1462,8 +1460,7 @@ static int nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
blk_mq_start_request(rq);
- map_len = nvme_map_len(rq);
- ret = nvme_rdma_map_data(queue, rq, map_len, c);
+ ret = nvme_rdma_map_data(queue, rq, c);
if (ret < 0) {
dev_err(queue->ctrl->ctrl.device,
"Failed to map data (%d)\n", ret);
--
2.1.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 4/4] sd: remove __data_len hack for WRITE SAME
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 11:29 ` Christoph Hellwig
-1 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
Now that we have the blk_rq_payload_bytes helper available to determine
the actual I/O size we don't need to mess around with __data_len for
WRITE SAME.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/sd.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b193304..1fbb1ec 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -836,7 +836,6 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
struct bio *bio = rq->bio;
sector_t sector = blk_rq_pos(rq);
unsigned int nr_sectors = blk_rq_sectors(rq);
- unsigned int nr_bytes = blk_rq_bytes(rq);
int ret;
if (sdkp->device->no_write_same)
@@ -869,21 +868,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
cmd->transfersize = sdp->sector_size;
cmd->allowed = SD_MAX_RETRIES;
-
- /*
- * For WRITE_SAME the data transferred in the DATA IN buffer is
- * different from the amount of data actually written to the target.
- *
- * We set up __data_len to the amount of data transferred from the
- * DATA IN buffer so that blk_rq_map_sg set up the proper S/G list
- * to transfer a single sector of data first, but then reset it to
- * the amount of data to be written right after so that the I/O path
- * knows how much to actually write.
- */
- rq->__data_len = sdp->sector_size;
- ret = scsi_init_io(cmd);
- rq->__data_len = nr_bytes;
- return ret;
+ return scsi_init_io(cmd);
}
static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
--
2.1.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 4/4] sd: remove __data_len hack for WRITE SAME
@ 2017-01-13 11:29 ` Christoph Hellwig
0 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-13 11:29 UTC (permalink / raw)
Now that we have the blk_rq_payload_bytes helper available to determine
the actual I/O size we don't need to mess around with __data_len for
WRITE SAME.
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
drivers/scsi/sd.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b193304..1fbb1ec 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -836,7 +836,6 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
struct bio *bio = rq->bio;
sector_t sector = blk_rq_pos(rq);
unsigned int nr_sectors = blk_rq_sectors(rq);
- unsigned int nr_bytes = blk_rq_bytes(rq);
int ret;
if (sdkp->device->no_write_same)
@@ -869,21 +868,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
cmd->transfersize = sdp->sector_size;
cmd->allowed = SD_MAX_RETRIES;
-
- /*
- * For WRITE_SAME the data transferred in the DATA IN buffer is
- * different from the amount of data actually written to the target.
- *
- * We set up __data_len to the amount of data transferred from the
- * DATA IN buffer so that blk_rq_map_sg set up the proper S/G list
- * to transfer a single sector of data first, but then reset it to
- * the amount of data to be written right after so that the I/O path
- * knows how much to actually write.
- */
- rq->__data_len = sdp->sector_size;
- ret = scsi_init_io(cmd);
- rq->__data_len = nr_bytes;
- return ret;
+ return scsi_init_io(cmd);
}
static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
--
2.1.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] block: add blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
(?)
@ 2017-01-13 11:43 ` Hannes Reinecke
-1 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Add a helper to calculate the actual data transfer size for special
> payload requests.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> include/linux/blkdev.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ff3d774..1ca8e8f 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
> return blk_rq_cur_bytes(rq) >> 9;
> }
>
> +/*
> + * Some commands like WRITE SAME have a payload or data transfer size which
> + * is different from the size of the request. Any driver that supports such
> + * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
> + * calculate the data transfer size.
> + */
> +static inline unsigned int blk_rq_payload_bytes(struct request *rq)
> +{
> + if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
> + return rq->special_vec.bv_len;
> + return blk_rq_bytes(rq);
> +}
> +
> static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
> int op)
> {
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: F. Imend�rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N�rnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] block: add blk_rq_payload_bytes
@ 2017-01-13 11:43 ` Hannes Reinecke
0 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Add a helper to calculate the actual data transfer size for special
> payload requests.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> include/linux/blkdev.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ff3d774..1ca8e8f 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
> return blk_rq_cur_bytes(rq) >> 9;
> }
>
> +/*
> + * Some commands like WRITE SAME have a payload or data transfer size which
> + * is different from the size of the request. Any driver that supports such
> + * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
> + * calculate the data transfer size.
> + */
> +static inline unsigned int blk_rq_payload_bytes(struct request *rq)
> +{
> + if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
> + return rq->special_vec.bv_len;
> + return blk_rq_bytes(rq);
> +}
> +
> static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
> int op)
> {
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/4] block: add blk_rq_payload_bytes
@ 2017-01-13 11:43 ` Hannes Reinecke
0 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Add a helper to calculate the actual data transfer size for special
> payload requests.
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
> include/linux/blkdev.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ff3d774..1ca8e8f 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
> return blk_rq_cur_bytes(rq) >> 9;
> }
>
> +/*
> + * Some commands like WRITE SAME have a payload or data transfer size which
> + * is different from the size of the request. Any driver that supports such
> + * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
> + * calculate the data transfer size.
> + */
> +static inline unsigned int blk_rq_payload_bytes(struct request *rq)
> +{
> + if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
> + return rq->special_vec.bv_len;
> + return blk_rq_bytes(rq);
> +}
> +
> static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
> int op)
> {
>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare at suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/4] scsi: use blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
(?)
@ 2017-01-13 11:43 ` Hannes Reinecke
-1 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Without that we'll pass a wrong payload size in cmd->sdb, which
> can lead to hangs with drivers that need the total transfer size.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Chris Valean <v-chvale@microsoft.com>
> Reported-by: Dexuan Cui <decui@microsoft.com>
> Fixes: f9d03f96 ("block: improve handling of the magic discard payload")
> ---
> drivers/scsi/scsi_lib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index c35b6de..ad4ff8f 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1018,7 +1018,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
> count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
> BUG_ON(count > sdb->table.nents);
> sdb->table.nents = count;
> - sdb->length = blk_rq_bytes(req);
> + sdb->length = blk_rq_payload_bytes(req);
> return BLKPREP_OK;
> }
>
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: F. Imend�rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N�rnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/4] scsi: use blk_rq_payload_bytes
@ 2017-01-13 11:43 ` Hannes Reinecke
0 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Without that we'll pass a wrong payload size in cmd->sdb, which
> can lead to hangs with drivers that need the total transfer size.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Chris Valean <v-chvale@microsoft.com>
> Reported-by: Dexuan Cui <decui@microsoft.com>
> Fixes: f9d03f96 ("block: improve handling of the magic discard payload")
> ---
> drivers/scsi/scsi_lib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index c35b6de..ad4ff8f 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1018,7 +1018,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
> count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
> BUG_ON(count > sdb->table.nents);
> sdb->table.nents = count;
> - sdb->length = blk_rq_bytes(req);
> + sdb->length = blk_rq_payload_bytes(req);
> return BLKPREP_OK;
> }
>
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 2/4] scsi: use blk_rq_payload_bytes
@ 2017-01-13 11:43 ` Hannes Reinecke
0 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Without that we'll pass a wrong payload size in cmd->sdb, which
> can lead to hangs with drivers that need the total transfer size.
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> Reported-by: Chris Valean <v-chvale at microsoft.com>
> Reported-by: Dexuan Cui <decui at microsoft.com>
> Fixes: f9d03f96 ("block: improve handling of the magic discard payload")
> ---
> drivers/scsi/scsi_lib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index c35b6de..ad4ff8f 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1018,7 +1018,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
> count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
> BUG_ON(count > sdb->table.nents);
> sdb->table.nents = count;
> - sdb->length = blk_rq_bytes(req);
> + sdb->length = blk_rq_payload_bytes(req);
> return BLKPREP_OK;
> }
>
>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare at suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/4] nvme: use blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
(?)
@ 2017-01-13 11:43 ` Hannes Reinecke
-1 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> The new blk_rq_payload_bytes generalizes the payload length hacks
> that nvme_map_len did before.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/nvme/host/fc.c | 5 ++---
> drivers/nvme/host/nvme.h | 8 --------
> drivers/nvme/host/pci.c | 19 ++++++++-----------
> drivers/nvme/host/rdma.c | 13 +++++--------
> 4 files changed, 15 insertions(+), 30 deletions(-)
>
[ .. ]
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: F. Imend�rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N�rnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/4] nvme: use blk_rq_payload_bytes
@ 2017-01-13 11:43 ` Hannes Reinecke
0 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> The new blk_rq_payload_bytes generalizes the payload length hacks
> that nvme_map_len did before.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/nvme/host/fc.c | 5 ++---
> drivers/nvme/host/nvme.h | 8 --------
> drivers/nvme/host/pci.c | 19 ++++++++-----------
> drivers/nvme/host/rdma.c | 13 +++++--------
> 4 files changed, 15 insertions(+), 30 deletions(-)
>
[ .. ]
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 3/4] nvme: use blk_rq_payload_bytes
@ 2017-01-13 11:43 ` Hannes Reinecke
0 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:43 UTC (permalink / raw)
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> The new blk_rq_payload_bytes generalizes the payload length hacks
> that nvme_map_len did before.
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
> drivers/nvme/host/fc.c | 5 ++---
> drivers/nvme/host/nvme.h | 8 --------
> drivers/nvme/host/pci.c | 19 ++++++++-----------
> drivers/nvme/host/rdma.c | 13 +++++--------
> 4 files changed, 15 insertions(+), 30 deletions(-)
>
[ .. ]
Reviewed-by: Hannes Reinecke <hare at suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare at suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] sd: remove __data_len hack for WRITE SAME
2017-01-13 11:29 ` Christoph Hellwig
(?)
@ 2017-01-13 11:44 ` Hannes Reinecke
-1 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:44 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Now that we have the blk_rq_payload_bytes helper available to determine
> the actual I/O size we don't need to mess around with __data_len for
> WRITE SAME.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/scsi/sd.c | 17 +----------------
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b193304..1fbb1ec 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -836,7 +836,6 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
> struct bio *bio = rq->bio;
> sector_t sector = blk_rq_pos(rq);
> unsigned int nr_sectors = blk_rq_sectors(rq);
> - unsigned int nr_bytes = blk_rq_bytes(rq);
> int ret;
>
> if (sdkp->device->no_write_same)
> @@ -869,21 +868,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
>
> cmd->transfersize = sdp->sector_size;
> cmd->allowed = SD_MAX_RETRIES;
> -
> - /*
> - * For WRITE_SAME the data transferred in the DATA IN buffer is
> - * different from the amount of data actually written to the target.
> - *
> - * We set up __data_len to the amount of data transferred from the
> - * DATA IN buffer so that blk_rq_map_sg set up the proper S/G list
> - * to transfer a single sector of data first, but then reset it to
> - * the amount of data to be written right after so that the I/O path
> - * knows how much to actually write.
> - */
> - rq->__data_len = sdp->sector_size;
> - ret = scsi_init_io(cmd);
> - rq->__data_len = nr_bytes;
> - return ret;
> + return scsi_init_io(cmd);
> }
>
> static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: F. Imend�rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N�rnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] sd: remove __data_len hack for WRITE SAME
@ 2017-01-13 11:44 ` Hannes Reinecke
0 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:44 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Now that we have the blk_rq_payload_bytes helper available to determine
> the actual I/O size we don't need to mess around with __data_len for
> WRITE SAME.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/scsi/sd.c | 17 +----------------
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b193304..1fbb1ec 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -836,7 +836,6 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
> struct bio *bio = rq->bio;
> sector_t sector = blk_rq_pos(rq);
> unsigned int nr_sectors = blk_rq_sectors(rq);
> - unsigned int nr_bytes = blk_rq_bytes(rq);
> int ret;
>
> if (sdkp->device->no_write_same)
> @@ -869,21 +868,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
>
> cmd->transfersize = sdp->sector_size;
> cmd->allowed = SD_MAX_RETRIES;
> -
> - /*
> - * For WRITE_SAME the data transferred in the DATA IN buffer is
> - * different from the amount of data actually written to the target.
> - *
> - * We set up __data_len to the amount of data transferred from the
> - * DATA IN buffer so that blk_rq_map_sg set up the proper S/G list
> - * to transfer a single sector of data first, but then reset it to
> - * the amount of data to be written right after so that the I/O path
> - * knows how much to actually write.
> - */
> - rq->__data_len = sdp->sector_size;
> - ret = scsi_init_io(cmd);
> - rq->__data_len = nr_bytes;
> - return ret;
> + return scsi_init_io(cmd);
> }
>
> static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 4/4] sd: remove __data_len hack for WRITE SAME
@ 2017-01-13 11:44 ` Hannes Reinecke
0 siblings, 0 replies; 39+ messages in thread
From: Hannes Reinecke @ 2017-01-13 11:44 UTC (permalink / raw)
On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Now that we have the blk_rq_payload_bytes helper available to determine
> the actual I/O size we don't need to mess around with __data_len for
> WRITE SAME.
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
> drivers/scsi/sd.c | 17 +----------------
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b193304..1fbb1ec 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -836,7 +836,6 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
> struct bio *bio = rq->bio;
> sector_t sector = blk_rq_pos(rq);
> unsigned int nr_sectors = blk_rq_sectors(rq);
> - unsigned int nr_bytes = blk_rq_bytes(rq);
> int ret;
>
> if (sdkp->device->no_write_same)
> @@ -869,21 +868,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
>
> cmd->transfersize = sdp->sector_size;
> cmd->allowed = SD_MAX_RETRIES;
> -
> - /*
> - * For WRITE_SAME the data transferred in the DATA IN buffer is
> - * different from the amount of data actually written to the target.
> - *
> - * We set up __data_len to the amount of data transferred from the
> - * DATA IN buffer so that blk_rq_map_sg set up the proper S/G list
> - * to transfer a single sector of data first, but then reset it to
> - * the amount of data to be written right after so that the I/O path
> - * knows how much to actually write.
> - */
> - rq->__data_len = sdp->sector_size;
> - ret = scsi_init_io(cmd);
> - rq->__data_len = nr_bytes;
> - return ret;
> + return scsi_init_io(cmd);
> }
>
> static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare at suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] block: add blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 21:52 ` Sagi Grimberg
-1 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-13 21:52 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: linux-block, Dexuan Cui, linux-nvme, linux-scsi
> Add a helper to calculate the actual data transfer size for special
> payload requests.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> include/linux/blkdev.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ff3d774..1ca8e8f 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
> return blk_rq_cur_bytes(rq) >> 9;
> }
>
> +/*
> + * Some commands like WRITE SAME have a payload or data transfer size which
Don't you mean here DISCARD?
> + * is different from the size of the request. Any driver that supports such
> + * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
> + * calculate the data transfer size.
> + */
> +static inline unsigned int blk_rq_payload_bytes(struct request *rq)
> +{
> + if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
> + return rq->special_vec.bv_len;
> + return blk_rq_bytes(rq);
> +}
> +
> static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
> int op)
> {
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/4] block: add blk_rq_payload_bytes
@ 2017-01-13 21:52 ` Sagi Grimberg
0 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-13 21:52 UTC (permalink / raw)
> Add a helper to calculate the actual data transfer size for special
> payload requests.
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
> include/linux/blkdev.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ff3d774..1ca8e8f 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
> return blk_rq_cur_bytes(rq) >> 9;
> }
>
> +/*
> + * Some commands like WRITE SAME have a payload or data transfer size which
Don't you mean here DISCARD?
> + * is different from the size of the request. Any driver that supports such
> + * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
> + * calculate the data transfer size.
> + */
> +static inline unsigned int blk_rq_payload_bytes(struct request *rq)
> +{
> + if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
> + return rq->special_vec.bv_len;
> + return blk_rq_bytes(rq);
> +}
> +
> static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
> int op)
> {
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/4] scsi: use blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 21:52 ` Sagi Grimberg
-1 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-13 21:52 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: linux-block, Dexuan Cui, linux-nvme, linux-scsi
Looks good,
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 2/4] scsi: use blk_rq_payload_bytes
@ 2017-01-13 21:52 ` Sagi Grimberg
0 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-13 21:52 UTC (permalink / raw)
Looks good,
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/4] nvme: use blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 21:54 ` Sagi Grimberg
-1 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-13 21:54 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: linux-block, Dexuan Cui, linux-nvme, linux-scsi
This looks good,
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 3/4] nvme: use blk_rq_payload_bytes
@ 2017-01-13 21:54 ` Sagi Grimberg
0 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-13 21:54 UTC (permalink / raw)
This looks good,
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] sd: remove __data_len hack for WRITE SAME
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 21:56 ` Sagi Grimberg
-1 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-13 21:56 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: linux-block, Dexuan Cui, linux-nvme, linux-scsi
Looks good,
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 4/4] sd: remove __data_len hack for WRITE SAME
@ 2017-01-13 21:56 ` Sagi Grimberg
0 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-13 21:56 UTC (permalink / raw)
Looks good,
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: fix data transfer size caculation for special payload requests
2017-01-13 11:29 ` Christoph Hellwig
@ 2017-01-13 22:17 ` Jens Axboe
-1 siblings, 0 replies; 39+ messages in thread
From: Jens Axboe @ 2017-01-13 22:17 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Dexuan Cui, linux-block, linux-scsi, linux-nvme
On Fri, Jan 13 2017, Christoph Hellwig wrote:
> Hi all,
>
> the first two fixes fix a regression in 4.10-rc1 where the data transfer
> size for discard commands wasn't set properly, leading to hangs with
> Hyper-V VMs. The use the new data transfer size helper added in patch 1
> more widely.
Added for this series, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 39+ messages in thread
* fix data transfer size caculation for special payload requests
@ 2017-01-13 22:17 ` Jens Axboe
0 siblings, 0 replies; 39+ messages in thread
From: Jens Axboe @ 2017-01-13 22:17 UTC (permalink / raw)
On Fri, Jan 13 2017, Christoph Hellwig wrote:
> Hi all,
>
> the first two fixes fix a regression in 4.10-rc1 where the data transfer
> size for discard commands wasn't set properly, leading to hangs with
> Hyper-V VMs. The use the new data transfer size helper added in patch 1
> more widely.
Added for this series, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/4] nvme: use blk_rq_payload_bytes
2017-01-13 11:29 ` Christoph Hellwig
(?)
@ 2017-01-17 20:06 ` Sagi Grimberg
-1 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-17 20:06 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: linux-block, Dexuan Cui, linux-scsi, linux-nvme
> @@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
> }
>
Christoph, a little above here we still look at blk_rq_bytes(),
shouldn't that look at blk_rq_payload_bytes() too?
> if (count == 1) {
> - if (rq_data_dir(rq) == WRITE &&
> - map_len <= nvme_rdma_inline_data_size(queue) &&
> - nvme_rdma_queue_idx(queue))
> + if (rq_data_dir(rq) == WRITE && nvme_rdma_queue_idx(queue) &&
> + blk_rq_payload_bytes(rq) <=
> + nvme_rdma_inline_data_size(queue))
> return nvme_rdma_map_sg_inline(queue, req, c);
>
> if (dev->pd->flags & IB_PD_UNSAFE_GLOBAL_RKEY)
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/4] nvme: use blk_rq_payload_bytes
@ 2017-01-17 20:06 ` Sagi Grimberg
0 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-17 20:06 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: linux-block, Dexuan Cui, linux-nvme, linux-scsi
> @@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
> }
>
Christoph, a little above here we still look at blk_rq_bytes(),
shouldn't that look at blk_rq_payload_bytes() too?
> if (count == 1) {
> - if (rq_data_dir(rq) == WRITE &&
> - map_len <= nvme_rdma_inline_data_size(queue) &&
> - nvme_rdma_queue_idx(queue))
> + if (rq_data_dir(rq) == WRITE && nvme_rdma_queue_idx(queue) &&
> + blk_rq_payload_bytes(rq) <=
> + nvme_rdma_inline_data_size(queue))
> return nvme_rdma_map_sg_inline(queue, req, c);
>
> if (dev->pd->flags & IB_PD_UNSAFE_GLOBAL_RKEY)
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 3/4] nvme: use blk_rq_payload_bytes
@ 2017-01-17 20:06 ` Sagi Grimberg
0 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-17 20:06 UTC (permalink / raw)
> @@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
> }
>
Christoph, a little above here we still look at blk_rq_bytes(),
shouldn't that look at blk_rq_payload_bytes() too?
> if (count == 1) {
> - if (rq_data_dir(rq) == WRITE &&
> - map_len <= nvme_rdma_inline_data_size(queue) &&
> - nvme_rdma_queue_idx(queue))
> + if (rq_data_dir(rq) == WRITE && nvme_rdma_queue_idx(queue) &&
> + blk_rq_payload_bytes(rq) <=
> + nvme_rdma_inline_data_size(queue))
> return nvme_rdma_map_sg_inline(queue, req, c);
>
> if (dev->pd->flags & IB_PD_UNSAFE_GLOBAL_RKEY)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/4] nvme: use blk_rq_payload_bytes
2017-01-17 20:06 ` Sagi Grimberg
@ 2017-01-18 9:09 ` Christoph Hellwig
-1 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-18 9:09 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Christoph Hellwig, Jens Axboe, linux-block, Dexuan Cui,
linux-nvme, linux-scsi
On Tue, Jan 17, 2017 at 10:06:51PM +0200, Sagi Grimberg wrote:
>
>> @@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
>> }
>>
>
> Christoph, a little above here we still look at blk_rq_bytes(),
> shouldn't that look at blk_rq_payload_bytes() too?
The check is ok for now as it's just zero vs non-zero. It's somewhat
broken for Write Zeroes, though. I've fixed it in this series
which I need to submit:
http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/write-zeroes-take-2
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 3/4] nvme: use blk_rq_payload_bytes
@ 2017-01-18 9:09 ` Christoph Hellwig
0 siblings, 0 replies; 39+ messages in thread
From: Christoph Hellwig @ 2017-01-18 9:09 UTC (permalink / raw)
On Tue, Jan 17, 2017@10:06:51PM +0200, Sagi Grimberg wrote:
>
>> @@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
>> }
>>
>
> Christoph, a little above here we still look at blk_rq_bytes(),
> shouldn't that look at blk_rq_payload_bytes() too?
The check is ok for now as it's just zero vs non-zero. It's somewhat
broken for Write Zeroes, though. I've fixed it in this series
which I need to submit:
http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/write-zeroes-take-2
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/4] nvme: use blk_rq_payload_bytes
2017-01-18 9:09 ` Christoph Hellwig
@ 2017-01-19 7:54 ` Sagi Grimberg
-1 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-19 7:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-block, Dexuan Cui, linux-nvme, linux-scsi
>>> @@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
>>> }
>>>
>>
>> Christoph, a little above here we still look at blk_rq_bytes(),
>> shouldn't that look at blk_rq_payload_bytes() too?
>
> The check is ok for now as it's just zero vs non-zero. It's somewhat
> broken for Write Zeroes, though. I've fixed it in this series
> which I need to submit:
>
> http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/write-zeroes-take-2
>
I see, thanks for the clarification.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 3/4] nvme: use blk_rq_payload_bytes
@ 2017-01-19 7:54 ` Sagi Grimberg
0 siblings, 0 replies; 39+ messages in thread
From: Sagi Grimberg @ 2017-01-19 7:54 UTC (permalink / raw)
>>> @@ -1014,9 +1013,9 @@ static int nvme_rdma_map_data(struct nvme_rdma_queue *queue,
>>> }
>>>
>>
>> Christoph, a little above here we still look at blk_rq_bytes(),
>> shouldn't that look at blk_rq_payload_bytes() too?
>
> The check is ok for now as it's just zero vs non-zero. It's somewhat
> broken for Write Zeroes, though. I've fixed it in this series
> which I need to submit:
>
> http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/write-zeroes-take-2
>
I see, thanks for the clarification.
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2017-01-19 7:54 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-13 11:29 fix data transfer size caculation for special payload requests Christoph Hellwig
2017-01-13 11:29 ` Christoph Hellwig
2017-01-13 11:29 ` [PATCH 1/4] block: add blk_rq_payload_bytes Christoph Hellwig
2017-01-13 11:29 ` Christoph Hellwig
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 21:52 ` Sagi Grimberg
2017-01-13 21:52 ` Sagi Grimberg
2017-01-13 11:29 ` [PATCH 2/4] scsi: use blk_rq_payload_bytes Christoph Hellwig
2017-01-13 11:29 ` Christoph Hellwig
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 21:52 ` Sagi Grimberg
2017-01-13 21:52 ` Sagi Grimberg
2017-01-13 11:29 ` [PATCH 3/4] nvme: " Christoph Hellwig
2017-01-13 11:29 ` Christoph Hellwig
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 11:43 ` Hannes Reinecke
2017-01-13 21:54 ` Sagi Grimberg
2017-01-13 21:54 ` Sagi Grimberg
2017-01-17 20:06 ` Sagi Grimberg
2017-01-17 20:06 ` Sagi Grimberg
2017-01-17 20:06 ` Sagi Grimberg
2017-01-18 9:09 ` Christoph Hellwig
2017-01-18 9:09 ` Christoph Hellwig
2017-01-19 7:54 ` Sagi Grimberg
2017-01-19 7:54 ` Sagi Grimberg
2017-01-13 11:29 ` [PATCH 4/4] sd: remove __data_len hack for WRITE SAME Christoph Hellwig
2017-01-13 11:29 ` Christoph Hellwig
2017-01-13 11:44 ` Hannes Reinecke
2017-01-13 11:44 ` Hannes Reinecke
2017-01-13 11:44 ` Hannes Reinecke
2017-01-13 21:56 ` Sagi Grimberg
2017-01-13 21:56 ` Sagi Grimberg
2017-01-13 22:17 ` fix data transfer size caculation for special payload requests Jens Axboe
2017-01-13 22:17 ` Jens Axboe
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.