[PATCHv2 1/2] nvme-rdma: move nvme cm status helper to .h file

[PATCHv2 1/2] nvme-rdma: move nvme cm status helper to .h file

[Max Gurtovoy]

This will enable the usage for nvme rdma target.
Also move from a lookup array to a switch statement.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
Reviewed-by: Parav Pandit <parav at mellanox.com>
---

Changes from v1:
 - updated the code in the switch label to be in a new line
 - updated the changelog

---
 drivers/nvme/host/rdma.c  |   22 ----------------------
 include/linux/nvme-rdma.h |   24 ++++++++++++++++++++++++
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 96285ef..b3e1071 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -42,28 +42,6 @@
 
 #define NVME_RDMA_MAX_INLINE_SEGMENTS	1
 
-static const char *const nvme_rdma_cm_status_strs[] = {
-	[NVME_RDMA_CM_INVALID_LEN]	= "invalid length",
-	[NVME_RDMA_CM_INVALID_RECFMT]	= "invalid record format",
-	[NVME_RDMA_CM_INVALID_QID]	= "invalid queue ID",
-	[NVME_RDMA_CM_INVALID_HSQSIZE]	= "invalid host SQ size",
-	[NVME_RDMA_CM_INVALID_HRQSIZE]	= "invalid host RQ size",
-	[NVME_RDMA_CM_NO_RSC]		= "resource not found",
-	[NVME_RDMA_CM_INVALID_IRD]	= "invalid IRD",
-	[NVME_RDMA_CM_INVALID_ORD]	= "Invalid ORD",
-};
-
-static const char *nvme_rdma_cm_msg(enum nvme_rdma_cm_status status)
-{
-	size_t index = status;
-
-	if (index < ARRAY_SIZE(nvme_rdma_cm_status_strs) &&
-	    nvme_rdma_cm_status_strs[index])
-		return nvme_rdma_cm_status_strs[index];
-	else
-		return "unrecognized reason";
-};
-
 /*
  * We handle AEN commands ourselves and don't even let the
  * block layer know about them.
diff --git a/include/linux/nvme-rdma.h b/include/linux/nvme-rdma.h
index bf240a3..a72fd04 100644
--- a/include/linux/nvme-rdma.h
+++ b/include/linux/nvme-rdma.h
@@ -29,6 +29,30 @@ enum nvme_rdma_cm_status {
 	NVME_RDMA_CM_INVALID_ORD	= 0x08,
 };
 
+static inline const char *nvme_rdma_cm_msg(enum nvme_rdma_cm_status status)
+{
+	switch (status) {
+	case NVME_RDMA_CM_INVALID_LEN:
+		return "invalid length";
+	case NVME_RDMA_CM_INVALID_RECFMT:
+		return "invalid record format";
+	case NVME_RDMA_CM_INVALID_QID:
+		return "invalid queue ID";
+	case NVME_RDMA_CM_INVALID_HSQSIZE:
+		return "invalid host SQ size";
+	case NVME_RDMA_CM_INVALID_HRQSIZE:
+		return "invalid host RQ size";
+	case NVME_RDMA_CM_NO_RSC:
+		return "resource not found";
+	case NVME_RDMA_CM_INVALID_IRD:
+		return "invalid IRD";
+	case NVME_RDMA_CM_INVALID_ORD:
+		return "Invalid ORD";
+	default:
+		return "unrecognized reason";
+	}
+}
+
 /**
  * struct nvme_rdma_cm_req - rdma connect request
  *
-- 
1.7.1

[PATCHv2 2/2] nvmet-rdma: use nvme cm status helper

[Max Gurtovoy]

Also remove redundant debug prints.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
Reviewed-by: Parav Pandit <parav at mellanox.com>
Reviewed-by: Christoph Hellwig <hch at lst.de>
---

Changes from v1:
 - Add Christoph as reviewer

---
 drivers/nvme/target/rdma.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 6099022..5d189c0 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1041,6 +1041,9 @@ static int nvmet_rdma_cm_reject(struct rdma_cm_id *cm_id,
 {
 	struct nvme_rdma_cm_rej rej;
 
+	pr_debug("rejecting connect request: status %d (%s)\n",
+		 status, nvme_rdma_cm_msg(status));
+
 	rej.recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0);
 	rej.sts = cpu_to_le16(status);
 
@@ -1135,7 +1138,6 @@ static int nvmet_rdma_cm_reject(struct rdma_cm_id *cm_id,
 out_free_queue:
 	kfree(queue);
 out_reject:
-	pr_debug("rejecting connect request with status code %d\n", ret);
 	nvmet_rdma_cm_reject(cm_id, ret);
 	return NULL;
 }
@@ -1188,7 +1190,6 @@ static int nvmet_rdma_queue_connect(struct rdma_cm_id *cm_id,
 
 	ndev = nvmet_rdma_find_get_device(cm_id);
 	if (!ndev) {
-		pr_err("no client data!\n");
 		nvmet_rdma_cm_reject(cm_id, NVME_RDMA_CM_NO_RSC);
 		return -ECONNREFUSED;
 	}
-- 
1.7.1

[PATCHv2 1/2] nvme-rdma: move nvme cm status helper to .h file

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch at lst.de>

[PATCHv2 1/2] nvme-rdma: move nvme cm status helper to .h file

[Bart Van Assche]

On 02/20/2017 03:44 AM, Max Gurtovoy wrote:
> This will enable the usage for nvme rdma target.
> Also move from a lookup array to a switch statement.

Hello Max,

In general un-inlining code that is not in the hot path is wrong because
this increases the size of the kernel and because this increases the
time needed to build the kernel. Would it have been possible to keep the
nvme_rdma_cm_msg() definition in a .c file?

Thanks,

Bart.

[PATCHv2 1/2] nvme-rdma: move nvme cm status helper to .h file

[Max Gurtovoy]

Hi Bart,

Currently we don't have any .c file to put this function implementation 
without creating a code duplication and module dependencies.
I think that this new solution is better than code duplication, right ?

thanks,
Max.


On 2/20/2017 7:03 PM, Bart Van Assche wrote:
> On 02/20/2017 03:44 AM, Max Gurtovoy wrote:
>> This will enable the usage for nvme rdma target.
>> Also move from a lookup array to a switch statement.
>
> Hello Max,
>
> In general un-inlining code that is not in the hot path is wrong because
> this increases the size of the kernel and because this increases the
> time needed to build the kernel. Would it have been possible to keep the
> nvme_rdma_cm_msg() definition in a .c file?
>
> Thanks,
>
> Bart.
>

[PATCHv2 1/2] nvme-rdma: move nvme cm status helper to .h file

[hch]

On Tue, Feb 21, 2017@11:24:24AM +0200, Max Gurtovoy wrote:
> Hi Bart,
> 
> Currently we don't have any .c file to put this function implementation
> without creating a code duplication and module dependencies.
> I think that this new solution is better than code duplication, right ?

Yes, it is - especially given that it's just a bit of constant and not
much code anyway, and each new module would take up at least a page of
memory.

[PATCHv2 1/2] nvme-rdma: move nvme cm status helper to .h file

[Max Gurtovoy]

On 2/21/2017 11:27 AM, hch@infradead.org wrote:
> On Tue, Feb 21, 2017@11:24:24AM +0200, Max Gurtovoy wrote:
>> Hi Bart,
>>
>> Currently we don't have any .c file to put this function implementation
>> without creating a code duplication and module dependencies.
>> I think that this new solution is better than code duplication, right ?
>
> Yes, it is - especially given that it's just a bit of constant and not
> much code anyway, and each new module would take up at least a page of
> memory.
>

Maybe we'll need it in the future, but for now let's take it as is.

Sagi,
can you take it to nvme-4.11 ?

Max.

[PATCHv2 1/2] nvme-rdma: move nvme cm status helper to .h file

[Sagi Grimberg]

> Sagi,
> can you take it to nvme-4.11 ?

Yea, queued, thanks.