Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1657043 > unrolled thread

[PATCH v4 7/8] nvmet: allow overriding the NVMe VS via configfs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 7/8] nvmet: allow overriding the NVMe VS via configfs Johannes Thumshirn <jthumshirn@suse.de> - 2017-06-04 12:40 +0200
    Re: [PATCH v4 7/8] nvmet: allow overriding the NVMe VS via configfs Sagi Grimberg <sagi@grimberg.me> - 2017-06-04 17:10 +0200
    Re: [PATCH v4 7/8] nvmet: allow overriding the NVMe VS via configfs Christoph Hellwig <hch@lst.de> - 2017-06-05 07:50 +0200
    Re: [PATCH v4 7/8] nvmet: allow overriding the NVMe VS via configfs Hannes Reinecke <hare@suse.de> - 2017-06-06 08:30 +0200

#1657043 — [PATCH v4 7/8] nvmet: allow overriding the NVMe VS via configfs

Allow overriding the announced NVMe Version of a via configfs.

This is particularly helpful when debugging new features for the host
or target side without bumping the hard coded version (as the target
might not be fully compliant to the announced version yet).

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/nvme/target/configfs.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/nvme.h           |  4 ++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 16f9f6e3a084..45421d4308a4 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -650,8 +650,42 @@ static ssize_t nvmet_subsys_attr_allow_any_host_store(struct config_item *item,
 
 CONFIGFS_ATTR(nvmet_subsys_, attr_allow_any_host);
 
+static ssize_t nvmet_subsys_version_show(struct config_item *item,
+					      char *page)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+	int major, minor, tertiary;
+	u32 ver;
+
+	ver = subsys->ver;
+	major = NVME_MAJOR(ver);
+	minor = NVME_MINOR(ver);
+	tertiary = NVME_TERRIARY(ver);
+
+	return snprintf(page, PAGE_SIZE, "%d %d %d\n", major, minor, tertiary);
+}
+
+static ssize_t nvmet_subsys_version_store(struct config_item *item,
+					       const char *page, size_t count)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+	int major, minor, tertiary;
+	int ret;
+
+
+	ret = sscanf(page, "%d %d %d\n", &major, &minor, &tertiary);
+	if (ret != 3)
+		return -EINVAL;
+
+	subsys->ver = NVME_VS(major, minor, tertiary);
+
+	return count;
+}
+CONFIGFS_ATTR(nvmet_subsys_, version);
+
 static struct configfs_attribute *nvmet_subsys_attrs[] = {
 	&nvmet_subsys_attr_attr_allow_any_host,
+	&nvmet_subsys_attr_version,
 	NULL,
 };
 
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index afa6ef484e50..0d6e307a7aa4 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -1069,4 +1069,8 @@ struct nvme_completion {
 #define NVME_VS(major, minor, tertiary) \
 	(((major) << 16) | ((minor) << 8) | (tertiary))
 
+#define NVME_MAJOR(ver)		((ver) >> 16)
+#define NVME_MINOR(ver)		(((ver) >> 8) & 0xff)
+#define NVME_TERRIARY(ver)	((ver) & 0xff)
+
 #endif /* _LINUX_NVME_H */
-- 
2.12.0

[toc] | [next] | [standalone]

#1657088

On 04/06/17 13:36, Johannes Thumshirn wrote:
> Allow overriding the announced NVMe Version of a via configfs.
> 
> This is particularly helpful when debugging new features for the host
> or target side without bumping the hard coded version (as the target
> might not be fully compliant to the announced version yet).
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>   drivers/nvme/target/configfs.c | 34 ++++++++++++++++++++++++++++++++++
>   include/linux/nvme.h           |  4 ++++
>   2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index 16f9f6e3a084..45421d4308a4 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -650,8 +650,42 @@ static ssize_t nvmet_subsys_attr_allow_any_host_store(struct config_item *item,
>   
>   CONFIGFS_ATTR(nvmet_subsys_, attr_allow_any_host);
>   
> +static ssize_t nvmet_subsys_version_show(struct config_item *item,
> +					      char *page)
> +{
> +	struct nvmet_subsys *subsys = to_subsys(item);
> +	int major, minor, tertiary;
> +	u32 ver;
> +
> +	ver = subsys->ver;
> +	major = NVME_MAJOR(ver);
> +	minor = NVME_MINOR(ver);
> +	tertiary = NVME_TERRIARY(ver);
> +
> +	return snprintf(page, PAGE_SIZE, "%d %d %d\n", major, minor, tertiary);

Nit: maybe a dot separator would be better? e.g. "1.3.0" rather than
"1 3 0"

Otherwise looks good,

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

[toc] | [prev] | [next] | [standalone]

#1657286

On Sun, Jun 04, 2017 at 12:36:48PM +0200, Johannes Thumshirn wrote:
> Allow overriding the announced NVMe Version of a via configfs.
> 
> This is particularly helpful when debugging new features for the host
> or target side without bumping the hard coded version (as the target
> might not be fully compliant to the announced version yet).
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  drivers/nvme/target/configfs.c | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/nvme.h           |  4 ++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index 16f9f6e3a084..45421d4308a4 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -650,8 +650,42 @@ static ssize_t nvmet_subsys_attr_allow_any_host_store(struct config_item *item,
>  
>  CONFIGFS_ATTR(nvmet_subsys_, attr_allow_any_host);
>  
> +static ssize_t nvmet_subsys_version_show(struct config_item *item,
> +					      char *page)
> +{
> +	struct nvmet_subsys *subsys = to_subsys(item);
> +	int major, minor, tertiary;
> +	u32 ver;
> +
> +	ver = subsys->ver;
> +	major = NVME_MAJOR(ver);
> +	minor = NVME_MINOR(ver);
> +	tertiary = NVME_TERRIARY(ver);
> +
> +	return snprintf(page, PAGE_SIZE, "%d %d %d\n", major, minor, tertiary);

Nit Dop we really need all these variables?  Why not:

	return snprintf(page, PAGE_SIZE, "%d %d %d\n", NVME_MAJOR(subsys->ver), 
			NVME_MINOR(subsys->ver),  NVME_TERRIARY(subsys->ver));

?

Also except for the 1.2.1 oddbackk NVMe versions usually have two
components, and should be printed as such if the tertiary version is
0.


> +static ssize_t nvmet_subsys_version_store(struct config_item *item,
> +					       const char *page, size_t count)
> +{
> +	struct nvmet_subsys *subsys = to_subsys(item);
> +	int major, minor, tertiary;
> +	int ret;
> +
> +
> +	ret = sscanf(page, "%d %d %d\n", &major, &minor, &tertiary);
> +	if (ret != 3)
> +		return -EINVAL;

Same issue here.  I also have to say I'm a bit sceptical about
just being able to set versions as there are various dependencies
on it.  E.g. for 1.0 the version field in identify namespace doesn't
even exists and should be cleared to 0.

> +
> +	subsys->ver = NVME_VS(major, minor, tertiary);

locking?

> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index afa6ef484e50..0d6e307a7aa4 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -1069,4 +1069,8 @@ struct nvme_completion {
>  #define NVME_VS(major, minor, tertiary) \
>  	(((major) << 16) | ((minor) << 8) | (tertiary))
>  
> +#define NVME_MAJOR(ver)		((ver) >> 16)
> +#define NVME_MINOR(ver)		(((ver) >> 8) & 0xff)
> +#define NVME_TERRIARY(ver)	((ver) & 0xff)

I think TERRIARY should be TERTIARY.

[toc] | [prev] | [next] | [standalone]

#1658387

On 06/04/2017 12:36 PM, Johannes Thumshirn wrote:
> Allow overriding the announced NVMe Version of a via configfs.
> 
> This is particularly helpful when debugging new features for the host
> or target side without bumping the hard coded version (as the target
> might not be fully compliant to the announced version yet).
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  drivers/nvme/target/configfs.c | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/nvme.h           |  4 ++++
>  2 files changed, 38 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web