[PATCH] nvme: use dev_err() to make message more useful

[PATCH] nvme: use dev_err() to make message more useful

[Baruch Siach]

pr_err() produces no prefix when called from pci.c, and only generic
prefix on other call sites. Switch to use dev_err() for more pleasant
debugging experience.

Suggested-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20241212062241.GA5586@lst.de
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/nvme/host/apple.c  | 2 +-
 drivers/nvme/host/nvme.h   | 6 +++---
 drivers/nvme/host/pci.c    | 2 +-
 drivers/nvme/host/rdma.c   | 2 +-
 drivers/nvme/host/tcp.c    | 6 +++---
 drivers/nvme/target/loop.c | 3 ++-
 6 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index 4319ab50c10d..a70a167ab8f4 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -592,7 +592,7 @@ static inline void apple_nvme_handle_cqe(struct apple_nvme_queue *q,
 
 	apple_nvmmu_inval(q, command_id);
 
-	req = nvme_find_rq(apple_nvme_queue_tagset(anv, q), command_id);
+	req = nvme_find_rq(&anv->ctrl, apple_nvme_queue_tagset(anv, q), command_id);
 	if (unlikely(!req)) {
 		dev_warn(anv->dev, "invalid id %d completed", command_id);
 		return;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 611b02c8a8b3..dbdf8bc45f0e 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -583,8 +583,8 @@ static inline u16 nvme_cid(struct request *rq)
 	return nvme_cid_install_genctr(nvme_req(rq)->genctr) | rq->tag;
 }
 
-static inline struct request *nvme_find_rq(struct blk_mq_tags *tags,
-		u16 command_id)
+static inline struct request *nvme_find_rq(struct nvme_ctrl *ctrl,
+		struct blk_mq_tags *tags, u16 command_id)
 {
 	u8 genctr = nvme_genctr_from_cid(command_id);
 	u16 tag = nvme_tag_from_cid(command_id);
@@ -592,7 +592,7 @@ static inline struct request *nvme_find_rq(struct blk_mq_tags *tags,
 
 	rq = blk_mq_tag_to_rq(tags, tag);
 	if (unlikely(!rq)) {
-		pr_err("could not locate request for tag %#x\n",
+		dev_err(ctrl->device, "could not locate request for tag %#x\n",
 			tag);
 		return NULL;
 	}
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 049ec18c037a..43fc557a85ab 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1122,7 +1122,7 @@ static inline void nvme_handle_cqe(struct nvme_queue *nvmeq,
 		return;
 	}
 
-	req = nvme_find_rq(nvme_queue_tagset(nvmeq), command_id);
+	req = nvme_find_rq(&nvmeq->dev->ctrl, nvme_queue_tagset(nvmeq), command_id);
 	if (unlikely(!req)) {
 		dev_warn(nvmeq->dev->ctrl.device,
 			"invalid id %d completed on queue %d\n",
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 86a2891d9bcc..d991ff7860b3 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1694,7 +1694,7 @@ static void nvme_rdma_process_nvme_rsp(struct nvme_rdma_queue *queue,
 	struct request *rq;
 	struct nvme_rdma_request *req;
 
-	rq = nvme_find_rq(nvme_rdma_tagset(queue), cqe->command_id);
+	rq = nvme_find_rq(&queue->ctrl->ctrl, nvme_rdma_tagset(queue), cqe->command_id);
 	if (!rq) {
 		dev_err(queue->ctrl->ctrl.device,
 			"got bad command_id %#x on QP %#x\n",
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 28c76a3e1bd2..6e9c03ceeee0 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -601,7 +601,7 @@ static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,
 	struct nvme_tcp_request *req;
 	struct request *rq;
 
-	rq = nvme_find_rq(nvme_tcp_tagset(queue), cqe->command_id);
+	rq = nvme_find_rq(&queue->ctrl->ctrl, nvme_tcp_tagset(queue), cqe->command_id);
 	if (!rq) {
 		dev_err(queue->ctrl->ctrl.device,
 			"got bad cqe.command_id %#x on queue %d\n",
@@ -626,7 +626,7 @@ static int nvme_tcp_handle_c2h_data(struct nvme_tcp_queue *queue,
 {
 	struct request *rq;
 
-	rq = nvme_find_rq(nvme_tcp_tagset(queue), pdu->command_id);
+	rq = nvme_find_rq(&queue->ctrl->ctrl, nvme_tcp_tagset(queue), pdu->command_id);
 	if (!rq) {
 		dev_err(queue->ctrl->ctrl.device,
 			"got bad c2hdata.command_id %#x on queue %d\n",
@@ -719,7 +719,7 @@ static int nvme_tcp_handle_r2t(struct nvme_tcp_queue *queue,
 	u32 r2t_length = le32_to_cpu(pdu->r2t_length);
 	u32 r2t_offset = le32_to_cpu(pdu->r2t_offset);
 
-	rq = nvme_find_rq(nvme_tcp_tagset(queue), pdu->command_id);
+	rq = nvme_find_rq(&queue->ctrl->ctrl, nvme_tcp_tagset(queue), pdu->command_id);
 	if (!rq) {
 		dev_err(queue->ctrl->ctrl.device,
 			"got bad r2t.command_id %#x on queue %d\n",
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index a9d112d34d4f..c6dfb5b796ec 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -107,7 +107,8 @@ static void nvme_loop_queue_response(struct nvmet_req *req)
 	} else {
 		struct request *rq;
 
-		rq = nvme_find_rq(nvme_loop_tagset(queue), cqe->command_id);
+		rq = nvme_find_rq(&queue->ctrl->ctrl, nvme_loop_tagset(queue),
+				cqe->command_id);
 		if (!rq) {
 			dev_err(queue->ctrl->ctrl.device,
 				"got bad command_id %#x on queue %d\n",
-- 
2.45.2

Re: [PATCH] nvme: use dev_err() to make message more useful

[Christoph Hellwig]

On Thu, Dec 12, 2024 at 09:20:12AM +0200, Baruch Siach wrote:
> +	req = nvme_find_rq(&anv->ctrl, apple_nvme_queue_tagset(anv, q), command_id);

Please avoid the overly long lines.  Note that the nvme_ctrl has pointers
to the admin and io tagset, so you can actuall simply the second paramter
here by just passing a 'bool admin_queue' and remove the
apple_nvme_queue_tagset, nvme_queue_tagset, nvme_rdma_tagset and
nvme_loop_tagset helpers entirely.  The TCP one still has other users
by might be worth looking into as a follow up.

Re: [PATCH] nvme: use dev_err() to make message more useful

[Baruch Siach]

Hi Christoph,

On Thu, Dec 12 2024, Christoph Hellwig wrote:
> On Thu, Dec 12, 2024 at 09:20:12AM +0200, Baruch Siach wrote:
>> +	req = nvme_find_rq(&anv->ctrl, apple_nvme_queue_tagset(anv, q), command_id);
>
> Please avoid the overly long lines.  Note that the nvme_ctrl has pointers
> to the admin and io tagset, so you can actuall simply the second paramter
> here by just passing a 'bool admin_queue' and remove the
> apple_nvme_queue_tagset, nvme_queue_tagset, nvme_rdma_tagset and
> nvme_loop_tagset helpers entirely.  The TCP one still has other users
> by might be worth looking into as a follow up.

Passing 'bool admin_queue' is not enough. You also need qid to select
blk_mq_tags in the tagset.tags[] array.

Is it worth it?

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -