[PATCH 00/14] nvme: few cleanups and small improvements

[PATCH 00/14] nvme: few cleanups and small improvements

[Chaitanya Kulkarni]

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

Chaitanya Kulkarni (14):
  nvme-core: remove duplicate kfree in init identify
  nvme-core: don't use switch for only one case use
  nvme-core: use right type for ARRAY_SIZE check
  nvme-core: fix the type for shutdown_timeout
  nvme-core: add helper to init ctrl transport attr
  nvme-core: add helper to init shutdown timeout
  nvme-core: add helper to init ctrl subsys quirk
  nvme-core: move util quirk at the top of the file
  nvme-core: mark nvme_setup_passsthru() inline
  nvme-core: remove one liner wrappers for streams
  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

 drivers/nvme/host/core.c | 333 ++++++++++++++++++++-------------------
 drivers/nvme/host/fc.c   |   2 +-
 drivers/nvme/target/fc.c |   1 +
 3 files changed, 172 insertions(+), 164 deletions(-)

-- 
2.22.1


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

[PATCH 01/14] nvme-core: remove duplicate kfree in init identify

[Chaitanya Kulkarni]

In nvme_init_identify() once we initialize nvme_mpath_init() we free
the id with kfree(). This is needed since we just return from all the
following calls to nvme_mpath_init(), that also makes kfree() call
duplicate.

Instead of returning after nvme_mpath_init() jump to the lable
out_free and remove the duplicate call to the kfree() since out_free
label already has one..

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d77f3f26d8d3..70042e111efe 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3209,10 +3209,8 @@ int nvme_init_identify(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);
@@ -3221,29 +3219,29 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 
 	ret = nvme_configure_apst(ctrl);
 	if (ret < 0)
-		return ret;
+		goto out_free;
 	
 	ret = nvme_configure_timestamp(ctrl);
 	if (ret < 0)
-		return ret;
+		goto out_free;
 
 	ret = nvme_configure_directives(ctrl);
 	if (ret < 0)
-		return ret;
+		goto out_free;
 
 	ret = nvme_configure_acre(ctrl);
 	if (ret < 0)
-		return ret;
+		goto out_free;
 
 	if (!ctrl->identified && !nvme_discovery_ctrl(ctrl)) {
 		ret = nvme_hwmon_init(ctrl);
 		if (ret < 0)
-			return ret;
+			goto out_free;
 	}
 
 	ctrl->identified = true;
 
-	return 0;
+	ret = 0;
 
 out_free:
 	kfree(id);
-- 
2.22.1


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

Re: [PATCH 01/14] nvme-core: remove duplicate kfree in init identify

[Christoph Hellwig]

On Tue, Feb 16, 2021 at 04:10:19PM -0800, Chaitanya Kulkarni wrote:
> In nvme_init_identify() once we initialize nvme_mpath_init() we free
> the id with kfree(). This is needed since we just return from all the
> following calls to nvme_mpath_init(), that also makes kfree() call
> duplicate.
> 
> Instead of returning after nvme_mpath_init() jump to the lable
> out_free and remove the duplicate call to the kfree() since out_free
> label already has one..

Hmm.  I'd rather split nvme_init_identify.  Rename the publically
called function to something like nvme_init_ctrl_finish, and
the split out a nvme_init_identify for just the section that deals
with the id_ns data.

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

Re: [PATCH 01/14] nvme-core: remove duplicate kfree in init identify

[Chaitanya Kulkarni]

On 2/24/21 08:35, Christoph Hellwig wrote:
> On Tue, Feb 16, 2021 at 04:10:19PM -0800, Chaitanya Kulkarni wrote:
>> In nvme_init_identify() once we initialize nvme_mpath_init() we free
>> the id with kfree(). This is needed since we just return from all the
>> following calls to nvme_mpath_init(), that also makes kfree() call
>> duplicate.
>>
>> Instead of returning after nvme_mpath_init() jump to the lable
>> out_free and remove the duplicate call to the kfree() since out_free
>> label already has one..
> Hmm.  I'd rather split nvme_init_identify.  Rename the publically
> called function to something like nvme_init_ctrl_finish, and
> the split out a nvme_init_identify for just the section that deals
> with the id_ns data.
>

How about following patch in [1] ? If you like [1] please suggest
the patch distribution. I'd add a prep patch to rename and second patch
to split the helper.

Shorten the original list to avoid churn, along with the documentation fixes
planning to keep following in V2, is that okay:-

  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


[1] nvme-core: split out id_ns into helper

From 4b4ec36b0c660fc54e22aa4d712db1b595b2e90d Mon Sep 17 00:00:00 2001
From: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Date: Wed, 24 Feb 2021 14:14:05 -0800
Subject: [PATCH] nvme-core: split out id_ns into helper

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e68a8c4ac5a6..68b0022bc60d 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;
@@ -3059,28 +3059,12 @@ 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_identify(struct nvme_ctrl *ctrl)
 {
     struct nvme_id_ctrl *id;
+    bool prev_apst_enabled;
     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 +3082,7 @@ int nvme_init_identify(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)
@@ -3136,6 +3120,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 
     atomic_set(&ctrl->abort_limit, id->acl + 1);
     ctrl->vwc = id->vwc;
+    page_shift = NVME_CAP_MPSMIN(ctrl->cap) + 12;
     if (id->mdts)
         max_hw_sectors = 1 << (id->mdts + page_shift - 9);
     else
@@ -3210,16 +3195,40 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
         ctrl->hmmaxd = le16_to_cpu(id->hmmaxd);
     }
 
+    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);
+
     ret = nvme_mpath_init(ctrl, id);
+
+out_free:
     kfree(id);
+    return ret;
+}
 
-    if (ret < 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)
+{
+    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->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);
+    if (ctrl->vs >= NVME_VS(1, 1, 0))
+        ctrl->subsystem = NVME_CAP_NSSRC(ctrl->cap);
+
+    ret = nvme_init_ctrl_finish(ctrl);
+    if (ret < 0)
+        return ret;
 
     ret = nvme_configure_apst(ctrl);
     if (ret < 0)
@@ -3246,12 +3255,8 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
     ctrl->identified = true;
 
     return 0;
-
-out_free:
-    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

[PATCH 02/14] nvme-core: don't use switch for only one case use

[Chaitanya Kulkarni]

There is no point in using switch case statement for only one case that
takes about 5 lines.

Remove switch case which only checks for the NVME_CTRL_LIVE state and
use simple if statement that only takes one line.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 70042e111efe..2f6b89f6309d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3254,12 +3254,8 @@ static int nvme_dev_open(struct inode *inode, struct file *file)
 	struct nvme_ctrl *ctrl =
 		container_of(inode->i_cdev, struct nvme_ctrl, cdev);
 
-	switch (ctrl->state) {
-	case NVME_CTRL_LIVE:
-		break;
-	default:
+	if (ctrl->state != NVME_CTRL_LIVE)
 		return -EWOULDBLOCK;
-	}
 
 	nvme_get_ctrl(ctrl);
 	if (!try_module_get(ctrl->ops->module)) {
-- 
2.22.1


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

[PATCH 03/14] nvme-core: use right type for ARRAY_SIZE check

[Chaitanya Kulkarni]

Use unsigend int value when comparing the values of the expression
that involves sizeof() in the macro ARRAY_SIZE() that returns
size_t.

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 2f6b89f6309d..cae526757e03 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3096,7 +3096,7 @@ int nvme_init_identify(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)
-- 
2.22.1


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

[PATCH 04/14] nvme-core: fix the type for shutdown_timeout

[Chaitanya Kulkarni]

In the function nvme_init_identify() we calculate the
ctrl->shutdown_timeout value with module parameter shutdown_timeout
and transition_timeout with clamp_t().

The variable ctrl->shutdown_timeout and transition_timeout is of type
unsigned int.

Change the module parameter shutdown_timeout from unsigned char to
unsigned int so it stays consistent with the type of local variable
transition_timeout, ctrl->shutdown_timeout & clamp_t unsigned int type.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index cae526757e03..840c32c755b2 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -40,8 +40,8 @@ module_param_named(io_timeout, nvme_io_timeout, uint, 0644);
 MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
 EXPORT_SYMBOL_GPL(nvme_io_timeout);
 
-static unsigned char shutdown_timeout = 5;
-module_param(shutdown_timeout, byte, 0644);
+static unsigned int shutdown_timeout = 5;
+module_param(shutdown_timeout, uint, 0644);
 MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
 
 static u8 nvme_max_retries = 5;
-- 
2.22.1


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

[PATCH 05/14] nvme-core: add helper to init ctrl transport attr

[Chaitanya Kulkarni]

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

Just like the tail of the function it has small helpers to initialize
the independent fields that needs some extra checking, add a new helper
nvme_init_identify_transport() to initialize transport-related fields.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 840c32c755b2..f5b8f8e0ee18 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3057,6 +3057,43 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
 	return 0;
 }
 
+static int nvme_init_identify_transport(struct nvme_ctrl *ctrl,
+		struct nvme_id_ctrl *id)
+{
+	if (ctrl->ops->flags & NVME_F_FABRICS) {
+		ctrl->icdoff = le16_to_cpu(id->icdoff);
+		ctrl->ioccsz = le32_to_cpu(id->ioccsz);
+		ctrl->iorcsz = le32_to_cpu(id->iorcsz);
+		ctrl->maxcmd = le16_to_cpu(id->maxcmd);
+
+		/*
+		 * In fabrics we need to verify the cntlid matches the
+		 * admin connect
+		 */
+		if (ctrl->cntlid != le16_to_cpu(id->cntlid)) {
+			dev_err(ctrl->device,
+				"Mismatching cntlid: Connect %u vs Identify "
+				"%u, rejecting\n",
+				ctrl->cntlid, le16_to_cpu(id->cntlid));
+			return -EINVAL;
+		}
+
+		if (!nvme_discovery_ctrl(ctrl) && !ctrl->kas) {
+			dev_err(ctrl->device,
+				"keep-alive support is mandatory for fabrics\n");
+			return -EINVAL;
+		}
+		return 0;
+	}
+
+	ctrl->hmpre = le32_to_cpu(id->hmpre);
+	ctrl->hmmin = le32_to_cpu(id->hmmin);
+	ctrl->hmminds = le32_to_cpu(id->hmminds);
+	ctrl->hmmaxd = le16_to_cpu(id->hmmaxd);
+
+	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
@@ -3176,37 +3213,9 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 	}
 	memcpy(ctrl->psd, id->psd, sizeof(ctrl->psd));
 
-	if (ctrl->ops->flags & NVME_F_FABRICS) {
-		ctrl->icdoff = le16_to_cpu(id->icdoff);
-		ctrl->ioccsz = le32_to_cpu(id->ioccsz);
-		ctrl->iorcsz = le32_to_cpu(id->iorcsz);
-		ctrl->maxcmd = le16_to_cpu(id->maxcmd);
-
-		/*
-		 * In fabrics we need to verify the cntlid matches the
-		 * admin connect
-		 */
-		if (ctrl->cntlid != le16_to_cpu(id->cntlid)) {
-			dev_err(ctrl->device,
-				"Mismatching cntlid: Connect %u vs Identify "
-				"%u, rejecting\n",
-				ctrl->cntlid, le16_to_cpu(id->cntlid));
-			ret = -EINVAL;
-			goto out_free;
-		}
-
-		if (!nvme_discovery_ctrl(ctrl) && !ctrl->kas) {
-			dev_err(ctrl->device,
-				"keep-alive support is mandatory for fabrics\n");
-			ret = -EINVAL;
-			goto out_free;
-		}
-	} else {
-		ctrl->hmpre = le32_to_cpu(id->hmpre);
-		ctrl->hmmin = le32_to_cpu(id->hmmin);
-		ctrl->hmminds = le32_to_cpu(id->hmminds);
-		ctrl->hmmaxd = le16_to_cpu(id->hmmaxd);
-	}
+	ret = nvme_init_identify_transport(ctrl, id);
+	if (ret < 0)
+		goto out_free;
 
 	ret = nvme_mpath_init(ctrl, id);
 	if (ret < 0)
-- 
2.22.1


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

[PATCH 06/14] nvme-core: add helper to init shutdown timeout

[Chaitanya Kulkarni]

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

Just like the tail of the function it has small helpers to initialize
the independent fields that needs some extra checking, add a new helper
nvme_init_shutdown_timeout() to initialize ctrl->shutdown_timeout.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f5b8f8e0ee18..77f79a2a396f 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3094,6 +3094,24 @@ static int nvme_init_identify_transport(struct nvme_ctrl *ctrl,
 	return 0;
 }
 
+static void nvme_init_shutdown_timeout(struct nvme_ctrl *ctrl,
+		struct nvme_id_ctrl *id)
+{
+	if (id->rtd3e) {
+		/* us -> s */
+		u32 transition_time = le32_to_cpu(id->rtd3e) / USEC_PER_SEC;
+
+		ctrl->shutdown_timeout = clamp_t(unsigned int, transition_time,
+						 shutdown_timeout, 60);
+
+		if (ctrl->shutdown_timeout != shutdown_timeout)
+			dev_info(ctrl->device,
+				 "Shutdown timeout set to %u seconds\n",
+				 ctrl->shutdown_timeout);
+	} else
+		ctrl->shutdown_timeout = shutdown_timeout;
+}
+
 /*
  * Initialize the cached copies of the Identify data and various controller
  * register in our nvme_ctrl structure.  This should be called as soon as
@@ -3184,19 +3202,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 	ctrl->max_namespaces = le32_to_cpu(id->mnan);
 	ctrl->ctratt = le32_to_cpu(id->ctratt);
 
-	if (id->rtd3e) {
-		/* us -> s */
-		u32 transition_time = le32_to_cpu(id->rtd3e) / USEC_PER_SEC;
-
-		ctrl->shutdown_timeout = clamp_t(unsigned int, transition_time,
-						 shutdown_timeout, 60);
-
-		if (ctrl->shutdown_timeout != shutdown_timeout)
-			dev_info(ctrl->device,
-				 "Shutdown timeout set to %u seconds\n",
-				 ctrl->shutdown_timeout);
-	} else
-		ctrl->shutdown_timeout = shutdown_timeout;
+	nvme_init_shutdown_timeout(ctrl, id);
 
 	ctrl->npss = id->npss;
 	ctrl->apsta = id->apsta;
-- 
2.22.1


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

[PATCH 07/14] nvme-core: add helper to init ctrl subsys quirk

[Chaitanya Kulkarni]

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

Just like the tail of the function it has small helpers to initialize
the independent fields that needs some extra checking, add a new helper
nvme_init_ctrl_subsys_and_quirks() to initialize ctrl subsystem and
quirks.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 77f79a2a396f..4a59a46193e8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3057,6 +3057,32 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
 	return 0;
 }
 
+static int nvme_init_ctrl_subsys_and_quirks(struct nvme_ctrl *ctrl,
+		struct nvme_id_ctrl *id)
+{
+	unsigned int i;
+	int ret;
+
+	ret = nvme_init_subsystem(ctrl, id);
+	if (ret)
+		return ret;
+
+	/*
+	 * Check for quirks.  Quirk can depend on firmware version,
+	 * so, in principle, the set of quirks present can change
+	 * across a reset.  As a possible future enhancement, we
+	 * could re-scan for quirks every time we reinitialize
+	 * the device, but we'd have to make sure that the driver
+	 * behaves intelligently if the quirks change.
+	 */
+	for (i = 0; i < ARRAY_SIZE(core_quirks); i++) {
+		if (quirk_matches(id, &core_quirks[i]))
+			ctrl->quirks |= core_quirks[i].quirks;
+	}
+
+	return 0;
+}
+
 static int nvme_init_identify_transport(struct nvme_ctrl *ctrl,
 		struct nvme_id_ctrl *id)
 {
@@ -3151,24 +3177,9 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 		ctrl->cntlid = le16_to_cpu(id->cntlid);
 
 	if (!ctrl->identified) {
-		unsigned int i;
-
-		ret = nvme_init_subsystem(ctrl, id);
+		ret = nvme_init_ctrl_subsys_and_quirks(ctrl, id);
 		if (ret)
 			goto out_free;
-
-		/*
-		 * Check for quirks.  Quirk can depend on firmware version,
-		 * so, in principle, the set of quirks present can change
-		 * across a reset.  As a possible future enhancement, we
-		 * could re-scan for quirks every time we reinitialize
-		 * the device, but we'd have to make sure that the driver
-		 * behaves intelligently if the quirks change.
-		 */
-		for (i = 0; i < ARRAY_SIZE(core_quirks); i++) {
-			if (quirk_matches(id, &core_quirks[i]))
-				ctrl->quirks |= core_quirks[i].quirks;
-		}
 	}
 
 	if (force_apst && (ctrl->quirks & NVME_QUIRK_NO_DEEPEST_PS)) {
-- 
2.22.1


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

[PATCH 08/14] nvme-core: move util quirk at the top of the file

[Chaitanya Kulkarni]

The quirk utility functions and structure are defined in the middle of
the file (line:2762) & they are not accessed immediately after their
declaration until we get to nvme_init_identify() (line:3114).

Move quirk related util functions at the top of the file that avoids
any global structure related definitions in the middle of the code
where they are not accessed immediately after definition.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 4a59a46193e8..5cb90bd8704d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -93,6 +93,69 @@ static void nvme_put_subsystem(struct nvme_subsystem *subsys);
 static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
 					   unsigned nsid);
 
+struct nvme_core_quirk_entry {
+	/*
+	 * NVMe model and firmware strings are padded with spaces.  For
+	 * simplicity, strings in the quirk table are padded with NULLs
+	 * instead.
+	 */
+	u16 vid;
+	const char *mn;
+	const char *fr;
+	unsigned long quirks;
+};
+
+static const struct nvme_core_quirk_entry core_quirks[] = {
+	{
+		/*
+		 * This Toshiba device seems to die using any APST states.  See:
+		 * https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1678184/comments/11
+		 */
+		.vid = 0x1179,
+		.mn = "THNSF5256GPUK TOSHIBA",
+		.quirks = NVME_QUIRK_NO_APST,
+	},
+	{
+		/*
+		 * This LiteON CL1-3D*-Q11 firmware version has a race
+		 * condition associated with actions related to suspend to idle
+		 * LiteON has resolved the problem in future firmware
+		 */
+		.vid = 0x14a4,
+		.fr = "22301111",
+		.quirks = NVME_QUIRK_SIMPLE_SUSPEND,
+	}
+};
+
+/* match is null-terminated but idstr is space-padded. */
+static bool string_matches(const char *idstr, const char *match, size_t len)
+{
+	size_t matchlen;
+
+	if (!match)
+		return true;
+
+	matchlen = strlen(match);
+	WARN_ON_ONCE(matchlen > len);
+
+	if (memcmp(idstr, match, matchlen))
+		return false;
+
+	for (; matchlen < len; matchlen++)
+		if (idstr[matchlen] != ' ')
+			return false;
+
+	return true;
+}
+
+static bool quirk_matches(const struct nvme_id_ctrl *id,
+			  const struct nvme_core_quirk_entry *q)
+{
+	return q->vid == le16_to_cpu(id->vid) &&
+		string_matches(id->mn, q->mn, sizeof(id->mn)) &&
+		string_matches(id->fr, q->fr, sizeof(id->fr));
+}
+
 /*
  * Prepare a queue for teardown.
  *
@@ -2704,69 +2767,6 @@ static void nvme_set_latency_tolerance(struct device *dev, s32 val)
 	}
 }
 
-struct nvme_core_quirk_entry {
-	/*
-	 * NVMe model and firmware strings are padded with spaces.  For
-	 * simplicity, strings in the quirk table are padded with NULLs
-	 * instead.
-	 */
-	u16 vid;
-	const char *mn;
-	const char *fr;
-	unsigned long quirks;
-};
-
-static const struct nvme_core_quirk_entry core_quirks[] = {
-	{
-		/*
-		 * This Toshiba device seems to die using any APST states.  See:
-		 * https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1678184/comments/11
-		 */
-		.vid = 0x1179,
-		.mn = "THNSF5256GPUK TOSHIBA",
-		.quirks = NVME_QUIRK_NO_APST,
-	},
-	{
-		/*
-		 * This LiteON CL1-3D*-Q11 firmware version has a race
-		 * condition associated with actions related to suspend to idle
-		 * LiteON has resolved the problem in future firmware
-		 */
-		.vid = 0x14a4,
-		.fr = "22301111",
-		.quirks = NVME_QUIRK_SIMPLE_SUSPEND,
-	}
-};
-
-/* match is null-terminated but idstr is space-padded. */
-static bool string_matches(const char *idstr, const char *match, size_t len)
-{
-	size_t matchlen;
-
-	if (!match)
-		return true;
-
-	matchlen = strlen(match);
-	WARN_ON_ONCE(matchlen > len);
-
-	if (memcmp(idstr, match, matchlen))
-		return false;
-
-	for (; matchlen < len; matchlen++)
-		if (idstr[matchlen] != ' ')
-			return false;
-
-	return true;
-}
-
-static bool quirk_matches(const struct nvme_id_ctrl *id,
-			  const struct nvme_core_quirk_entry *q)
-{
-	return q->vid == le16_to_cpu(id->vid) &&
-		string_matches(id->mn, q->mn, sizeof(id->mn)) &&
-		string_matches(id->fr, q->fr, sizeof(id->fr));
-}
-
 static void nvme_init_subnqn(struct nvme_subsystem *subsys, struct nvme_ctrl *ctrl,
 		struct nvme_id_ctrl *id)
 {
-- 
2.22.1


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

[PATCH 09/14] nvme-core: mark nvme_setup_passsthru() inline

[Chaitanya Kulkarni]

Since nvmet_setup_passthru() function falls in fast path when used with
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 5cb90bd8704d..b43557ebc564 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -788,7 +788,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

[PATCH 10/14] nvme-core: remove one liner wrappers for streams

[Chaitanya Kulkarni]

The functions nvme_[enable|disable]_streams() are just one line wrapper
to the nvme_toggel_streams(). Also, there is only one call exist for
them.

Instead of maintaining one liners, rename original function
nvme_toggle_streams() -> nvme_ctrl_allow_streams() and pass the boolean
value to the function to enable/disable streams. While we are at it
remove the memset() call and zero initialize the structure just like in
nvme_identify_ctrl().

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b43557ebc564..139cbeb11c7d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -686,11 +686,9 @@ static struct request *nvme_alloc_request_qid(struct request_queue *q,
 	return req;
 }
 
-static int nvme_toggle_streams(struct nvme_ctrl *ctrl, bool enable)
+static int nvme_ctrl_allow_streams(struct nvme_ctrl *ctrl, bool enable)
 {
-	struct nvme_command c;
-
-	memset(&c, 0, sizeof(c));
+	struct nvme_command c = { };
 
 	c.directive.opcode = nvme_admin_directive_send;
 	c.directive.nsid = cpu_to_le32(NVME_NSID_ALL);
@@ -702,16 +700,6 @@ static int nvme_toggle_streams(struct nvme_ctrl *ctrl, bool enable)
 	return nvme_submit_sync_cmd(ctrl->admin_q, &c, NULL, 0);
 }
 
-static int nvme_disable_streams(struct nvme_ctrl *ctrl)
-{
-	return nvme_toggle_streams(ctrl, false);
-}
-
-static int nvme_enable_streams(struct nvme_ctrl *ctrl)
-{
-	return nvme_toggle_streams(ctrl, true);
-}
-
 static int nvme_get_stream_params(struct nvme_ctrl *ctrl,
 				  struct streams_directive_params *s, u32 nsid)
 {
@@ -739,7 +727,7 @@ static int nvme_configure_directives(struct nvme_ctrl *ctrl)
 	if (!streams)
 		return 0;
 
-	ret = nvme_enable_streams(ctrl);
+	ret = nvme_ctrl_allow_streams(ctrl, true);
 	if (ret)
 		return ret;
 
@@ -759,7 +747,7 @@ static int nvme_configure_directives(struct nvme_ctrl *ctrl)
 	return 0;
 
 out_disable_stream:
-	nvme_disable_streams(ctrl);
+	nvme_ctrl_allow_streams(ctrl, false);
 	return ret;
 }
 
-- 
2.22.1


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

[PATCH 11/14] nvme-core: use likely in nvme_init_request()

[Chaitanya Kulkarni]

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 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 139cbeb11c7d..a0d0c3a5abff 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -652,7 +652,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

[PATCH 12/14] nvme-core: don't check nvme_req flags for new req

[Chaitanya Kulkarni]

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 a0d0c3a5abff..608462df2c39 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -637,11 +637,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)
@@ -943,7 +941,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

[PATCH 13/14] nvme-fc: fix the function documentation comment

[Chaitanya Kulkarni]

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'
---
 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 20dadd86e981..d894d3eebadf 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

[PATCH 14/14] nvmet-fc: update function documentation

[Chaitanya Kulkarni]

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
---
 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

Re: [PATCH 00/14] nvme: few cleanups and small improvements

[Keith Busch]

On Tue, Feb 16, 2021 at 04:10:18PM -0800, Chaitanya Kulkarni wrote:
> 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.

The two documentation fixes at the end look fine, but much of the rest
looks unnecessary. 
 
> Chaitanya Kulkarni (14):
>   nvme-core: remove duplicate kfree in init identify
>   nvme-core: don't use switch for only one case use
>   nvme-core: use right type for ARRAY_SIZE check
>   nvme-core: fix the type for shutdown_timeout
>   nvme-core: add helper to init ctrl transport attr
>   nvme-core: add helper to init shutdown timeout
>   nvme-core: add helper to init ctrl subsys quirk
>   nvme-core: move util quirk at the top of the file
>   nvme-core: mark nvme_setup_passsthru() inline
>   nvme-core: remove one liner wrappers for streams
>   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
> 
>  drivers/nvme/host/core.c | 333 ++++++++++++++++++++-------------------
>  drivers/nvme/host/fc.c   |   2 +-
>  drivers/nvme/target/fc.c |   1 +
>  3 files changed, 172 insertions(+), 164 deletions(-)
> 
> -- 
> 2.22.1

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

Re: [PATCH 00/14] nvme: few cleanups and small improvements

[Chaitanya Kulkarni]

On 2/24/21 07:42, Keith Busch wrote:
> On Tue, Feb 16, 2021 at 04:10:18PM -0800, Chaitanya Kulkarni wrote:
>> 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.
> The two documentation fixes at the end look fine, but much of the rest
> looks unnecessary. 
>  
Thanks for the feedback Keith.


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