Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* misc nvme cleanups
@ 2020-03-25 13:19 Christoph Hellwig
  2020-03-25 13:19 ` [PATCH 1/3] nvme: refactor nvme_identify_ns_descs error handling Christoph Hellwig
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Christoph Hellwig @ 2020-03-25 13:19 UTC (permalink / raw)
  To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme

Hi Keith and Sagi,

a few trivial cleanups based on some exploration I've done for namespace
types support.

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

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

* [PATCH 1/3] nvme: refactor nvme_identify_ns_descs error handling
  2020-03-25 13:19 misc nvme cleanups Christoph Hellwig
@ 2020-03-25 13:19 ` Christoph Hellwig
  2020-03-25 13:19 ` [PATCH 2/3] nvme: rename __nvme_find_ns_head to nvme_find_ns_head Christoph Hellwig
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2020-03-25 13:19 UTC (permalink / raw)
  To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme

Move the handling of an error into the function from the caller, and
only do it for an actual error on the admin command itself, not the
command parsing, as that should be enough to deal with devices claiming
a bogus version compliance.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/core.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9961d0ec2b91..29716cca3768 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1102,8 +1102,17 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
 
 	status = nvme_submit_sync_cmd(ctrl->admin_q, &c, data,
 				      NVME_IDENTIFY_DATA_SIZE);
-	if (status)
+	if (status) {
+		dev_warn(ctrl->device,
+			"Identify Descriptors failed (%d)\n", status);
+		 /*
+		  * Don't treat an error as fatal, as we potentially already
+		  * have a NGUID or EUI-64.
+		  */
+		if (status > 0)
+			status = 0;
 		goto free_data;
+	}
 
 	for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
 		struct nvme_ns_id_desc *cur = data + pos;
@@ -1757,26 +1766,15 @@ static void nvme_config_write_zeroes(struct gendisk *disk, struct nvme_ns *ns)
 static int nvme_report_ns_ids(struct nvme_ctrl *ctrl, unsigned int nsid,
 		struct nvme_id_ns *id, struct nvme_ns_ids *ids)
 {
-	int ret = 0;
-
 	memset(ids, 0, sizeof(*ids));
 
 	if (ctrl->vs >= NVME_VS(1, 1, 0))
 		memcpy(ids->eui64, id->eui64, sizeof(id->eui64));
 	if (ctrl->vs >= NVME_VS(1, 2, 0))
 		memcpy(ids->nguid, id->nguid, sizeof(id->nguid));
-	if (ctrl->vs >= NVME_VS(1, 3, 0)) {
-		 /* Don't treat error as fatal we potentially
-		  * already have a NGUID or EUI-64
-		  */
-		ret = nvme_identify_ns_descs(ctrl, nsid, ids);
-		if (ret)
-			dev_warn(ctrl->device,
-				 "Identify Descriptors failed (%d)\n", ret);
-		if (ret > 0)
-			ret = 0;
-	}
-	return ret;
+	if (ctrl->vs >= NVME_VS(1, 3, 0))
+		return nvme_identify_ns_descs(ctrl, nsid, ids);
+	return 0;
 }
 
 static bool nvme_ns_ids_valid(struct nvme_ns_ids *ids)
-- 
2.25.1


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

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

* [PATCH 2/3] nvme: rename __nvme_find_ns_head to nvme_find_ns_head
  2020-03-25 13:19 misc nvme cleanups Christoph Hellwig
  2020-03-25 13:19 ` [PATCH 1/3] nvme: refactor nvme_identify_ns_descs error handling Christoph Hellwig
@ 2020-03-25 13:19 ` Christoph Hellwig
  2020-03-25 13:19 ` [PATCH 3/3] nvme: cleanup namespace identifier reporting in nvme_init_ns_head Christoph Hellwig
  2020-03-26  9:00 ` misc nvme cleanups Sagi Grimberg
  3 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2020-03-25 13:19 UTC (permalink / raw)
  To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme

There is no non __-prefixed version, so make the name a little more
readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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 29716cca3768..51517368e56a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3357,7 +3357,7 @@ static const struct attribute_group *nvme_dev_attr_groups[] = {
 	NULL,
 };
 
-static struct nvme_ns_head *__nvme_find_ns_head(struct nvme_subsystem *subsys,
+static struct nvme_ns_head *nvme_find_ns_head(struct nvme_subsystem *subsys,
 		unsigned nsid)
 {
 	struct nvme_ns_head *h;
@@ -3457,7 +3457,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
 
 	mutex_lock(&ctrl->subsys->lock);
 	if (is_shared)
-		head = __nvme_find_ns_head(ctrl->subsys, nsid);
+		head = nvme_find_ns_head(ctrl->subsys, nsid);
 	if (!head) {
 		head = nvme_alloc_ns_head(ctrl, nsid, id);
 		if (IS_ERR(head)) {
-- 
2.25.1


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

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

* [PATCH 3/3] nvme: cleanup namespace identifier reporting in nvme_init_ns_head
  2020-03-25 13:19 misc nvme cleanups Christoph Hellwig
  2020-03-25 13:19 ` [PATCH 1/3] nvme: refactor nvme_identify_ns_descs error handling Christoph Hellwig
  2020-03-25 13:19 ` [PATCH 2/3] nvme: rename __nvme_find_ns_head to nvme_find_ns_head Christoph Hellwig
@ 2020-03-25 13:19 ` Christoph Hellwig
  2020-03-25 14:54   ` Keith Busch
  2020-03-26  9:00 ` misc nvme cleanups Sagi Grimberg
  3 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-03-25 13:19 UTC (permalink / raw)
  To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme

Lift the common namespace identifier reporting between the shared
namespace and new nshead cases into common code.  This also means
one less lock is held while doing I/O.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/core.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 51517368e56a..986df944130d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3390,7 +3390,8 @@ static int __nvme_check_ids(struct nvme_subsystem *subsys,
 }
 
 static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
-		unsigned nsid, struct nvme_id_ns *id)
+		unsigned nsid, struct nvme_id_ns *id,
+		struct nvme_ns_ids *ids)
 {
 	struct nvme_ns_head *head;
 	size_t size = sizeof(*head);
@@ -3413,12 +3414,9 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
 		goto out_ida_remove;
 	head->subsys = ctrl->subsys;
 	head->ns_id = nsid;
+	head->ids = *ids;
 	kref_init(&head->ref);
 
-	ret = nvme_report_ns_ids(ctrl, nsid, id, &head->ids);
-	if (ret)
-		goto out_cleanup_srcu;
-
 	ret = __nvme_check_ids(ctrl->subsys, head);
 	if (ret) {
 		dev_err(ctrl->device,
@@ -3453,24 +3451,23 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
 	struct nvme_ctrl *ctrl = ns->ctrl;
 	bool is_shared = id->nmic & (1 << 0);
 	struct nvme_ns_head *head = NULL;
+	struct nvme_ns_ids ids;
 	int ret = 0;
 
+	ret = nvme_report_ns_ids(ctrl, nsid, id, &ids);
+	if (ret)
+		goto out;;
+
 	mutex_lock(&ctrl->subsys->lock);
 	if (is_shared)
 		head = nvme_find_ns_head(ctrl->subsys, nsid);
 	if (!head) {
-		head = nvme_alloc_ns_head(ctrl, nsid, id);
+		head = nvme_alloc_ns_head(ctrl, nsid, id, &ids);
 		if (IS_ERR(head)) {
 			ret = PTR_ERR(head);
 			goto out_unlock;
 		}
 	} else {
-		struct nvme_ns_ids ids;
-
-		ret = nvme_report_ns_ids(ctrl, nsid, id, &ids);
-		if (ret)
-			goto out_unlock;
-
 		if (!nvme_ns_ids_equal(&head->ids, &ids)) {
 			dev_err(ctrl->device,
 				"IDs don't match for shared namespace %d\n",
@@ -3485,6 +3482,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
 
 out_unlock:
 	mutex_unlock(&ctrl->subsys->lock);
+out:
 	if (ret > 0)
 		ret = blk_status_to_errno(nvme_error_status(ret));
 	return ret;
-- 
2.25.1


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

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

* Re: [PATCH 3/3] nvme: cleanup namespace identifier reporting in nvme_init_ns_head
  2020-03-25 13:19 ` [PATCH 3/3] nvme: cleanup namespace identifier reporting in nvme_init_ns_head Christoph Hellwig
@ 2020-03-25 14:54   ` Keith Busch
  0 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2020-03-25 14:54 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, linux-nvme

On Wed, Mar 25, 2020 at 02:19:37PM +0100, Christoph Hellwig wrote:
> +	ret = nvme_report_ns_ids(ctrl, nsid, id, &ids);
> +	if (ret)
> +		goto out;;

I removed the extra ';'.

Otherwise series looks good, queued up for 5.7.

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

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

* Re: misc nvme cleanups
  2020-03-25 13:19 misc nvme cleanups Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-03-25 13:19 ` [PATCH 3/3] nvme: cleanup namespace identifier reporting in nvme_init_ns_head Christoph Hellwig
@ 2020-03-26  9:00 ` Sagi Grimberg
  3 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2020-03-26  9:00 UTC (permalink / raw)
  To: Christoph Hellwig, Keith Busch; +Cc: linux-nvme

Series looks good to me,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25 13:19 misc nvme cleanups Christoph Hellwig
2020-03-25 13:19 ` [PATCH 1/3] nvme: refactor nvme_identify_ns_descs error handling Christoph Hellwig
2020-03-25 13:19 ` [PATCH 2/3] nvme: rename __nvme_find_ns_head to nvme_find_ns_head Christoph Hellwig
2020-03-25 13:19 ` [PATCH 3/3] nvme: cleanup namespace identifier reporting in nvme_init_ns_head Christoph Hellwig
2020-03-25 14:54   ` Keith Busch
2020-03-26  9:00 ` misc nvme cleanups Sagi Grimberg

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git