Skip to contentSkip to conversation

NVMe: Fix obtaining command result

PATCH New Keith Busch 3 participants 4 messages May 22, 2015 – May 26, 2015

Subsystems (2)
Block layer NVMe
lore
Share & tools
lore

Keith Busch

author
#0 lore raw

PATCH 1/1 NVMe: Fix obtaining command result

Replaces req->sense_len usage, which is not owned by the LLD, to req->special to contain the command result for driver created commands, and sets the result unconditionally on completion.

Signed-off-by Keith Busch <keith.busch at intel.com>
Cc Christoph Hellwig <hch at lst.de>
Cc Jens Axboe <axboe at fb.com>
---
 drivers/block/nvme-core.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/block/nvme-core.c +6 -4
index 03bd638..c42bc53 100644
@@ -606,13 +606,16 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx,
 			return;
 		}
 		if (req->cmd_type == REQ_TYPE_DRV_PRIV) {
-			req->sense_len = le32_to_cpup(&cqe->result);
 			req->errors = status;
 		} else {
 			req->errors = nvme_error_status(status);
 		}
 	} else
 		req->errors = 0;
+	if (req->cmd_type == REQ_TYPE_DRV_PRIV) {
+		u32 result = le32_to_cpup(&cqe->result);
+		req->special = (void *)(uintptr_t)result;
+	}
 
 	if (cmd_rq->aborted)
 		dev_warn(nvmeq->dev->dev,

@@ -1015,8 +1018,7 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 
 	req->cmd = (unsigned char *)cmd;
 	req->cmd_len = sizeof(struct nvme_command);
-	req->sense = NULL;
-	req->sense_len = 0;
+	req->special = (void *)0;
 
 	if (buffer && bufflen) {
 		ret = blk_rq_map_kern(q, req, buffer, bufflen, __GFP_WAIT);

@@ -1033,7 +1035,7 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 	if (bio)
 		blk_rq_unmap_user(bio);
 	if (result)
-		*result = req->sense_len;
+		*result = (u32)(uintptr_t)req->special;
 	ret = req->errors;
  out:
 	blk_mq_free_request(req);
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/1 NVMe: Fix obtaining command result

Quoted 3 lines · Keith Busch

Replaces req->sense_len usage, which is not owned by the LLD, to req->special to contain the command result for driver created commands, and sets the result unconditionally on completion.

Thanks Keith, added. I added a Fixes tag too.

Signature · 1 line
Jens Axboe
maintainer
#2 lore raw

PATCH 1/1 NVMe: Fix obtaining command result

In reply to Keith Busch
Quoted 3 lines · Keith Busch

Replaces req->sense_len usage, which is not owned by the LLD, to req->special to contain the command result for driver created commands, and sets the result unconditionally on completion.

Thanks Keith,

sorry for the mess, after porting this from my series to use replace the BLOCK_PC specific fields to mainline I only tested this on Qemu which only has a single queue. I'll make sure to always do final testing on real hardware as well.

author
#3 lore raw

PATCH 1/1 NVMe: Fix obtaining command result

Quoted 11 lines · Christoph Hellwig
Quoted 3 lines · Keith Busch

Replaces req->sense_len usage, which is not owned by the LLD, to req->special to contain the command result for driver created commands, and sets the result unconditionally on completion.

Thanks Keith,

sorry for the mess, after porting this from my series to use replace the BLOCK_PC specific fields to mainline I only tested this on Qemu which only has a single queue. I'll make sure to always do final testing on real hardware as well.

No problem, it's a good enhancement overall and caught early enough, so no a big deal. Break and fix cycles are better than doing nothing. :)

BTW, the qemu nvme block device *should* be multi-queue capable if you allocate multiple CPUs (-smp <cpus>) to your guest. It works in my tree at least, so I hope it works in qemu mainline. Maybe I'll have time this week to look at that.