All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] small set of nvme cleanups
@ 2019-07-19 19:45 Sagi Grimberg
  2019-07-19 19:45 ` [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap Sagi Grimberg
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Sagi Grimberg @ 2019-07-19 19:45 UTC (permalink / raw)


Centralizes queue size settings for fabrics and
removes redundant argument to nvme_disable/enable_ctrl.

Sagi Grimberg (4):
  nvme: have nvme_init_identify set ctrl->cap
  nvme: move sqsize setting to the core
  nvme: don't pass cap to nvme_disable_ctrl
  nvme: don't pass cap to nvme_enable_ctrl

 drivers/nvme/host/core.c   | 19 ++++++++++---------
 drivers/nvme/host/fc.c     | 12 +-----------
 drivers/nvme/host/nvme.h   |  4 ++--
 drivers/nvme/host/pci.c    |  7 ++++---
 drivers/nvme/host/rdma.c   | 15 ++-------------
 drivers/nvme/host/tcp.c    | 13 ++-----------
 drivers/nvme/target/loop.c | 12 +-----------
 7 files changed, 22 insertions(+), 60 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap
  2019-07-19 19:45 [PATCH 0/4] small set of nvme cleanups Sagi Grimberg
@ 2019-07-19 19:45 ` Sagi Grimberg
  2019-07-20  7:55   ` Minwoo Im
  2019-07-20 21:15   ` Chaitanya Kulkarni
  2019-07-19 19:45 ` [PATCH 2/4] nvme: move sqsize setting to the core Sagi Grimberg
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Sagi Grimberg @ 2019-07-19 19:45 UTC (permalink / raw)


No need to use a stack cap variable.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 76cd3dd8736a..058e06e40df8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2563,7 +2563,6 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl)
 int nvme_init_identify(struct nvme_ctrl *ctrl)
 {
 	struct nvme_id_ctrl *id;
-	u64 cap;
 	int ret, page_shift;
 	u32 max_hw_sectors;
 	bool prev_apst_enabled;
@@ -2574,15 +2573,15 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 		return ret;
 	}
 
-	ret = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &cap);
+	ret = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &ctrl->cap);
 	if (ret) {
 		dev_err(ctrl->device, "Reading CAP failed (%d)\n", ret);
 		return ret;
 	}
-	page_shift = NVME_CAP_MPSMIN(cap) + 12;
+	page_shift = NVME_CAP_MPSMIN(ctrl->cap) + 12;
 
 	if (ctrl->vs >= NVME_VS(1, 1, 0))
-		ctrl->subsystem = NVME_CAP_NSSRC(cap);
+		ctrl->subsystem = NVME_CAP_NSSRC(ctrl->cap);
 
 	ret = nvme_identify_ctrl(ctrl, &id);
 	if (ret) {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 2/4] nvme: move sqsize setting to the core
  2019-07-19 19:45 [PATCH 0/4] small set of nvme cleanups Sagi Grimberg
  2019-07-19 19:45 ` [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap Sagi Grimberg
@ 2019-07-19 19:45 ` Sagi Grimberg
  2019-07-20  7:59   ` Minwoo Im
  2019-07-20 21:18   ` Chaitanya Kulkarni
  2019-07-19 19:45 ` [PATCH 3/4] nvme: don't pass cap to nvme_disable_ctrl Sagi Grimberg
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Sagi Grimberg @ 2019-07-19 19:45 UTC (permalink / raw)


nvme_init_identify reads the cap register right after, so
no need to do that locally in the transport driver.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c   |  1 +
 drivers/nvme/host/fc.c     | 10 ----------
 drivers/nvme/host/pci.c    |  1 +
 drivers/nvme/host/rdma.c   | 11 -----------
 drivers/nvme/host/tcp.c    |  9 ---------
 drivers/nvme/target/loop.c | 10 ----------
 6 files changed, 2 insertions(+), 40 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 058e06e40df8..da4182aa16b0 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2579,6 +2579,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 		return ret;
 	}
 	page_shift = NVME_CAP_MPSMIN(ctrl->cap) + 12;
+	ctrl->sqsize = min_t(int, NVME_CAP_MQES(ctrl->cap), ctrl->sqsize);
 
 	if (ctrl->vs >= NVME_VS(1, 1, 0))
 		ctrl->subsystem = NVME_CAP_NSSRC(ctrl->cap);
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 1a391aa1f7d5..67d420bd79f6 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2647,16 +2647,6 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
 	 * prior connection values
 	 */
 
-	ret = nvmf_reg_read64(&ctrl->ctrl, NVME_REG_CAP, &ctrl->ctrl.cap);
-	if (ret) {
-		dev_err(ctrl->ctrl.device,
-			"prop_get NVME_REG_CAP failed\n");
-		goto out_disconnect_admin_queue;
-	}
-
-	ctrl->ctrl.sqsize =
-		min_t(int, NVME_CAP_MQES(ctrl->ctrl.cap), ctrl->ctrl.sqsize);
-
 	ret = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
 	if (ret)
 		goto out_disconnect_admin_queue;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index bb970ca82517..0f38c1d96d19 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2316,6 +2316,7 @@ static int nvme_pci_enable(struct nvme_dev *dev)
 
 	dev->q_depth = min_t(int, NVME_CAP_MQES(dev->ctrl.cap) + 1,
 				io_queue_depth);
+	dev->ctrl.sqsize = dev->q_depth - 1; /* 0's based queue depth */
 	dev->db_stride = 1 << NVME_CAP_STRIDE(dev->ctrl.cap);
 	dev->dbs = dev->bar + 4096;
 
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 7b074323bcdf..8e9f0fd5b01f 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -809,17 +809,6 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl,
 	if (error)
 		goto out_cleanup_queue;
 
-	error = ctrl->ctrl.ops->reg_read64(&ctrl->ctrl, NVME_REG_CAP,
-			&ctrl->ctrl.cap);
-	if (error) {
-		dev_err(ctrl->ctrl.device,
-			"prop_get NVME_REG_CAP failed\n");
-		goto out_stop_queue;
-	}
-
-	ctrl->ctrl.sqsize =
-		min_t(int, NVME_CAP_MQES(ctrl->ctrl.cap), ctrl->ctrl.sqsize);
-
 	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
 	if (error)
 		goto out_stop_queue;
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 8d4965031399..fba80dc61ce1 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1721,15 +1721,6 @@ static int nvme_tcp_configure_admin_queue(struct nvme_ctrl *ctrl, bool new)
 	if (error)
 		goto out_cleanup_queue;
 
-	error = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &ctrl->cap);
-	if (error) {
-		dev_err(ctrl->device,
-			"prop_get NVME_REG_CAP failed\n");
-		goto out_stop_queue;
-	}
-
-	ctrl->sqsize = min_t(int, NVME_CAP_MQES(ctrl->cap), ctrl->sqsize);
-
 	error = nvme_enable_ctrl(ctrl, ctrl->cap);
 	if (error)
 		goto out_stop_queue;
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 9e211ad6bdd3..d901a019e3a2 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -369,16 +369,6 @@ static int nvme_loop_configure_admin_queue(struct nvme_loop_ctrl *ctrl)
 
 	set_bit(NVME_LOOP_Q_LIVE, &ctrl->queues[0].flags);
 
-	error = nvmf_reg_read64(&ctrl->ctrl, NVME_REG_CAP, &ctrl->ctrl.cap);
-	if (error) {
-		dev_err(ctrl->ctrl.device,
-			"prop_get NVME_REG_CAP failed\n");
-		goto out_cleanup_queue;
-	}
-
-	ctrl->ctrl.sqsize =
-		min_t(int, NVME_CAP_MQES(ctrl->ctrl.cap), ctrl->ctrl.sqsize);
-
 	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
 	if (error)
 		goto out_cleanup_queue;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 3/4] nvme: don't pass cap to nvme_disable_ctrl
  2019-07-19 19:45 [PATCH 0/4] small set of nvme cleanups Sagi Grimberg
  2019-07-19 19:45 ` [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap Sagi Grimberg
  2019-07-19 19:45 ` [PATCH 2/4] nvme: move sqsize setting to the core Sagi Grimberg
@ 2019-07-19 19:45 ` Sagi Grimberg
  2019-07-20  8:00   ` Minwoo Im
  2019-07-20 21:22   ` Chaitanya Kulkarni
  2019-07-19 19:45 ` [PATCH 4/4] nvme: don't pass cap to nvme_enable_ctrl Sagi Grimberg
  2019-07-22 15:42 ` [PATCH 0/4] small set of nvme cleanups James Smart
  4 siblings, 2 replies; 15+ messages in thread
From: Sagi Grimberg @ 2019-07-19 19:45 UTC (permalink / raw)


All seem to call it with ctrl->cap so no need to pass it
at all.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c | 4 ++--
 drivers/nvme/host/nvme.h | 2 +-
 drivers/nvme/host/pci.c  | 4 ++--
 drivers/nvme/host/rdma.c | 2 +-
 drivers/nvme/host/tcp.c  | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index da4182aa16b0..60e924b0b510 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1949,7 +1949,7 @@ static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled)
  * bits', but doing so may cause the device to complete commands to the
  * admin queue ... and we don't know what memory that might be pointing at!
  */
-int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
+int nvme_disable_ctrl(struct nvme_ctrl *ctrl)
 {
 	int ret;
 
@@ -1963,7 +1963,7 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 	if (ctrl->quirks & NVME_QUIRK_DELAY_BEFORE_CHK_RDY)
 		msleep(NVME_QUIRK_DELAY_AMOUNT);
 
-	return nvme_wait_ready(ctrl, cap, false);
+	return nvme_wait_ready(ctrl, ctrl->cap, false);
 }
 EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 716a876119c8..a7f8fccb1f35 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -426,7 +426,7 @@ void nvme_complete_rq(struct request *req);
 bool nvme_cancel_request(struct request *req, void *data, bool reserved);
 bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 		enum nvme_ctrl_state new_state);
-int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
+int nvme_disable_ctrl(struct nvme_ctrl *ctrl);
 int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
 int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl);
 int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0f38c1d96d19..f2e74043dc6b 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1403,7 +1403,7 @@ static void nvme_disable_admin_queue(struct nvme_dev *dev, bool shutdown)
 	if (shutdown)
 		nvme_shutdown_ctrl(&dev->ctrl);
 	else
-		nvme_disable_ctrl(&dev->ctrl, dev->ctrl.cap);
+		nvme_disable_ctrl(&dev->ctrl);
 
 	nvme_poll_irqdisable(nvmeq, -1);
 }
@@ -1679,7 +1679,7 @@ static int nvme_pci_configure_admin_queue(struct nvme_dev *dev)
 	    (readl(dev->bar + NVME_REG_CSTS) & NVME_CSTS_NSSRO))
 		writel(NVME_CSTS_NSSRO, dev->bar + NVME_REG_CSTS);
 
-	result = nvme_disable_ctrl(&dev->ctrl, dev->ctrl.cap);
+	result = nvme_disable_ctrl(&dev->ctrl);
 	if (result < 0)
 		return result;
 
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 8e9f0fd5b01f..8e2094fb5e7e 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1860,7 +1860,7 @@ static void nvme_rdma_shutdown_ctrl(struct nvme_rdma_ctrl *ctrl, bool shutdown)
 	if (shutdown)
 		nvme_shutdown_ctrl(&ctrl->ctrl);
 	else
-		nvme_disable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
+		nvme_disable_ctrl(&ctrl->ctrl);
 	nvme_rdma_teardown_admin_queue(ctrl, shutdown);
 }
 
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index fba80dc61ce1..4a32d5350fc5 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1896,7 +1896,7 @@ static void nvme_tcp_teardown_ctrl(struct nvme_ctrl *ctrl, bool shutdown)
 	if (shutdown)
 		nvme_shutdown_ctrl(ctrl);
 	else
-		nvme_disable_ctrl(ctrl, ctrl->cap);
+		nvme_disable_ctrl(ctrl);
 	nvme_tcp_teardown_admin_queue(ctrl, shutdown);
 }
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 4/4] nvme: don't pass cap to nvme_enable_ctrl
  2019-07-19 19:45 [PATCH 0/4] small set of nvme cleanups Sagi Grimberg
                   ` (2 preceding siblings ...)
  2019-07-19 19:45 ` [PATCH 3/4] nvme: don't pass cap to nvme_disable_ctrl Sagi Grimberg
@ 2019-07-19 19:45 ` Sagi Grimberg
  2019-07-20 21:22   ` Chaitanya Kulkarni
  2019-07-22 15:42 ` [PATCH 0/4] small set of nvme cleanups James Smart
  4 siblings, 1 reply; 15+ messages in thread
From: Sagi Grimberg @ 2019-07-19 19:45 UTC (permalink / raw)


All seem to call it with ctrl->cap so no need to pass it
at all.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c   | 7 ++++---
 drivers/nvme/host/fc.c     | 2 +-
 drivers/nvme/host/nvme.h   | 2 +-
 drivers/nvme/host/pci.c    | 2 +-
 drivers/nvme/host/rdma.c   | 2 +-
 drivers/nvme/host/tcp.c    | 2 +-
 drivers/nvme/target/loop.c | 2 +-
 7 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 60e924b0b510..93b7e362f0ac 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1967,14 +1967,15 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl)
 }
 EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
 
-int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
+int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
 {
 	/*
 	 * Default to a 4K page size, with the intention to update this
 	 * path in the future to accomodate architectures with differing
 	 * kernel and IO page sizes.
 	 */
-	unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12, page_shift = 12;
+	unsigned dev_page_min = NVME_CAP_MPSMIN(ctrl->cap) + 12;
+	unsigned page_shift = 12;
 	int ret;
 
 	if (page_shift < dev_page_min) {
@@ -1995,7 +1996,7 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 	ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
 	if (ret)
 		return ret;
-	return nvme_wait_ready(ctrl, cap, true);
+	return nvme_wait_ready(ctrl, ctrl->cap, true);
 }
 EXPORT_SYMBOL_GPL(nvme_enable_ctrl);
 
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 67d420bd79f6..e5c2a03a6c22 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2647,7 +2647,7 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
 	 * prior connection values
 	 */
 
-	ret = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
+	ret = nvme_enable_ctrl(&ctrl->ctrl);
 	if (ret)
 		goto out_disconnect_admin_queue;
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a7f8fccb1f35..740241e84d29 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -427,7 +427,7 @@ bool nvme_cancel_request(struct request *req, void *data, bool reserved);
 bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 		enum nvme_ctrl_state new_state);
 int nvme_disable_ctrl(struct nvme_ctrl *ctrl);
-int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
+int nvme_enable_ctrl(struct nvme_ctrl *ctrl);
 int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl);
 int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 		const struct nvme_ctrl_ops *ops, unsigned long quirks);
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index f2e74043dc6b..4b508d5e45cf 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1695,7 +1695,7 @@ static int nvme_pci_configure_admin_queue(struct nvme_dev *dev)
 	lo_hi_writeq(nvmeq->sq_dma_addr, dev->bar + NVME_REG_ASQ);
 	lo_hi_writeq(nvmeq->cq_dma_addr, dev->bar + NVME_REG_ACQ);
 
-	result = nvme_enable_ctrl(&dev->ctrl, dev->ctrl.cap);
+	result = nvme_enable_ctrl(&dev->ctrl);
 	if (result)
 		return result;
 
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 8e2094fb5e7e..3fd44e1c279d 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -809,7 +809,7 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl,
 	if (error)
 		goto out_cleanup_queue;
 
-	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
+	error = nvme_enable_ctrl(&ctrl->ctrl);
 	if (error)
 		goto out_stop_queue;
 
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 4a32d5350fc5..77c3d4d06d29 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1721,7 +1721,7 @@ static int nvme_tcp_configure_admin_queue(struct nvme_ctrl *ctrl, bool new)
 	if (error)
 		goto out_cleanup_queue;
 
-	error = nvme_enable_ctrl(ctrl, ctrl->cap);
+	error = nvme_enable_ctrl(ctrl);
 	if (error)
 		goto out_stop_queue;
 
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index d901a019e3a2..a53a0c0170f5 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -369,7 +369,7 @@ static int nvme_loop_configure_admin_queue(struct nvme_loop_ctrl *ctrl)
 
 	set_bit(NVME_LOOP_Q_LIVE, &ctrl->queues[0].flags);
 
-	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
+	error = nvme_enable_ctrl(&ctrl->ctrl);
 	if (error)
 		goto out_cleanup_queue;
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap
  2019-07-19 19:45 ` [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap Sagi Grimberg
@ 2019-07-20  7:55   ` Minwoo Im
  2019-07-20  8:13     ` Minwoo Im
  2019-07-20 21:15   ` Chaitanya Kulkarni
  1 sibling, 1 reply; 15+ messages in thread
From: Minwoo Im @ 2019-07-20  7:55 UTC (permalink / raw)


On 19-07-19 12:45:43, Sagi Grimberg wrote:
> No need to use a stack cap variable.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>

This looks good to me.

Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 2/4] nvme: move sqsize setting to the core
  2019-07-19 19:45 ` [PATCH 2/4] nvme: move sqsize setting to the core Sagi Grimberg
@ 2019-07-20  7:59   ` Minwoo Im
  2019-07-20 21:18   ` Chaitanya Kulkarni
  1 sibling, 0 replies; 15+ messages in thread
From: Minwoo Im @ 2019-07-20  7:59 UTC (permalink / raw)


On 19-07-19 12:45:44, Sagi Grimberg wrote:
> nvme_init_identify reads the cap register right after, so
> no need to do that locally in the transport driver.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>

If the first patch is good to be picked up, I'm fine with this patch.

Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 3/4] nvme: don't pass cap to nvme_disable_ctrl
  2019-07-19 19:45 ` [PATCH 3/4] nvme: don't pass cap to nvme_disable_ctrl Sagi Grimberg
@ 2019-07-20  8:00   ` Minwoo Im
  2019-07-20 21:22   ` Chaitanya Kulkarni
  1 sibling, 0 replies; 15+ messages in thread
From: Minwoo Im @ 2019-07-20  8:00 UTC (permalink / raw)


On 19-07-19 12:45:45, Sagi Grimberg wrote:
> All seem to call it with ctrl->cap so no need to pass it
> at all.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>

Looks good to me.

Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap
  2019-07-20  7:55   ` Minwoo Im
@ 2019-07-20  8:13     ` Minwoo Im
  0 siblings, 0 replies; 15+ messages in thread
From: Minwoo Im @ 2019-07-20  8:13 UTC (permalink / raw)


Sagi,

I think the ctrl->cap is already configured frmo the following
functions which are all before the nvme_init_identify():
	pci: nvme_pci_enable()
	rdma: nvme_rdma_configure_admin_queue()
	fc: nvme_fc_create_association()

Even it's already stored before it can be read again, but should we
really make it read again here?

Also I can see the 2nd patch description which tells that
nvme_init_identify() loaded the ctrl->cap value instead of the functinos
above.

If there's somethiig I missed here, Please let me know!

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap
  2019-07-19 19:45 ` [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap Sagi Grimberg
  2019-07-20  7:55   ` Minwoo Im
@ 2019-07-20 21:15   ` Chaitanya Kulkarni
  1 sibling, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-20 21:15 UTC (permalink / raw)


Looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 07/19/2019 12:46 PM, Sagi Grimberg wrote:
> No need to use a stack cap variable.
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   drivers/nvme/host/core.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 76cd3dd8736a..058e06e40df8 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2563,7 +2563,6 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl)
>   int nvme_init_identify(struct nvme_ctrl *ctrl)
>   {
>   	struct nvme_id_ctrl *id;
> -	u64 cap;
>   	int ret, page_shift;
>   	u32 max_hw_sectors;
>   	bool prev_apst_enabled;
> @@ -2574,15 +2573,15 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
>   		return ret;
>   	}
>
> -	ret = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &cap);
> +	ret = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &ctrl->cap);
>   	if (ret) {
>   		dev_err(ctrl->device, "Reading CAP failed (%d)\n", ret);
>   		return ret;
>   	}
> -	page_shift = NVME_CAP_MPSMIN(cap) + 12;
> +	page_shift = NVME_CAP_MPSMIN(ctrl->cap) + 12;
>
>   	if (ctrl->vs >= NVME_VS(1, 1, 0))
> -		ctrl->subsystem = NVME_CAP_NSSRC(cap);
> +		ctrl->subsystem = NVME_CAP_NSSRC(ctrl->cap);
>
>   	ret = nvme_identify_ctrl(ctrl, &id);
>   	if (ret) {
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 2/4] nvme: move sqsize setting to the core
  2019-07-19 19:45 ` [PATCH 2/4] nvme: move sqsize setting to the core Sagi Grimberg
  2019-07-20  7:59   ` Minwoo Im
@ 2019-07-20 21:18   ` Chaitanya Kulkarni
  1 sibling, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-20 21:18 UTC (permalink / raw)


Looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

On 07/19/2019 12:46 PM, Sagi Grimberg wrote:
> nvme_init_identify reads the cap register right after, so
> no need to do that locally in the transport driver.
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   drivers/nvme/host/core.c   |  1 +
>   drivers/nvme/host/fc.c     | 10 ----------
>   drivers/nvme/host/pci.c    |  1 +
>   drivers/nvme/host/rdma.c   | 11 -----------
>   drivers/nvme/host/tcp.c    |  9 ---------
>   drivers/nvme/target/loop.c | 10 ----------
>   6 files changed, 2 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 058e06e40df8..da4182aa16b0 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2579,6 +2579,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
>   		return ret;
>   	}
>   	page_shift = NVME_CAP_MPSMIN(ctrl->cap) + 12;
> +	ctrl->sqsize = min_t(int, NVME_CAP_MQES(ctrl->cap), ctrl->sqsize);
>
>   	if (ctrl->vs >= NVME_VS(1, 1, 0))
>   		ctrl->subsystem = NVME_CAP_NSSRC(ctrl->cap);
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 1a391aa1f7d5..67d420bd79f6 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -2647,16 +2647,6 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
>   	 * prior connection values
>   	 */
>
> -	ret = nvmf_reg_read64(&ctrl->ctrl, NVME_REG_CAP, &ctrl->ctrl.cap);
> -	if (ret) {
> -		dev_err(ctrl->ctrl.device,
> -			"prop_get NVME_REG_CAP failed\n");
> -		goto out_disconnect_admin_queue;
> -	}
> -
> -	ctrl->ctrl.sqsize =
> -		min_t(int, NVME_CAP_MQES(ctrl->ctrl.cap), ctrl->ctrl.sqsize);
> -
>   	ret = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
>   	if (ret)
>   		goto out_disconnect_admin_queue;
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index bb970ca82517..0f38c1d96d19 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2316,6 +2316,7 @@ static int nvme_pci_enable(struct nvme_dev *dev)
>
>   	dev->q_depth = min_t(int, NVME_CAP_MQES(dev->ctrl.cap) + 1,
>   				io_queue_depth);
> +	dev->ctrl.sqsize = dev->q_depth - 1; /* 0's based queue depth */
>   	dev->db_stride = 1 << NVME_CAP_STRIDE(dev->ctrl.cap);
>   	dev->dbs = dev->bar + 4096;
>
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 7b074323bcdf..8e9f0fd5b01f 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -809,17 +809,6 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl,
>   	if (error)
>   		goto out_cleanup_queue;
>
> -	error = ctrl->ctrl.ops->reg_read64(&ctrl->ctrl, NVME_REG_CAP,
> -			&ctrl->ctrl.cap);
> -	if (error) {
> -		dev_err(ctrl->ctrl.device,
> -			"prop_get NVME_REG_CAP failed\n");
> -		goto out_stop_queue;
> -	}
> -
> -	ctrl->ctrl.sqsize =
> -		min_t(int, NVME_CAP_MQES(ctrl->ctrl.cap), ctrl->ctrl.sqsize);
> -
>   	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
>   	if (error)
>   		goto out_stop_queue;
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 8d4965031399..fba80dc61ce1 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1721,15 +1721,6 @@ static int nvme_tcp_configure_admin_queue(struct nvme_ctrl *ctrl, bool new)
>   	if (error)
>   		goto out_cleanup_queue;
>
> -	error = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &ctrl->cap);
> -	if (error) {
> -		dev_err(ctrl->device,
> -			"prop_get NVME_REG_CAP failed\n");
> -		goto out_stop_queue;
> -	}
> -
> -	ctrl->sqsize = min_t(int, NVME_CAP_MQES(ctrl->cap), ctrl->sqsize);
> -
>   	error = nvme_enable_ctrl(ctrl, ctrl->cap);
>   	if (error)
>   		goto out_stop_queue;
> diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
> index 9e211ad6bdd3..d901a019e3a2 100644
> --- a/drivers/nvme/target/loop.c
> +++ b/drivers/nvme/target/loop.c
> @@ -369,16 +369,6 @@ static int nvme_loop_configure_admin_queue(struct nvme_loop_ctrl *ctrl)
>
>   	set_bit(NVME_LOOP_Q_LIVE, &ctrl->queues[0].flags);
>
> -	error = nvmf_reg_read64(&ctrl->ctrl, NVME_REG_CAP, &ctrl->ctrl.cap);
> -	if (error) {
> -		dev_err(ctrl->ctrl.device,
> -			"prop_get NVME_REG_CAP failed\n");
> -		goto out_cleanup_queue;
> -	}
> -
> -	ctrl->ctrl.sqsize =
> -		min_t(int, NVME_CAP_MQES(ctrl->ctrl.cap), ctrl->ctrl.sqsize);
> -
>   	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
>   	if (error)
>   		goto out_cleanup_queue;
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 3/4] nvme: don't pass cap to nvme_disable_ctrl
  2019-07-19 19:45 ` [PATCH 3/4] nvme: don't pass cap to nvme_disable_ctrl Sagi Grimberg
  2019-07-20  8:00   ` Minwoo Im
@ 2019-07-20 21:22   ` Chaitanya Kulkarni
  1 sibling, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-20 21:22 UTC (permalink / raw)


Looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

On 07/19/2019 12:46 PM, Sagi Grimberg wrote:
> All seem to call it with ctrl->cap so no need to pass it
> at all.
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   drivers/nvme/host/core.c | 4 ++--
>   drivers/nvme/host/nvme.h | 2 +-
>   drivers/nvme/host/pci.c  | 4 ++--
>   drivers/nvme/host/rdma.c | 2 +-
>   drivers/nvme/host/tcp.c  | 2 +-
>   5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index da4182aa16b0..60e924b0b510 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1949,7 +1949,7 @@ static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled)
>    * bits', but doing so may cause the device to complete commands to the
>    * admin queue ... and we don't know what memory that might be pointing at!
>    */
> -int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
> +int nvme_disable_ctrl(struct nvme_ctrl *ctrl)
>   {
>   	int ret;
>
> @@ -1963,7 +1963,7 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
>   	if (ctrl->quirks & NVME_QUIRK_DELAY_BEFORE_CHK_RDY)
>   		msleep(NVME_QUIRK_DELAY_AMOUNT);
>
> -	return nvme_wait_ready(ctrl, cap, false);
> +	return nvme_wait_ready(ctrl, ctrl->cap, false);
>   }
>   EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
>
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 716a876119c8..a7f8fccb1f35 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -426,7 +426,7 @@ void nvme_complete_rq(struct request *req);
>   bool nvme_cancel_request(struct request *req, void *data, bool reserved);
>   bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
>   		enum nvme_ctrl_state new_state);
> -int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
> +int nvme_disable_ctrl(struct nvme_ctrl *ctrl);
>   int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
>   int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl);
>   int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 0f38c1d96d19..f2e74043dc6b 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1403,7 +1403,7 @@ static void nvme_disable_admin_queue(struct nvme_dev *dev, bool shutdown)
>   	if (shutdown)
>   		nvme_shutdown_ctrl(&dev->ctrl);
>   	else
> -		nvme_disable_ctrl(&dev->ctrl, dev->ctrl.cap);
> +		nvme_disable_ctrl(&dev->ctrl);
>
>   	nvme_poll_irqdisable(nvmeq, -1);
>   }
> @@ -1679,7 +1679,7 @@ static int nvme_pci_configure_admin_queue(struct nvme_dev *dev)
>   	    (readl(dev->bar + NVME_REG_CSTS) & NVME_CSTS_NSSRO))
>   		writel(NVME_CSTS_NSSRO, dev->bar + NVME_REG_CSTS);
>
> -	result = nvme_disable_ctrl(&dev->ctrl, dev->ctrl.cap);
> +	result = nvme_disable_ctrl(&dev->ctrl);
>   	if (result < 0)
>   		return result;
>
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 8e9f0fd5b01f..8e2094fb5e7e 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -1860,7 +1860,7 @@ static void nvme_rdma_shutdown_ctrl(struct nvme_rdma_ctrl *ctrl, bool shutdown)
>   	if (shutdown)
>   		nvme_shutdown_ctrl(&ctrl->ctrl);
>   	else
> -		nvme_disable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
> +		nvme_disable_ctrl(&ctrl->ctrl);
>   	nvme_rdma_teardown_admin_queue(ctrl, shutdown);
>   }
>
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index fba80dc61ce1..4a32d5350fc5 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1896,7 +1896,7 @@ static void nvme_tcp_teardown_ctrl(struct nvme_ctrl *ctrl, bool shutdown)
>   	if (shutdown)
>   		nvme_shutdown_ctrl(ctrl);
>   	else
> -		nvme_disable_ctrl(ctrl, ctrl->cap);
> +		nvme_disable_ctrl(ctrl);
>   	nvme_tcp_teardown_admin_queue(ctrl, shutdown);
>   }
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 4/4] nvme: don't pass cap to nvme_enable_ctrl
  2019-07-19 19:45 ` [PATCH 4/4] nvme: don't pass cap to nvme_enable_ctrl Sagi Grimberg
@ 2019-07-20 21:22   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-20 21:22 UTC (permalink / raw)


Looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

On 07/19/2019 12:47 PM, Sagi Grimberg wrote:
> All seem to call it with ctrl->cap so no need to pass it
> at all.
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   drivers/nvme/host/core.c   | 7 ++++---
>   drivers/nvme/host/fc.c     | 2 +-
>   drivers/nvme/host/nvme.h   | 2 +-
>   drivers/nvme/host/pci.c    | 2 +-
>   drivers/nvme/host/rdma.c   | 2 +-
>   drivers/nvme/host/tcp.c    | 2 +-
>   drivers/nvme/target/loop.c | 2 +-
>   7 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 60e924b0b510..93b7e362f0ac 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1967,14 +1967,15 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl)
>   }
>   EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
>
> -int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
> +int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
>   {
>   	/*
>   	 * Default to a 4K page size, with the intention to update this
>   	 * path in the future to accomodate architectures with differing
>   	 * kernel and IO page sizes.
>   	 */
> -	unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12, page_shift = 12;
> +	unsigned dev_page_min = NVME_CAP_MPSMIN(ctrl->cap) + 12;
> +	unsigned page_shift = 12;
>   	int ret;
>
>   	if (page_shift < dev_page_min) {
> @@ -1995,7 +1996,7 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
>   	ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
>   	if (ret)
>   		return ret;
> -	return nvme_wait_ready(ctrl, cap, true);
> +	return nvme_wait_ready(ctrl, ctrl->cap, true);
>   }
>   EXPORT_SYMBOL_GPL(nvme_enable_ctrl);
>
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 67d420bd79f6..e5c2a03a6c22 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -2647,7 +2647,7 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
>   	 * prior connection values
>   	 */
>
> -	ret = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
> +	ret = nvme_enable_ctrl(&ctrl->ctrl);
>   	if (ret)
>   		goto out_disconnect_admin_queue;
>
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index a7f8fccb1f35..740241e84d29 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -427,7 +427,7 @@ bool nvme_cancel_request(struct request *req, void *data, bool reserved);
>   bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
>   		enum nvme_ctrl_state new_state);
>   int nvme_disable_ctrl(struct nvme_ctrl *ctrl);
> -int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
> +int nvme_enable_ctrl(struct nvme_ctrl *ctrl);
>   int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl);
>   int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
>   		const struct nvme_ctrl_ops *ops, unsigned long quirks);
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index f2e74043dc6b..4b508d5e45cf 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1695,7 +1695,7 @@ static int nvme_pci_configure_admin_queue(struct nvme_dev *dev)
>   	lo_hi_writeq(nvmeq->sq_dma_addr, dev->bar + NVME_REG_ASQ);
>   	lo_hi_writeq(nvmeq->cq_dma_addr, dev->bar + NVME_REG_ACQ);
>
> -	result = nvme_enable_ctrl(&dev->ctrl, dev->ctrl.cap);
> +	result = nvme_enable_ctrl(&dev->ctrl);
>   	if (result)
>   		return result;
>
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 8e2094fb5e7e..3fd44e1c279d 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -809,7 +809,7 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl,
>   	if (error)
>   		goto out_cleanup_queue;
>
> -	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
> +	error = nvme_enable_ctrl(&ctrl->ctrl);
>   	if (error)
>   		goto out_stop_queue;
>
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 4a32d5350fc5..77c3d4d06d29 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1721,7 +1721,7 @@ static int nvme_tcp_configure_admin_queue(struct nvme_ctrl *ctrl, bool new)
>   	if (error)
>   		goto out_cleanup_queue;
>
> -	error = nvme_enable_ctrl(ctrl, ctrl->cap);
> +	error = nvme_enable_ctrl(ctrl);
>   	if (error)
>   		goto out_stop_queue;
>
> diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
> index d901a019e3a2..a53a0c0170f5 100644
> --- a/drivers/nvme/target/loop.c
> +++ b/drivers/nvme/target/loop.c
> @@ -369,7 +369,7 @@ static int nvme_loop_configure_admin_queue(struct nvme_loop_ctrl *ctrl)
>
>   	set_bit(NVME_LOOP_Q_LIVE, &ctrl->queues[0].flags);
>
> -	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->ctrl.cap);
> +	error = nvme_enable_ctrl(&ctrl->ctrl);
>   	if (error)
>   		goto out_cleanup_queue;
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 0/4] small set of nvme cleanups
  2019-07-19 19:45 [PATCH 0/4] small set of nvme cleanups Sagi Grimberg
                   ` (3 preceding siblings ...)
  2019-07-19 19:45 ` [PATCH 4/4] nvme: don't pass cap to nvme_enable_ctrl Sagi Grimberg
@ 2019-07-22 15:42 ` James Smart
  2019-07-22 18:11   ` Sagi Grimberg
  4 siblings, 1 reply; 15+ messages in thread
From: James Smart @ 2019-07-22 15:42 UTC (permalink / raw)




On 7/19/2019 12:45 PM, Sagi Grimberg wrote:
> Centralizes queue size settings for fabrics and
> removes redundant argument to nvme_disable/enable_ctrl.
>
> Sagi Grimberg (4):
>    nvme: have nvme_init_identify set ctrl->cap
>    nvme: move sqsize setting to the core
>    nvme: don't pass cap to nvme_disable_ctrl
>    nvme: don't pass cap to nvme_enable_ctrl
>
>   drivers/nvme/host/core.c   | 19 ++++++++++---------
>   drivers/nvme/host/fc.c     | 12 +-----------
>   drivers/nvme/host/nvme.h   |  4 ++--
>   drivers/nvme/host/pci.c    |  7 ++++---
>   drivers/nvme/host/rdma.c   | 15 ++-------------
>   drivers/nvme/host/tcp.c    | 13 ++-----------
>   drivers/nvme/target/loop.c | 12 +-----------
>   7 files changed, 22 insertions(+), 60 deletions(-)
>

The changes look fine except for 1 main issue:? All of the nvmf 
transports call nvme_enable_ctrl(), which still references ctrl->cap, 
before they call nvme_init_identify().

-- james

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 0/4] small set of nvme cleanups
  2019-07-22 15:42 ` [PATCH 0/4] small set of nvme cleanups James Smart
@ 2019-07-22 18:11   ` Sagi Grimberg
  0 siblings, 0 replies; 15+ messages in thread
From: Sagi Grimberg @ 2019-07-22 18:11 UTC (permalink / raw)



> The changes look fine except for 1 main issue:? All of the nvmf 
> transports call nvme_enable_ctrl(), which still references ctrl->cap, 
> before they call nvme_init_identify().

You're right, I'll move the cap register read to nvme_enable_ctrl.

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-07-22 18:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-19 19:45 [PATCH 0/4] small set of nvme cleanups Sagi Grimberg
2019-07-19 19:45 ` [PATCH 1/4] nvme: have nvme_init_identify set ctrl->cap Sagi Grimberg
2019-07-20  7:55   ` Minwoo Im
2019-07-20  8:13     ` Minwoo Im
2019-07-20 21:15   ` Chaitanya Kulkarni
2019-07-19 19:45 ` [PATCH 2/4] nvme: move sqsize setting to the core Sagi Grimberg
2019-07-20  7:59   ` Minwoo Im
2019-07-20 21:18   ` Chaitanya Kulkarni
2019-07-19 19:45 ` [PATCH 3/4] nvme: don't pass cap to nvme_disable_ctrl Sagi Grimberg
2019-07-20  8:00   ` Minwoo Im
2019-07-20 21:22   ` Chaitanya Kulkarni
2019-07-19 19:45 ` [PATCH 4/4] nvme: don't pass cap to nvme_enable_ctrl Sagi Grimberg
2019-07-20 21:22   ` Chaitanya Kulkarni
2019-07-22 15:42 ` [PATCH 0/4] small set of nvme cleanups James Smart
2019-07-22 18:11   ` Sagi Grimberg

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.