linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/8] nvme: few cleanups and small improvements
@ 2021-03-01  2:06 Chaitanya Kulkarni
  2021-03-01  2:06 ` [PATCH V2 1/8] nvme-core: rename nvme_init_identify() Chaitanya Kulkarni
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

Hi,

This has few cleanups such as variable data type, removing oneliner
wrappers for single callers, removing duplicate kfree() and small
improvements for NVMeOF Passthru fast path such as likely annotation,
removing the extra checks, making function inline for host/core.c.
Last couple of patches fixes warning for FC function header
documentation.

-ck

v1->v2:

1. Rename the nvme_init_identify() to nvme_init_ctrl_finish(),
2. Move nvme_id_ctrl struct initialization code into separate helper
   nvme_init_identify().
3. Remove the patches to avoid extra churn, keep the fc patches.
4. Keep the small improvements which are needed for the passthru code.
5. Add pr cleanup patch.

Chaitanya Kulkarni (8):
  nvme-core: rename nvme_init_identify()
  nvme-core: split init identify into helper
  nvme-core: mark nvme_setup_passsthru() inline
  nvme-core: use likely in nvme_init_request()
  nvme-core: don't check nvme_req flags for new req
  nvme-fc: fix the function documentation comment
  nvmet-fc: update function documentation
  nvme-core: add new line after variable declatation

 drivers/nvme/host/core.c   | 79 ++++++++++++++++++++++----------------
 drivers/nvme/host/fc.c     |  4 +-
 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/fc.c   |  1 +
 drivers/nvme/target/loop.c |  2 +-
 8 files changed, 53 insertions(+), 41 deletions(-)

-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V2 1/8] nvme-core: rename nvme_init_identify()
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
@ 2021-03-01  2:06 ` Chaitanya Kulkarni
  2021-03-01  2:06 ` [PATCH V2 2/8] nvme-core: split init identify into helper Chaitanya Kulkarni
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

This is a prep patch so that we can move the identify data structure
related code initialization from nvme_init_identify() into a helper.

Rename the function nvmet_init_identify() to nvmet_init_ctrl_finish().

Next patch will move the nvme_id_ctrl related initialization from newly
renamed function nvme_init_ctrl_finish() into the nvme_init_identify()
helper.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/core.c   | 8 ++++----
 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(+), 10 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e68a8c4ac5a6..b72d0ec706be 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1119,7 +1119,7 @@ static void nvme_passthru_end(struct nvme_ctrl *ctrl, u32 effects)
 		mutex_unlock(&ctrl->scan_lock);
 	}
 	if (effects & NVME_CMD_EFFECTS_CCC)
-		nvme_init_identify(ctrl);
+		nvme_init_ctrl_finish(ctrl);
 	if (effects & (NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC)) {
 		nvme_queue_scan(ctrl);
 		flush_work(&ctrl->scan_work);
@@ -1978,7 +1978,7 @@ static void nvme_config_write_zeroes(struct gendisk *disk, struct nvme_ns *ns)
 	 * In order to be more cautious use controller's max_hw_sectors value
 	 * to configure the maximum sectors for the write-zeroes which is
 	 * configured based on the controller's MDTS field in the
-	 * nvme_init_identify() if available.
+	 * nvme_init_ctrl_finish() if available.
 	 */
 	if (ns->ctrl->max_hw_sectors == UINT_MAX)
 		max_blocks = (u64)USHRT_MAX + 1;
@@ -3064,7 +3064,7 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
  * register in our nvme_ctrl structure.  This should be called as soon as
  * the admin queue is fully up and running.
  */
-int nvme_init_identify(struct nvme_ctrl *ctrl)
+int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
 {
 	struct nvme_id_ctrl *id;
 	int ret, page_shift;
@@ -3251,7 +3251,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 	kfree(id);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(nvme_init_identify);
+EXPORT_SYMBOL_GPL(nvme_init_ctrl_finish);
 
 static int nvme_dev_open(struct inode *inode, struct file *file)
 {
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 20dadd86e981..962987a330d6 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3085,7 +3085,7 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
 
 	blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
 
-	ret = nvme_init_identify(&ctrl->ctrl);
+	ret = nvme_init_ctrl_finish(&ctrl->ctrl);
 	if (ret || test_bit(ASSOC_FAILED, &ctrl->flags))
 		goto out_disconnect_admin_queue;
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 07b34175c6ce..624fc4334a2c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -599,7 +599,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 void nvme_uninit_ctrl(struct nvme_ctrl *ctrl);
 void nvme_start_ctrl(struct nvme_ctrl *ctrl);
 void nvme_stop_ctrl(struct nvme_ctrl *ctrl);
-int nvme_init_identify(struct nvme_ctrl *ctrl);
+int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl);
 
 void nvme_remove_namespaces(struct nvme_ctrl *ctrl);
 
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 207137a0ed8e..b8f0bfaa8f2a 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2652,7 +2652,7 @@ static void nvme_reset_work(struct work_struct *work)
 	 */
 	dev->ctrl.max_integrity_segments = 1;
 
-	result = nvme_init_identify(&dev->ctrl);
+	result = nvme_init_ctrl_finish(&dev->ctrl);
 	if (result)
 		goto out;
 
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 53ac4d7442ba..9c710839b03a 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -917,7 +917,7 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl,
 
 	blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
 
-	error = nvme_init_identify(&ctrl->ctrl);
+	error = nvme_init_ctrl_finish(&ctrl->ctrl);
 	if (error)
 		goto out_quiesce_queue;
 
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 69f59d2c5799..735e768f9f43 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1875,7 +1875,7 @@ static int nvme_tcp_configure_admin_queue(struct nvme_ctrl *ctrl, bool new)
 
 	blk_mq_unquiesce_queue(ctrl->admin_q);
 
-	error = nvme_init_identify(ctrl);
+	error = nvme_init_ctrl_finish(ctrl);
 	if (error)
 		goto out_quiesce_queue;
 
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index cb6f86572b24..a7f97c8b2f77 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -396,7 +396,7 @@ static int nvme_loop_configure_admin_queue(struct nvme_loop_ctrl *ctrl)
 
 	blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
 
-	error = nvme_init_identify(&ctrl->ctrl);
+	error = nvme_init_ctrl_finish(&ctrl->ctrl);
 	if (error)
 		goto out_cleanup_queue;
 
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V2 2/8] nvme-core: split init identify into helper
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
  2021-03-01  2:06 ` [PATCH V2 1/8] nvme-core: rename nvme_init_identify() Chaitanya Kulkarni
@ 2021-03-01  2:06 ` Chaitanya Kulkarni
  2021-03-09 10:09   ` Christoph Hellwig
  2021-03-01  2:06 ` [PATCH V2 3/8] nvme-core: mark nvme_setup_passsthru() inline Chaitanya Kulkarni
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

The function nvme_init_ctrl_finish() (formerly nvme_init_identify()) has
grown over the period of time about ~200 lines given the size of nvme id
ctrl data structure.

Move the nvme_id_ctrl data structure related initilzation into helper
nvme_init_identify() and call it from nvme_init_ctrl_finish().

When we move the code into nvme_init_identify() change the local
variable i from int to unsigned int and remove the duplicate kfree()
after nvme_mpath_init() and jump to the label out_free if
nvme_mpath_ini() fails.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/core.c | 55 +++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b72d0ec706be..379ebb54f5dd 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3059,28 +3059,14 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
 	return 0;
 }
 
-/*
- * Initialize the cached copies of the Identify data and various controller
- * register in our nvme_ctrl structure.  This should be called as soon as
- * the admin queue is fully up and running.
- */
-int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
+static int nvme_init_identify(struct nvme_ctrl *ctrl)
 {
 	struct nvme_id_ctrl *id;
 	int ret, page_shift;
 	u32 max_hw_sectors;
 	bool prev_apst_enabled;
 
-	ret = ctrl->ops->reg_read32(ctrl, NVME_REG_VS, &ctrl->vs);
-	if (ret) {
-		dev_err(ctrl->device, "Reading VS failed (%d)\n", ret);
-		return ret;
-	}
 	page_shift = NVME_CAP_MPSMIN(ctrl->cap) + 12;
-	ctrl->sqsize = min_t(u16, NVME_CAP_MQES(ctrl->cap), ctrl->sqsize);
-
-	if (ctrl->vs >= NVME_VS(1, 1, 0))
-		ctrl->subsystem = NVME_CAP_NSSRC(ctrl->cap);
 
 	ret = nvme_identify_ctrl(ctrl, &id);
 	if (ret) {
@@ -3098,7 +3084,7 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
 		ctrl->cntlid = le16_to_cpu(id->cntlid);
 
 	if (!ctrl->identified) {
-		int i;
+		unsigned int i;
 
 		ret = nvme_init_subsystem(ctrl, id);
 		if (ret)
@@ -3211,16 +3197,43 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
 	}
 
 	ret = nvme_mpath_init(ctrl, id);
-	kfree(id);
-
 	if (ret < 0)
-		return ret;
+		goto out_free;
 
 	if (ctrl->apst_enabled && !prev_apst_enabled)
 		dev_pm_qos_expose_latency_tolerance(ctrl->device);
 	else if (!ctrl->apst_enabled && prev_apst_enabled)
 		dev_pm_qos_hide_latency_tolerance(ctrl->device);
 
+out_free:
+	kfree(id);
+	return ret;
+}
+
+/*
+ * Initialize the cached copies of the Identify data and various controller
+ * register in our nvme_ctrl structure.  This should be called as soon as
+ * the admin queue is fully up and running.
+ */
+int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
+{
+	int ret;
+
+	ret = ctrl->ops->reg_read32(ctrl, NVME_REG_VS, &ctrl->vs);
+	if (ret) {
+		dev_err(ctrl->device, "Reading VS failed (%d)\n", ret);
+		return ret;
+	}
+
+	ctrl->sqsize = min_t(u16, NVME_CAP_MQES(ctrl->cap), ctrl->sqsize);
+
+	if (ctrl->vs >= NVME_VS(1, 1, 0))
+		ctrl->subsystem = NVME_CAP_NSSRC(ctrl->cap);
+
+	ret = nvme_init_identify(ctrl);
+	if (ret)
+		return ret;
+
 	ret = nvme_configure_apst(ctrl);
 	if (ret < 0)
 		return ret;
@@ -3246,10 +3259,6 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
 	ctrl->identified = true;
 
 	return 0;
-
-out_free:
-	kfree(id);
-	return ret;
 }
 EXPORT_SYMBOL_GPL(nvme_init_ctrl_finish);
 
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V2 3/8] nvme-core: mark nvme_setup_passsthru() inline
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
  2021-03-01  2:06 ` [PATCH V2 1/8] nvme-core: rename nvme_init_identify() Chaitanya Kulkarni
  2021-03-01  2:06 ` [PATCH V2 2/8] nvme-core: split init identify into helper Chaitanya Kulkarni
@ 2021-03-01  2:06 ` Chaitanya Kulkarni
  2021-03-01  2:06 ` [PATCH V2 4/8] nvme-core: use likely in nvme_init_request() Chaitanya Kulkarni
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

Since nvmet_setup_passthru() function falls in fast path when called
from the NVMeOF passthru backend, make it inline.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 379ebb54f5dd..484866e52ba7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -725,7 +725,7 @@ static void nvme_assign_write_stream(struct nvme_ctrl *ctrl,
 		req->q->write_hints[streamid] += blk_rq_bytes(req) >> 9;
 }
 
-static void nvme_setup_passthrough(struct request *req,
+static inline void nvme_setup_passthrough(struct request *req,
 		struct nvme_command *cmd)
 {
 	memcpy(cmd, nvme_req(req)->cmd, sizeof(*cmd));
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V2 4/8] nvme-core: use likely in nvme_init_request()
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
                   ` (2 preceding siblings ...)
  2021-03-01  2:06 ` [PATCH V2 3/8] nvme-core: mark nvme_setup_passsthru() inline Chaitanya Kulkarni
@ 2021-03-01  2:06 ` Chaitanya Kulkarni
  2021-03-09 10:07   ` Christoph Hellwig
  2021-03-01  2:06 ` [PATCH V2 5/8] nvme-core: don't check nvme_req flags for new req Chaitanya Kulkarni
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

For NVMeOF Target passthru backend we allocate the passthru request with
nvme_alloc_request(). The functions :-

nvmet_passthru_execute_cmd()
 nvme_alloc_request()
  nvme_init_request()

are in the fast path for I/O commands.

In nvme_init_request() we set the timeout based on the
req->q->queuedata check & that is always true for the I/O commands
coming from the passthru backend which are high frequency commands.

Annotate req->q->queuedata check with likely() in nvme_init_request().

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 484866e52ba7..dab18472e39f 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -589,7 +589,7 @@ static inline unsigned int nvme_req_op(struct nvme_command *cmd)
 static inline void nvme_init_request(struct request *req,
 		struct nvme_command *cmd)
 {
-	if (req->q->queuedata)
+	if (likely(req->q->queuedata))
 		req->timeout = NVME_IO_TIMEOUT;
 	else /* no queuedata implies admin queue */
 		req->timeout = NVME_ADMIN_TIMEOUT;
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V2 5/8] nvme-core: don't check nvme_req flags for new req
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
                   ` (3 preceding siblings ...)
  2021-03-01  2:06 ` [PATCH V2 4/8] nvme-core: use likely in nvme_init_request() Chaitanya Kulkarni
@ 2021-03-01  2:06 ` Chaitanya Kulkarni
  2021-03-01  2:06 ` [PATCH V2 6/8] nvme-fc: fix the function documentation comment Chaitanya Kulkarni
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

nvme_clear_request() has a check for flag REQ_DONTPREP and it is called
from nvme_init_request() and nvme_setuo_cmd().

The function nvme_init_request() is called from nvme_alloc_request()
and nvme_alloc_request_qid(). From these two callers new request is
allocated everytime. For newly allocated request RQF_DONTPREP is never
set. Since after getting a tag, block layer sets the req->rq_flags == 0
and never sets the REQ_DONTPREP when returning the request :-

nvme_alloc_request()
	blk_mq_alloc_request()
		blk_mq_rq_ctx_init()
			rq->rq_flags = 0 <----

nvme_alloc_request_qid()
	blk_mq_alloc_request_hctx()
		blk_mq_rq_ctx_init()
			rq->rq_flags = 0 <----

The block layer does set req->rq_flags but REQ_DONTPREP is not one of
them and that is set by the driver.

That means we can unconditinally set the REQ_DONTPREP value to the
rq->rq_flags when nvme_init_request()->nvme_clear_request() is called
from above two callers.

Move the check for REQ_DONTPREP from nvme_clear_nvme_request() into
nvme_setup_cmd().

This is needed since nvme_alloc_request() now gets called from fast
path when NVMeOF target is configured with passthru backend to avoid
unnecessary checks in the fast path.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dab18472e39f..4858114f4f8a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -574,11 +574,9 @@ EXPORT_SYMBOL_NS_GPL(nvme_put_ns, NVME_TARGET_PASSTHRU);
 
 static inline void nvme_clear_nvme_request(struct request *req)
 {
-	if (!(req->rq_flags & RQF_DONTPREP)) {
-		nvme_req(req)->retries = 0;
-		nvme_req(req)->flags = 0;
-		req->rq_flags |= RQF_DONTPREP;
-	}
+	nvme_req(req)->retries = 0;
+	nvme_req(req)->flags = 0;
+	req->rq_flags |= RQF_DONTPREP;
 }
 
 static inline unsigned int nvme_req_op(struct nvme_command *cmd)
@@ -892,7 +890,8 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req,
 {
 	blk_status_t ret = BLK_STS_OK;
 
-	nvme_clear_nvme_request(req);
+	if (!(req->rq_flags & RQF_DONTPREP))
+		nvme_clear_nvme_request(req);
 
 	memset(cmd, 0, sizeof(*cmd));
 	switch (req_op(req)) {
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V2 6/8] nvme-fc: fix the function documentation comment
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
                   ` (4 preceding siblings ...)
  2021-03-01  2:06 ` [PATCH V2 5/8] nvme-core: don't check nvme_req flags for new req Chaitanya Kulkarni
@ 2021-03-01  2:06 ` Chaitanya Kulkarni
  2021-03-01 19:09   ` James Smart
  2021-03-01  2:06 ` [PATCH V2 7/8] nvmet-fc: update function documentation Chaitanya Kulkarni
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

The nvme_fc_rcv_ls_req() function has first argument as pointer to
remoteport named portprt, but in the documentation comment that is name
is used as remoteport. Fix that to get rid if the compilation warning.

drivers/nvme//host/fc.c:1724: warning: Function parameter or member 'portptr' not described in 'nvme_fc_rcv_ls_req'
drivers/nvme//host/fc.c:1724: warning: Excess function parameter 'remoteport' description in 'nvme_fc_rcv_ls_req'

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/fc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 962987a330d6..ad6be2263a62 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1708,7 +1708,7 @@ nvme_fc_handle_ls_rqst_work(struct work_struct *work)
  *
  * If this routine returns error, the LLDD should abort the exchange.
  *
- * @remoteport: pointer to the (registered) remote port that the LS
+ * @portptr:    pointer to the (registered) remote port that the LS
  *              was received from. The remoteport is associated with
  *              a specific localport.
  * @lsrsp:      pointer to a nvmefc_ls_rsp response structure to be
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V2 7/8] nvmet-fc: update function documentation
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
                   ` (5 preceding siblings ...)
  2021-03-01  2:06 ` [PATCH V2 6/8] nvme-fc: fix the function documentation comment Chaitanya Kulkarni
@ 2021-03-01  2:06 ` Chaitanya Kulkarni
  2021-03-01 19:10   ` James Smart
  2021-03-01  2:06 ` [PATCH V2 8/8] nvme-core: add new line after variable declatation Chaitanya Kulkarni
  2021-03-09 10:09 ` [PATCH V2 0/8] nvme: few cleanups and small improvements Christoph Hellwig
  8 siblings, 1 reply; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

Add minimum description of the hosthandle parameter for
nvmet_fc_rcv_ls_req() so that we can get rid of the following warning.

drivers/nvme//target/fc.c:2009: warning: Function parameter or member 'hosthandle' not described in 'nvmet_fc_rcv_ls_req

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/fc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index d375745fc4ed..1f1c70f9f8eb 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1996,6 +1996,7 @@ nvmet_fc_handle_ls_rqst_work(struct work_struct *work)
  *
  * @target_port: pointer to the (registered) target port the LS was
  *              received on.
+ * @hosthandle: pointer to the host specific data, gets stored in iod.
  * @lsrsp:      pointer to a lsrsp structure to be used to reference
  *              the exchange corresponding to the LS.
  * @lsreqbuf:   pointer to the buffer containing the LS Request
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V2 8/8] nvme-core: add new line after variable declatation
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
                   ` (6 preceding siblings ...)
  2021-03-01  2:06 ` [PATCH V2 7/8] nvmet-fc: update function documentation Chaitanya Kulkarni
@ 2021-03-01  2:06 ` Chaitanya Kulkarni
  2021-03-09 10:09 ` [PATCH V2 0/8] nvme: few cleanups and small improvements Christoph Hellwig
  8 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-01  2:06 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, hch, Chaitanya Kulkarni, james.smart

Add a new line in functions nvme_pr_preempt(), nvme_pr_clear(), and
nvme_pr_release() after variable declaration which follows the rest of
the code in the nvme/host/core.c.

No functional change(s) in this patch.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 4858114f4f8a..472fd39a8286 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2323,18 +2323,21 @@ static int nvme_pr_preempt(struct block_device *bdev, u64 old, u64 new,
 		enum pr_type type, bool abort)
 {
 	u32 cdw10 = nvme_pr_type(type) << 8 | (abort ? 2 : 1);
+
 	return nvme_pr_command(bdev, cdw10, old, new, nvme_cmd_resv_acquire);
 }
 
 static int nvme_pr_clear(struct block_device *bdev, u64 key)
 {
 	u32 cdw10 = 1 | (key ? 1 << 3 : 0);
+
 	return nvme_pr_command(bdev, cdw10, key, 0, nvme_cmd_resv_register);
 }
 
 static int nvme_pr_release(struct block_device *bdev, u64 key, enum pr_type type)
 {
 	u32 cdw10 = nvme_pr_type(type) << 8 | (key ? 1 << 3 : 0);
+
 	return nvme_pr_command(bdev, cdw10, key, 0, nvme_cmd_resv_release);
 }
 
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH V2 6/8] nvme-fc: fix the function documentation comment
  2021-03-01  2:06 ` [PATCH V2 6/8] nvme-fc: fix the function documentation comment Chaitanya Kulkarni
@ 2021-03-01 19:09   ` James Smart
  0 siblings, 0 replies; 15+ messages in thread
From: James Smart @ 2021-03-01 19:09 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-nvme; +Cc: kbusch, james.smart, sagi, hch

On 2/28/2021 6:06 PM, Chaitanya Kulkarni wrote:
> The nvme_fc_rcv_ls_req() function has first argument as pointer to
> remoteport named portprt, but in the documentation comment that is name
> is used as remoteport. Fix that to get rid if the compilation warning.
> 
> drivers/nvme//host/fc.c:1724: warning: Function parameter or member 'portptr' not described in 'nvme_fc_rcv_ls_req'
> drivers/nvme//host/fc.c:1724: warning: Excess function parameter 'remoteport' description in 'nvme_fc_rcv_ls_req'
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
>

Thanks

Reviewed-by:  James Smart <jsmart2021@gmail.com>

-- james

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V2 7/8] nvmet-fc: update function documentation
  2021-03-01  2:06 ` [PATCH V2 7/8] nvmet-fc: update function documentation Chaitanya Kulkarni
@ 2021-03-01 19:10   ` James Smart
  0 siblings, 0 replies; 15+ messages in thread
From: James Smart @ 2021-03-01 19:10 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-nvme; +Cc: kbusch, james.smart, sagi, hch

On 2/28/2021 6:06 PM, Chaitanya Kulkarni wrote:
> Add minimum description of the hosthandle parameter for
> nvmet_fc_rcv_ls_req() so that we can get rid of the following warning.
> 
> drivers/nvme//target/fc.c:2009: warning: Function parameter or member 'hosthandle' not described in 'nvmet_fc_rcv_ls_req
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>

Thanks

Reviewed-by: James Smart <jsmart2021@gmail.com>

-- james


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V2 4/8] nvme-core: use likely in nvme_init_request()
  2021-03-01  2:06 ` [PATCH V2 4/8] nvme-core: use likely in nvme_init_request() Chaitanya Kulkarni
@ 2021-03-09 10:07   ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2021-03-09 10:07 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-nvme, kbusch, hch, james.smart, sagi

How is a likely for a trivial variable assignment ever going to make
a difference?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V2 2/8] nvme-core: split init identify into helper
  2021-03-01  2:06 ` [PATCH V2 2/8] nvme-core: split init identify into helper Chaitanya Kulkarni
@ 2021-03-09 10:09   ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2021-03-09 10:09 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-nvme, kbusch, hch, james.smart, sagi

I've applied this as-is for now, but here is something that I think we
should fix up:

>  	if (ctrl->apst_enabled && !prev_apst_enabled)
>  		dev_pm_qos_expose_latency_tolerance(ctrl->device);
>  	else if (!ctrl->apst_enabled && prev_apst_enabled)
>  		dev_pm_qos_hide_latency_tolerance(ctrl->device);

I don't think this belongs into init_identify, as it has nothing to do
with the identify data.

> +	ret = nvme_init_identify(ctrl);
> +	if (ret)
> +		return ret;
> +
>  	ret = nvme_configure_apst(ctrl);
>  	if (ret < 0)
>  		return ret;

OTOH it would make a whole lot sense in nvme_configure_apst.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V2 0/8] nvme: few cleanups and small improvements
  2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
                   ` (7 preceding siblings ...)
  2021-03-01  2:06 ` [PATCH V2 8/8] nvme-core: add new line after variable declatation Chaitanya Kulkarni
@ 2021-03-09 10:09 ` Christoph Hellwig
  2021-03-09 19:42   ` Chaitanya Kulkarni
  8 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2021-03-09 10:09 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-nvme, kbusch, hch, james.smart, sagi

Applied to nvme-5.13 except for the one likely patch.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V2 0/8] nvme: few cleanups and small improvements
  2021-03-09 10:09 ` [PATCH V2 0/8] nvme: few cleanups and small improvements Christoph Hellwig
@ 2021-03-09 19:42   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-09 19:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, kbusch, james.smart, sagi

On 3/9/21 02:09, Christoph Hellwig wrote:
> Applied to nvme-5.13 except for the one likely patch
Okay.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-03-09 19:43 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01  2:06 [PATCH V2 0/8] nvme: few cleanups and small improvements Chaitanya Kulkarni
2021-03-01  2:06 ` [PATCH V2 1/8] nvme-core: rename nvme_init_identify() Chaitanya Kulkarni
2021-03-01  2:06 ` [PATCH V2 2/8] nvme-core: split init identify into helper Chaitanya Kulkarni
2021-03-09 10:09   ` Christoph Hellwig
2021-03-01  2:06 ` [PATCH V2 3/8] nvme-core: mark nvme_setup_passsthru() inline Chaitanya Kulkarni
2021-03-01  2:06 ` [PATCH V2 4/8] nvme-core: use likely in nvme_init_request() Chaitanya Kulkarni
2021-03-09 10:07   ` Christoph Hellwig
2021-03-01  2:06 ` [PATCH V2 5/8] nvme-core: don't check nvme_req flags for new req Chaitanya Kulkarni
2021-03-01  2:06 ` [PATCH V2 6/8] nvme-fc: fix the function documentation comment Chaitanya Kulkarni
2021-03-01 19:09   ` James Smart
2021-03-01  2:06 ` [PATCH V2 7/8] nvmet-fc: update function documentation Chaitanya Kulkarni
2021-03-01 19:10   ` James Smart
2021-03-01  2:06 ` [PATCH V2 8/8] nvme-core: add new line after variable declatation Chaitanya Kulkarni
2021-03-09 10:09 ` [PATCH V2 0/8] nvme: few cleanups and small improvements Christoph Hellwig
2021-03-09 19:42   ` Chaitanya Kulkarni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).