All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] nvme: nidt and ns-desc related clanup
@ 2021-05-18  5:05 Chaitanya Kulkarni
  2021-05-18  5:05 ` [PATCH 1/4] nvme: calculate data offset once for ns-desc Chaitanya Kulkarni
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Chaitanya Kulkarni @ 2021-05-18  5:05 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, hch, sagi, Chaitanya Kulkarni

Hi,

This is a small patch-series tries to minimize the number of the macro
identifiers. 

In the current code we maintain NVME_NIDT_XXX identifire values and the
their length NVME_NIDT_XXX_LEN separately which has 1:1 mapping. This
adds founr extra macros and open coding of such macros in the host and
the target.

With this open coding in future we will have to maintain one extra macro
representing length for each new NIDT identifire that we will add.

I did the light testing on this with nvme-loop [1].

-ck

Chaitanya Kulkarni (4):
  nvme: calculate data offset once for ns-desc
  nvme-core: add a helper to get the len from nid
  nvmet: use nvme_nidt_len() helper for id-desc
  nvme-core: remove the NVME_NIDT_XXX_LEN macros

 drivers/nvme/host/core.c        | 25 +++++++++++++------------
 drivers/nvme/target/admin-cmd.c |  8 +++++---
 include/linux/nvme.h            | 21 ++++++++++++++++-----
 3 files changed, 34 insertions(+), 20 deletions(-)

# nvme list  | tr -s ' ' ' ' | grep -v '-'
Node SN Model Namespace Usage Format FW Rev 
/dev/nvme0n1 foo QEMU NVMe Ctrl 1 1.07 GB / 1.07 GB 512 B + 0 B 1.0 
/dev/nvme1n1 e3a9ed423c786510 Linux 1 2.15 GB / 2.15 GB 4 KiB + 0 B 5.12.0nv
# cat /sys/kernel/config/nvmet/subsystems/z/namespaces/1/device_nguid 
11111111-2222-3333-4444-000000000000
# cat /sys/kernel/config/nvmet/subsystems/z/namespaces/1/device_uuid 
aeed37e9-39b3-48cc-bc9d-1c799fa56df7
# nvme ns-descs /dev/nvme1n1 
NVME Namespace Identification Descriptors NS 1:
uuid    : aeed37e9-39b3-48cc-bc9d-1c799fa56df7
nguid   : 11111111222233334444000000000000

-- 
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] 7+ messages in thread

* [PATCH 1/4] nvme: calculate data offset once for ns-desc
  2021-05-18  5:05 [PATCH 0/4] nvme: nidt and ns-desc related clanup Chaitanya Kulkarni
@ 2021-05-18  5:05 ` Chaitanya Kulkarni
  2021-05-18  5:05 ` [PATCH 2/4] nvme-core: add a helper to get the len from nid Chaitanya Kulkarni
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Chaitanya Kulkarni @ 2021-05-18  5:05 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, hch, sagi, Chaitanya Kulkarni

Calculate the data buffer offset only once instead of repeating the
calculation for all the nidt values when processing the ns-desc.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 762125f2905f..f8a3a36eba86 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1302,6 +1302,7 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
 {
 	const char *warn_str = "ctrl returned bogus length:";
 	void *data = cur;
+	void *buf = data + sizeof(*cur);
 
 	switch (cur->nidt) {
 	case NVME_NIDT_EUI64:
@@ -1310,7 +1311,7 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
 				 warn_str, cur->nidl);
 			return -1;
 		}
-		memcpy(ids->eui64, data + sizeof(*cur), NVME_NIDT_EUI64_LEN);
+		memcpy(ids->eui64, buf, NVME_NIDT_EUI64_LEN);
 		return NVME_NIDT_EUI64_LEN;
 	case NVME_NIDT_NGUID:
 		if (cur->nidl != NVME_NIDT_NGUID_LEN) {
@@ -1318,7 +1319,7 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
 				 warn_str, cur->nidl);
 			return -1;
 		}
-		memcpy(ids->nguid, data + sizeof(*cur), NVME_NIDT_NGUID_LEN);
+		memcpy(ids->nguid, buf, NVME_NIDT_NGUID_LEN);
 		return NVME_NIDT_NGUID_LEN;
 	case NVME_NIDT_UUID:
 		if (cur->nidl != NVME_NIDT_UUID_LEN) {
@@ -1326,7 +1327,7 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
 				 warn_str, cur->nidl);
 			return -1;
 		}
-		uuid_copy(&ids->uuid, data + sizeof(*cur));
+		uuid_copy(&ids->uuid, buf);
 		return NVME_NIDT_UUID_LEN;
 	case NVME_NIDT_CSI:
 		if (cur->nidl != NVME_NIDT_CSI_LEN) {
@@ -1334,7 +1335,7 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
 				 warn_str, cur->nidl);
 			return -1;
 		}
-		memcpy(&ids->csi, data + sizeof(*cur), NVME_NIDT_CSI_LEN);
+		memcpy(&ids->csi, buf, NVME_NIDT_CSI_LEN);
 		*csi_seen = true;
 		return NVME_NIDT_CSI_LEN;
 	default:
-- 
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] 7+ messages in thread

* [PATCH 2/4] nvme-core: add a helper to get the len from nid
  2021-05-18  5:05 [PATCH 0/4] nvme: nidt and ns-desc related clanup Chaitanya Kulkarni
  2021-05-18  5:05 ` [PATCH 1/4] nvme: calculate data offset once for ns-desc Chaitanya Kulkarni
@ 2021-05-18  5:05 ` Chaitanya Kulkarni
  2021-05-18  5:05 ` [PATCH 3/4] nvmet: use nvme_nidt_len() helper for id-desc Chaitanya Kulkarni
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Chaitanya Kulkarni @ 2021-05-18  5:05 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, hch, sagi, Chaitanya Kulkarni

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3583 bytes --]

In the current code we maintain NVME_NIDT_XXX identifier values and the 
their length NVME_NIDT_XXX_LEN separately which has 1:1 mapping. This
adds four extra macros and open coding of such macros in the host and 
the target.                                                                                            

With this open coding in future we will have to maintain one extra macro
representing length when we add a new NIDT identifier(s).

In this preparation patch add a helper function and use it so that it
can now remove the open coding of the NVME_NIDT_XXX_LEN for host.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/core.c | 22 +++++++++++-----------
 include/linux/nvme.h     | 16 ++++++++++++++++
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f8a3a36eba86..aac33f575f19 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1306,38 +1306,38 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
 
 	switch (cur->nidt) {
 	case NVME_NIDT_EUI64:
-		if (cur->nidl != NVME_NIDT_EUI64_LEN) {
+		if (cur->nidl != nvme_nidt_len(NVME_NIDT_EUI64)) {
 			dev_warn(ctrl->device, "%s %d for NVME_NIDT_EUI64\n",
 				 warn_str, cur->nidl);
 			return -1;
 		}
-		memcpy(ids->eui64, buf, NVME_NIDT_EUI64_LEN);
-		return NVME_NIDT_EUI64_LEN;
+		memcpy(ids->eui64, buf, nvme_nidt_len(NVME_NIDT_EUI64));
+		return nvme_nidt_len(NVME_NIDT_EUI64);
 	case NVME_NIDT_NGUID:
-		if (cur->nidl != NVME_NIDT_NGUID_LEN) {
+		if (cur->nidl != nvme_nidt_len(NVME_NIDT_NGUID)) {
 			dev_warn(ctrl->device, "%s %d for NVME_NIDT_NGUID\n",
 				 warn_str, cur->nidl);
 			return -1;
 		}
-		memcpy(ids->nguid, buf, NVME_NIDT_NGUID_LEN);
-		return NVME_NIDT_NGUID_LEN;
+		memcpy(ids->nguid, buf, nvme_nidt_len(NVME_NIDT_NGUID));
+		return nvme_nidt_len(NVME_NIDT_NGUID);
 	case NVME_NIDT_UUID:
-		if (cur->nidl != NVME_NIDT_UUID_LEN) {
+		if (cur->nidl != nvme_nidt_len(NVME_NIDT_UUID)) {
 			dev_warn(ctrl->device, "%s %d for NVME_NIDT_UUID\n",
 				 warn_str, cur->nidl);
 			return -1;
 		}
 		uuid_copy(&ids->uuid, buf);
-		return NVME_NIDT_UUID_LEN;
+		return nvme_nidt_len(NVME_NIDT_UUID);
 	case NVME_NIDT_CSI:
-		if (cur->nidl != NVME_NIDT_CSI_LEN) {
+		if (cur->nidl != nvme_nidt_len(NVME_NIDT_CSI)) {
 			dev_warn(ctrl->device, "%s %d for NVME_NIDT_CSI\n",
 				 warn_str, cur->nidl);
 			return -1;
 		}
-		memcpy(&ids->csi, buf, NVME_NIDT_CSI_LEN);
+		memcpy(&ids->csi, buf, nvme_nidt_len(NVME_NIDT_CSI));
 		*csi_seen = true;
-		return NVME_NIDT_CSI_LEN;
+		return nvme_nidt_len(NVME_NIDT_CSI);
 	default:
 		/* Skip unknown types */
 		return cur->nidl;
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index edcbd60b88b9..5d6b0fe64db1 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -9,6 +9,7 @@
 
 #include <linux/types.h>
 #include <linux/uuid.h>
+#include <linux/kernel.h>
 
 /* NQN names in commands fields specified one size */
 #define NVMF_NQN_FIELD_LEN	256
@@ -497,6 +498,21 @@ enum {
 	NVME_NIDT_CSI		= 0x04,
 };
 
+static inline unsigned int nvme_nidt_len(unsigned int id)
+{
+	static int nvme_nidt_len_map[] = {
+		[NVME_NIDT_EUI64]	= 8,
+		[NVME_NIDT_NGUID]	= 16,
+		[NVME_NIDT_UUID]	= 16,
+		[NVME_NIDT_CSI]		= 1,
+	};
+
+	if (id > ARRAY_SIZE(nvme_nidt_len_map) || !nvme_nidt_len_map[id])
+		return 0;
+
+	return nvme_nidt_len_map[id];
+}
+
 struct nvme_smart_log {
 	__u8			critical_warning;
 	__u8			temperature[2];
-- 
2.22.1



[-- Attachment #2: Type: text/plain, Size: 158 bytes --]

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

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

* [PATCH 3/4] nvmet: use nvme_nidt_len() helper for id-desc
  2021-05-18  5:05 [PATCH 0/4] nvme: nidt and ns-desc related clanup Chaitanya Kulkarni
  2021-05-18  5:05 ` [PATCH 1/4] nvme: calculate data offset once for ns-desc Chaitanya Kulkarni
  2021-05-18  5:05 ` [PATCH 2/4] nvme-core: add a helper to get the len from nid Chaitanya Kulkarni
@ 2021-05-18  5:05 ` Chaitanya Kulkarni
  2021-05-18  5:05 ` [PATCH 4/4] nvme-core: remove the NVME_NIDT_XXX_LEN macros Chaitanya Kulkarni
  2021-05-18  6:49 ` [PATCH 0/4] nvme: nidt and ns-desc related clanup Christoph Hellwig
  4 siblings, 0 replies; 7+ messages in thread
From: Chaitanya Kulkarni @ 2021-05-18  5:05 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, hch, sagi, Chaitanya Kulkarni

Use the previously added helper to remove the open coding of the
NVME_NIDT_XXX_LEN for target.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index dcd49a72f2f3..1a58719ef952 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -7,6 +7,7 @@
 #include <linux/module.h>
 #include <linux/rculist.h>
 #include <linux/part_stat.h>
+#include <linux/nvme.h>
 
 #include <generated/utsrelease.h>
 #include <asm/unaligned.h>
@@ -622,15 +623,16 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)
 
 	if (memchr_inv(&req->ns->uuid, 0, sizeof(req->ns->uuid))) {
 		status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID,
-						  NVME_NIDT_UUID_LEN,
+						  nvme_nidt_len(NVME_NIDT_UUID),
 						  &req->ns->uuid, &off);
 		if (status)
 			goto out;
 	}
 	if (memchr_inv(req->ns->nguid, 0, sizeof(req->ns->nguid))) {
+		u8 len = nvme_nidt_len(NVME_NIDT_NGUID);
+
 		status = nvmet_copy_ns_identifier(req, NVME_NIDT_NGUID,
-						  NVME_NIDT_NGUID_LEN,
-						  &req->ns->nguid, &off);
+						  len, &req->ns->nguid, &off);
 		if (status)
 			goto out;
 	}
-- 
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] 7+ messages in thread

* [PATCH 4/4] nvme-core: remove the NVME_NIDT_XXX_LEN macros
  2021-05-18  5:05 [PATCH 0/4] nvme: nidt and ns-desc related clanup Chaitanya Kulkarni
                   ` (2 preceding siblings ...)
  2021-05-18  5:05 ` [PATCH 3/4] nvmet: use nvme_nidt_len() helper for id-desc Chaitanya Kulkarni
@ 2021-05-18  5:05 ` Chaitanya Kulkarni
  2021-05-18  6:49 ` [PATCH 0/4] nvme: nidt and ns-desc related clanup Christoph Hellwig
  4 siblings, 0 replies; 7+ messages in thread
From: Chaitanya Kulkarni @ 2021-05-18  5:05 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, hch, sagi, Chaitanya Kulkarni

Now that we have removed all the open coding of the NVME_NIDT_XXX_LEN
macros, remove the macros.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 include/linux/nvme.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 5d6b0fe64db1..61a0de4bb71e 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -486,11 +486,6 @@ struct nvme_ns_id_desc {
 	__le16 reserved;
 };
 
-#define NVME_NIDT_EUI64_LEN	8
-#define NVME_NIDT_NGUID_LEN	16
-#define NVME_NIDT_UUID_LEN	16
-#define NVME_NIDT_CSI_LEN	1
-
 enum {
 	NVME_NIDT_EUI64		= 0x01,
 	NVME_NIDT_NGUID		= 0x02,
-- 
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] 7+ messages in thread

* Re: [PATCH 0/4] nvme: nidt and ns-desc related clanup
  2021-05-18  5:05 [PATCH 0/4] nvme: nidt and ns-desc related clanup Chaitanya Kulkarni
                   ` (3 preceding siblings ...)
  2021-05-18  5:05 ` [PATCH 4/4] nvme-core: remove the NVME_NIDT_XXX_LEN macros Chaitanya Kulkarni
@ 2021-05-18  6:49 ` Christoph Hellwig
  2021-05-18 20:29   ` Chaitanya Kulkarni
  4 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2021-05-18  6:49 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-nvme, kbusch, hch, sagi

On Mon, May 17, 2021 at 10:05:53PM -0700, Chaitanya Kulkarni wrote:
> Hi,
> 
> This is a small patch-series tries to minimize the number of the macro
> identifiers. 
> 
> In the current code we maintain NVME_NIDT_XXX identifire values and the
> their length NVME_NIDT_XXX_LEN separately which has 1:1 mapping. This
> adds founr extra macros and open coding of such macros in the host and
> the target.
> 
> With this open coding in future we will have to maintain one extra macro
> representing length for each new NIDT identifire that we will add.

I really don't see the point.  You remove one define per type, but
need to create a mapping table entry.  This adds (slightly) more code
and is less obvious.  Do you have any grand plans for which this is
a preparation?

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

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

* Re: [PATCH 0/4] nvme: nidt and ns-desc related clanup
  2021-05-18  6:49 ` [PATCH 0/4] nvme: nidt and ns-desc related clanup Christoph Hellwig
@ 2021-05-18 20:29   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 7+ messages in thread
From: Chaitanya Kulkarni @ 2021-05-18 20:29 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, kbusch, sagi

On 5/17/21 23:49, Christoph Hellwig wrote:
>> With this open coding in future we will have to maintain one extra macro
>> representing length for each new NIDT identifire that we will add.
> I really don't see the point.  You remove one define per type, but
> need to create a mapping table entry.  This adds (slightly) more code
> and is less obvious.  Do you have any grand plans for which this is
> a preparation?
>

Okay, we can drop this. No grand plans, I just thought open coding could
be avoided, but if that makes code complex no point.



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

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

end of thread, other threads:[~2021-05-18 20:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-18  5:05 [PATCH 0/4] nvme: nidt and ns-desc related clanup Chaitanya Kulkarni
2021-05-18  5:05 ` [PATCH 1/4] nvme: calculate data offset once for ns-desc Chaitanya Kulkarni
2021-05-18  5:05 ` [PATCH 2/4] nvme-core: add a helper to get the len from nid Chaitanya Kulkarni
2021-05-18  5:05 ` [PATCH 3/4] nvmet: use nvme_nidt_len() helper for id-desc Chaitanya Kulkarni
2021-05-18  5:05 ` [PATCH 4/4] nvme-core: remove the NVME_NIDT_XXX_LEN macros Chaitanya Kulkarni
2021-05-18  6:49 ` [PATCH 0/4] nvme: nidt and ns-desc related clanup Christoph Hellwig
2021-05-18 20:29   ` Chaitanya Kulkarni

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.