Skip to contentSkip to conversation

nvme-core: change commas to semicolons in nvme_get_log_page

PATCH New Sagi Grimberg 5 participants 10 messages Jun 18, 2017 – Jun 19, 2017

Subsystems (3)
NVMe NVMe target NVMe FC transport
lore
Share & tools
lore

Sagi Grimberg

author maintainer
#0 lore raw

PATCH v2 1/2 nvme-core: change commas to semicolons in nvme_get_log_page

Signed-off-by Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/nvme/host/core.c +3 -3
index 4ff5114f467d..17a10549d688 100644
@@ -815,11 +815,11 @@ int nvme_get_log_page(struct nvme_ctrl *dev, struct nvme_smart_log **log)
 	struct nvme_command c = { };
 	int error;
 
-	c.common.opcode = nvme_admin_get_log_page,
-	c.common.nsid = cpu_to_le32(0xFFFFFFFF),
+	c.common.opcode = nvme_admin_get_log_page;
+	c.common.nsid = cpu_to_le32(0xFFFFFFFF);
 	c.common.cdw10[0] = cpu_to_le32(
 			(((sizeof(struct nvme_smart_log) / 4) - 1) << 16) |
-			 NVME_LOG_SMART),
+			 NVME_LOG_SMART);
 
 	*log = kmalloc(sizeof(struct nvme_smart_log), GFP_KERNEL);
 	if (!*log)
Signature · 1 line
2.7.4
author maintainer
#1 lore raw

PATCH v2 2/2 nvme: use a single NVME_AQ_DEPTH and relax it to 32

No need to differentiate fabrics from pci/loop, also lower it to 32 as we don't really need 256 inflight admin commands.

Signed-off-by Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/fabrics.c     |  8 +-------
 drivers/nvme/host/fc.c          |  2 +-
 drivers/nvme/host/pci.c         |  1 -
 drivers/nvme/host/rdma.c        | 10 +++++-----
 drivers/nvme/target/discovery.c |  2 +-
 drivers/nvme/target/loop.c      |  4 +---
 drivers/nvme/target/rdma.c      |  2 +-
 include/linux/nvme.h            |  2 +-
 8 files changed, 11 insertions(+), 20 deletions(-)
drivers/nvme/host/fabrics.c +1 -7
index 6e6864516ce6..64db2c46c5ea 100644
@@ -394,13 +394,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 	cmd.connect.opcode = nvme_fabrics_command;
 	cmd.connect.fctype = nvme_fabrics_type_connect;
 	cmd.connect.qid = 0;
-
-	/*
-	 * fabrics spec sets a minimum of depth 32 for admin queue,
-	 * so set the queue with this depth always until
-	 * justification otherwise.
-	 */
-	cmd.connect.sqsize = cpu_to_le16(NVMF_AQ_DEPTH - 1);
+	cmd.connect.sqsize = cpu_to_le16(NVME_AQ_DEPTH - 1);
 
 	/*
 	 * Set keep-alive timeout in seconds granularity (ms * 1000)
drivers/nvme/host/fc.c +1 -1
index 5165007e86a6..5d5ecefd8dbe 100644
@@ -36,7 +36,7 @@
  */
 #define NVME_FC_NR_AEN_COMMANDS	1
 #define NVME_FC_AQ_BLKMQ_DEPTH	\
-	(NVMF_AQ_DEPTH - NVME_FC_NR_AEN_COMMANDS)
+	(NVME_AQ_DEPTH - NVME_FC_NR_AEN_COMMANDS)
 #define AEN_CMDID_BASE		(NVME_FC_AQ_BLKMQ_DEPTH + 1)
 
 enum nvme_fc_queue_flags {
drivers/nvme/host/pci.c +0 -1
index 2a9ee769ce9e..32a98e2740ad 100644
@@ -36,7 +36,6 @@
 #include "nvme.h"
 
 #define NVME_Q_DEPTH		1024
-#define NVME_AQ_DEPTH		256
 #define SQ_SIZE(depth)		(depth * sizeof(struct nvme_command))
 #define CQ_SIZE(depth)		(depth * sizeof(struct nvme_completion))
drivers/nvme/host/rdma.c +5 -5
index 066123a452c3..7533138d2244 100644
@@ -49,7 +49,7 @@
  */
 #define NVME_RDMA_NR_AEN_COMMANDS      1
 #define NVME_RDMA_AQ_BLKMQ_DEPTH       \
-	(NVMF_AQ_DEPTH - NVME_RDMA_NR_AEN_COMMANDS)
+	(NVME_AQ_DEPTH - NVME_RDMA_NR_AEN_COMMANDS)
 
 struct nvme_rdma_device {
 	struct ib_device       *dev;

@@ -726,7 +726,7 @@ static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work)
 	if (ret)
 		goto requeue;
 
-	ret = nvme_rdma_init_queue(ctrl, 0, NVMF_AQ_DEPTH);
+	ret = nvme_rdma_init_queue(ctrl, 0, NVME_AQ_DEPTH);
 	if (ret)
 		goto requeue;
 

@@ -1298,8 +1298,8 @@ static int nvme_rdma_route_resolved(struct nvme_rdma_queue *queue)
 	 * specified by the Fabrics standard.
 	 */
 	if (priv.qid == 0) {
-		priv.hrqsize = cpu_to_le16(NVMF_AQ_DEPTH);
-		priv.hsqsize = cpu_to_le16(NVMF_AQ_DEPTH - 1);
+		priv.hrqsize = cpu_to_le16(NVME_AQ_DEPTH);
+		priv.hsqsize = cpu_to_le16(NVME_AQ_DEPTH - 1);
 	} else {
 		/*
 		 * current interpretation of the fabrics spec

@@ -1545,7 +1545,7 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl)
 {
 	int error;
 
-	error = nvme_rdma_init_queue(ctrl, 0, NVMF_AQ_DEPTH);
+	error = nvme_rdma_init_queue(ctrl, 0, NVME_AQ_DEPTH);
 	if (error)
 		return error;
drivers/nvme/target/discovery.c +1 -1
index c7a90384dd75..8f3b57b4c97b 100644
@@ -53,7 +53,7 @@ static void nvmet_format_discovery_entry(struct nvmf_disc_rsp_page_hdr *hdr,
 	e->portid = port->disc_addr.portid;
 	/* we support only dynamic controllers */
 	e->cntlid = cpu_to_le16(NVME_CNTLID_DYNAMIC);
-	e->asqsz = cpu_to_le16(NVMF_AQ_DEPTH);
+	e->asqsz = cpu_to_le16(NVME_AQ_DEPTH);
 	e->subtype = type;
 	memcpy(e->trsvcid, port->disc_addr.trsvcid, NVMF_TRSVCID_SIZE);
 	memcpy(e->traddr, port->disc_addr.traddr, NVMF_TRADDR_SIZE);
drivers/nvme/target/loop.c +1 -3
index f67606523724..86c09e2a1490 100644
@@ -21,8 +21,6 @@
 #include "../host/nvme.h"
 #include "../host/fabrics.h"
 
-#define NVME_LOOP_AQ_DEPTH		256
-
 #define NVME_LOOP_MAX_SEGMENTS		256
 
 /*

@@ -31,7 +29,7 @@
  */
 #define NVME_LOOP_NR_AEN_COMMANDS	1
 #define NVME_LOOP_AQ_BLKMQ_DEPTH	\
-	(NVME_LOOP_AQ_DEPTH - NVME_LOOP_NR_AEN_COMMANDS)
+	(NVME_AQ_DEPTH - NVME_LOOP_NR_AEN_COMMANDS)
 
 struct nvme_loop_iod {
 	struct nvme_request	nvme_req;
drivers/nvme/target/rdma.c +1 -1
index 9e45cde63376..32aa10b521c8 100644
@@ -1027,7 +1027,7 @@ nvmet_rdma_parse_cm_connect_req(struct rdma_conn_param *conn,
 	queue->recv_queue_size = le16_to_cpu(req->hsqsize) + 1;
 	queue->send_queue_size = le16_to_cpu(req->hrqsize);
 
-	if (!queue->host_qid && queue->recv_queue_size > NVMF_AQ_DEPTH)
+	if (!queue->host_qid && queue->recv_queue_size > NVME_AQ_DEPTH)
 		return NVME_RDMA_CM_INVALID_HSQSIZE;
 
 	/* XXX: Should we enforce some kind of max for IO queues? */
include/linux/nvme.h +1 -1
index 6d476f242ee6..22232a36f01d 100644
@@ -87,7 +87,7 @@ enum {
 	NVMF_RDMA_CMS_RDMA_CM	= 1, /* Sockets based endpoint addressing */
 };
 
-#define NVMF_AQ_DEPTH		32
+#define NVME_AQ_DEPTH		32
 
 enum {
 	NVME_REG_CAP	= 0x0000,	/* Controller Capabilities */
Signature · 1 line
2.7.4
#2 lore raw

PATCH v2 2/2 nvme: use a single NVME_AQ_DEPTH and relax it to 32

Looks good,

Reviewed-by Max Gurtovoy <maxg at mellanox.com>
maintainer
#3 lore raw

PATCH v2 2/2 nvme: use a single NVME_AQ_DEPTH and relax it to 32

In reply to Sagi Grimberg

Looks fine, and I can't see any good reason for the large number on PCIe either:

Reviewed-by Christoph Hellwig <hch at lst.de>
#4 lore raw

PATCH v2 2/2 nvme: use a single NVME_AQ_DEPTH and relax it to 32

In reply to Sagi Grimberg
Quoted 2 lines · Sagi Grimberg

No need to differentiate fabrics from pci/loop, also lower it to 32 as we don't really need 256 inflight admin commands.

Might want to check with Jens on reducing the count. d31af0a325ca4f increased the limit from 64 to 256 for a particular use case.

author maintainer
#5 lore raw

PATCH v2 2/2 nvme: use a single NVME_AQ_DEPTH and relax it to 32

Quoted 6 lines
Quoted 3 lines

No need to differentiate fabrics from pci/loop, also lower it to 32 as we don't really need 256 inflight admin commands.

Might want to check with Jens on reducing the count. d31af0a325ca4f increased the limit from 64 to 256 for a particular use case.

Can we modparam it? or quirk it like Martin suggested? My impression is that its an overkill for 99.9% of the use-cases...

#6 lore raw

PATCH v2 2/2 nvme: use a single NVME_AQ_DEPTH and relax it to 32

In reply to Sagi Grimberg

Sagi,

Quoted 2 lines

No need to differentiate fabrics from pci/loop, also lower it to 32 as we don't really need 256 inflight admin commands.

That's fine with me. I was about to submit a patch quirking it to 64 due to a controller problem.

Reviewed-by Martin K. Petersen <martin.petersen at oracle.com>
Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
maintainer
#8 lore raw

PATCH v2 1/2 nvme-core: change commas to semicolons in nvme_get_log_page

In reply to Sagi Grimberg

This looks ok, but I think it's just too much churn at the moment. We already have one patch that moves it to scsi.c as part of the fw activation without reset patchset, and we if the scsi emulation removal goes in it should just go away entirely..

author maintainer
#9 lore raw

PATCH v2 1/2 nvme-core: change commas to semicolons in nvme_get_log_page

Quoted 5 lines

This looks ok, but I think it's just too much churn at the moment. We already have one patch that moves it to scsi.c as part of the fw activation without reset patchset, and we if the scsi emulation removal goes in it should just go away entirely..

OK, we can discard it then...