nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [ndctl PATCH 1/2] ndctl: update .gitignore
@ 2018-01-25 23:26 Ross Zwisler
  2018-01-25 23:26 ` [ndctl PATCH 2/2] ndctl: add sector_size to 'ndctl list' output Ross Zwisler
  2018-01-25 23:51 ` [ndctl PATCH 1/2] ndctl: update .gitignore Dan Williams
  0 siblings, 2 replies; 9+ messages in thread
From: Ross Zwisler @ 2018-01-25 23:26 UTC (permalink / raw)
  To: Dan Williams, linux-nvdimm

I noticed that my 'git status' output was quite noisy.  Here's what I get
with a full build using the suggested
./configure CFLAGS='-g -O0' --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib64
command and with some cscope files built:

  $ git status
  On branch pending
  Your branch is up-to-date with 'github/pending'.

  Untracked files:
    (use "git add <file>..." to include in what will be committed)

  	Documentation/daxctl/asciidoc.conf
  	Documentation/daxctl/daxctl-io.1
  	Documentation/daxctl/daxctl-list.1
  	Documentation/daxctl/daxctl.1
  	Documentation/ndctl/asciidoc.conf
  	Documentation/ndctl/ndctl-check-labels.1
  	Documentation/ndctl/ndctl-check-namespace.1
  	Documentation/ndctl/ndctl-create-namespace.1
  	Documentation/ndctl/ndctl-destroy-namespace.1
  	Documentation/ndctl/ndctl-disable-dimm.1
  	Documentation/ndctl/ndctl-disable-namespace.1
  	Documentation/ndctl/ndctl-disable-region.1
  	Documentation/ndctl/ndctl-enable-dimm.1
  	Documentation/ndctl/ndctl-enable-namespace.1
  	Documentation/ndctl/ndctl-enable-region.1
  	Documentation/ndctl/ndctl-init-labels.1
  	Documentation/ndctl/ndctl-inject-error.1
  	Documentation/ndctl/ndctl-list.1
  	Documentation/ndctl/ndctl-read-labels.1
  	Documentation/ndctl/ndctl-write-labels.1
  	Documentation/ndctl/ndctl-zero-labels.1
  	Documentation/ndctl/ndctl.1
  	ccan/list/.dirstamp
  	ccan/str/.dirstamp
  	cscope.files
  	cscope.out
  	daxctl/daxctl
  	daxctl/lib/libdaxctl.la
  	daxctl/lib/libdaxctl.lo
  	daxctl/lib/libdaxctl.pc
  	libccan.a
  	libutil.a
  	ndctl/lib/libndctl.pc
  	ndctl/ndctl
  	ndctl/util/.dirstamp
  	rhel/
  	sles/ndctl.spec
  	util/.dirstamp
  	util/log.lo
  	util/sysfs.lo
  	version.m4

  nothing added to commit but untracked files present (use "git add" to track)

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 .gitignore | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/.gitignore b/.gitignore
index 4899534..64e58fa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,3 +12,22 @@ Makefile.in
 /configure
 /libtool
 /stamp-h1
+*.1
+Documentation/daxctl/asciidoc.conf
+Documentation/ndctl/asciidoc.conf
+.dirstamp
+daxctl/daxctl
+daxctl/lib/libdaxctl.la
+daxctl/lib/libdaxctl.lo
+daxctl/lib/libdaxctl.pc
+*.a
+ndctl/lib/libndctl.pc
+ndctl/ndctl
+rhel/
+sles/ndctl.spec
+util/log.lo
+util/sysfs.lo
+version.m4
+*.swp
+cscope.files
+cscope.out
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [ndctl PATCH 2/2] ndctl: add sector_size to 'ndctl list' output
  2018-01-25 23:26 [ndctl PATCH 1/2] ndctl: update .gitignore Ross Zwisler
@ 2018-01-25 23:26 ` Ross Zwisler
  2018-01-25 23:39   ` Dave Jiang
  2018-01-26 20:54   ` [ndctl PATCH v2 " Ross Zwisler
  2018-01-25 23:51 ` [ndctl PATCH 1/2] ndctl: update .gitignore Dan Williams
  1 sibling, 2 replies; 9+ messages in thread
From: Ross Zwisler @ 2018-01-25 23:26 UTC (permalink / raw)
  To: Dan Williams, linux-nvdimm

It used to be that the only PMEM namespaces with a variable sector size
were ones with a BTT, but that has changed.  sector_size still doesn't make
sense for device DAX since we don't have a block I/O path, and we also skip
it if we don't have a specified sector size, as happens with namespaces of
devtype nd_namespace_io.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 util/json.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/util/json.c b/util/json.c
index 95beaa2..89306f0 100644
--- a/util/json.c
+++ b/util/json.c
@@ -718,11 +718,6 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 			goto err;
 		json_object_object_add(jndns, "uuid", jobj);
 
-		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
-		if (!jobj)
-			goto err;
-		json_object_object_add(jndns, "sector_size", jobj);
-
 		bdev = ndctl_btt_get_block_device(btt);
 	} else if (pfn) {
 		ndctl_pfn_get_uuid(pfn, uuid);
@@ -774,6 +769,23 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 	} else
 		bdev = ndctl_namespace_get_block_device(ndns);
 
+	jobj = NULL;
+	if (btt) {
+		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
+		if (!jobj)
+			goto err;
+	} else if (!dax) {
+		unsigned int sector_size = ndctl_namespace_get_sector_size(ndns);
+
+		if (sector_size) {
+			jobj = json_object_new_int(sector_size);
+			if (!jobj)
+				goto err;
+		}
+	}
+	if (jobj)
+		json_object_object_add(jndns, "sector_size", jobj);
+
 	if (bdev && bdev[0]) {
 		jobj = json_object_new_string(bdev);
 		if (!jobj)
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [ndctl PATCH 2/2] ndctl: add sector_size to 'ndctl list' output
  2018-01-25 23:26 ` [ndctl PATCH 2/2] ndctl: add sector_size to 'ndctl list' output Ross Zwisler
@ 2018-01-25 23:39   ` Dave Jiang
  2018-01-26 20:54   ` [ndctl PATCH v2 " Ross Zwisler
  1 sibling, 0 replies; 9+ messages in thread
From: Dave Jiang @ 2018-01-25 23:39 UTC (permalink / raw)
  To: Ross Zwisler, Dan Williams, linux-nvdimm



On 01/25/2018 04:26 PM, Ross Zwisler wrote:
> It used to be that the only PMEM namespaces with a variable sector size
> were ones with a BTT, but that has changed.  sector_size still doesn't make
> sense for device DAX since we don't have a block I/O path, and we also skip
> it if we don't have a specified sector size, as happens with namespaces of
> devtype nd_namespace_io.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>


> ---
>   util/json.c | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/util/json.c b/util/json.c
> index 95beaa2..89306f0 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -718,11 +718,6 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>   			goto err;
>   		json_object_object_add(jndns, "uuid", jobj);
>   
> -		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
> -		if (!jobj)
> -			goto err;
> -		json_object_object_add(jndns, "sector_size", jobj);
> -
>   		bdev = ndctl_btt_get_block_device(btt);
>   	} else if (pfn) {
>   		ndctl_pfn_get_uuid(pfn, uuid);
> @@ -774,6 +769,23 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>   	} else
>   		bdev = ndctl_namespace_get_block_device(ndns);
>   
> +	jobj = NULL;
> +	if (btt) {
> +		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
> +		if (!jobj)
> +			goto err;
> +	} else if (!dax) {
> +		unsigned int sector_size = ndctl_namespace_get_sector_size(ndns);
> +
> +		if (sector_size) {
> +			jobj = json_object_new_int(sector_size);
> +			if (!jobj)
> +				goto err;
> +		}
> +	}
> +	if (jobj)
> +		json_object_object_add(jndns, "sector_size", jobj);
> +
>   	if (bdev && bdev[0]) {
>   		jobj = json_object_new_string(bdev);
>   		if (!jobj)
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [ndctl PATCH 1/2] ndctl: update .gitignore
  2018-01-25 23:26 [ndctl PATCH 1/2] ndctl: update .gitignore Ross Zwisler
  2018-01-25 23:26 ` [ndctl PATCH 2/2] ndctl: add sector_size to 'ndctl list' output Ross Zwisler
@ 2018-01-25 23:51 ` Dan Williams
  1 sibling, 0 replies; 9+ messages in thread
From: Dan Williams @ 2018-01-25 23:51 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: linux-nvdimm

On Thu, Jan 25, 2018 at 3:26 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> I noticed that my 'git status' output was quite noisy.  Here's what I get
> with a full build using the suggested
> ./configure CFLAGS='-g -O0' --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib64
> command and with some cscope files built:
>
>   $ git status
>   On branch pending
>   Your branch is up-to-date with 'github/pending'.
>
>   Untracked files:
>     (use "git add <file>..." to include in what will be committed)
>
>         Documentation/daxctl/asciidoc.conf
>         Documentation/daxctl/daxctl-io.1
>         Documentation/daxctl/daxctl-list.1
>         Documentation/daxctl/daxctl.1
>         Documentation/ndctl/asciidoc.conf
>         Documentation/ndctl/ndctl-check-labels.1
>         Documentation/ndctl/ndctl-check-namespace.1
>         Documentation/ndctl/ndctl-create-namespace.1
>         Documentation/ndctl/ndctl-destroy-namespace.1
>         Documentation/ndctl/ndctl-disable-dimm.1
>         Documentation/ndctl/ndctl-disable-namespace.1
>         Documentation/ndctl/ndctl-disable-region.1
>         Documentation/ndctl/ndctl-enable-dimm.1
>         Documentation/ndctl/ndctl-enable-namespace.1
>         Documentation/ndctl/ndctl-enable-region.1
>         Documentation/ndctl/ndctl-init-labels.1
>         Documentation/ndctl/ndctl-inject-error.1
>         Documentation/ndctl/ndctl-list.1
>         Documentation/ndctl/ndctl-read-labels.1
>         Documentation/ndctl/ndctl-write-labels.1
>         Documentation/ndctl/ndctl-zero-labels.1
>         Documentation/ndctl/ndctl.1
>         ccan/list/.dirstamp
>         ccan/str/.dirstamp
>         cscope.files
>         cscope.out
>         daxctl/daxctl
>         daxctl/lib/libdaxctl.la
>         daxctl/lib/libdaxctl.lo
>         daxctl/lib/libdaxctl.pc
>         libccan.a
>         libutil.a
>         ndctl/lib/libndctl.pc
>         ndctl/ndctl
>         ndctl/util/.dirstamp
>         rhel/
>         sles/ndctl.spec
>         util/.dirstamp
>         util/log.lo
>         util/sysfs.lo
>         version.m4
>
>   nothing added to commit but untracked files present (use "git add" to track)

Looks good to me, you can tell I don't use 'git status' in my workflow.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [ndctl PATCH v2 2/2] ndctl: add sector_size to 'ndctl list' output
  2018-01-25 23:26 ` [ndctl PATCH 2/2] ndctl: add sector_size to 'ndctl list' output Ross Zwisler
  2018-01-25 23:39   ` Dave Jiang
@ 2018-01-26 20:54   ` Ross Zwisler
  2018-01-26 20:56     ` Dave Jiang
  2018-01-26 21:00     ` Dan Williams
  1 sibling, 2 replies; 9+ messages in thread
From: Ross Zwisler @ 2018-01-26 20:54 UTC (permalink / raw)
  To: Dan Williams, linux-nvdimm, Dave Jiang

It used to be that the only PMEM namespaces with a variable sector size
were ones with a BTT, but that has changed.  sector_size still doesn't make
sense for device DAX since we don't have a block I/O path.

If we don't have a specified sector size, as happens with namespaces of
devtype nd_namespace_io and older v1.1 namespace labels, we will display
the default sector size of 512.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 util/json.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/util/json.c b/util/json.c
index 95beaa2..647bf03 100644
--- a/util/json.c
+++ b/util/json.c
@@ -718,11 +718,6 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 			goto err;
 		json_object_object_add(jndns, "uuid", jobj);
 
-		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
-		if (!jobj)
-			goto err;
-		json_object_object_add(jndns, "sector_size", jobj);
-
 		bdev = ndctl_btt_get_block_device(btt);
 	} else if (pfn) {
 		ndctl_pfn_get_uuid(pfn, uuid);
@@ -774,6 +769,24 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 	} else
 		bdev = ndctl_namespace_get_block_device(ndns);
 
+	jobj = NULL;
+	if (btt) {
+		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
+		if (!jobj)
+			goto err;
+	} else if (!dax) {
+		unsigned int sector_size = ndctl_namespace_get_sector_size(ndns);
+
+		if (!sector_size)
+			sector_size = 512;
+
+		jobj = json_object_new_int(sector_size);
+		if (!jobj)
+			goto err;
+	}
+	if (jobj)
+		json_object_object_add(jndns, "sector_size", jobj);
+
 	if (bdev && bdev[0]) {
 		jobj = json_object_new_string(bdev);
 		if (!jobj)
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [ndctl PATCH v2 2/2] ndctl: add sector_size to 'ndctl list' output
  2018-01-26 20:54   ` [ndctl PATCH v2 " Ross Zwisler
@ 2018-01-26 20:56     ` Dave Jiang
  2018-01-26 21:00     ` Dan Williams
  1 sibling, 0 replies; 9+ messages in thread
From: Dave Jiang @ 2018-01-26 20:56 UTC (permalink / raw)
  To: Ross Zwisler, Dan Williams, linux-nvdimm



On 01/26/2018 01:54 PM, Ross Zwisler wrote:
> It used to be that the only PMEM namespaces with a variable sector size
> were ones with a BTT, but that has changed.  sector_size still doesn't make
> sense for device DAX since we don't have a block I/O path.
> 
> If we don't have a specified sector size, as happens with namespaces of
> devtype nd_namespace_io and older v1.1 namespace labels, we will display
> the default sector size of 512.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   util/json.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/util/json.c b/util/json.c
> index 95beaa2..647bf03 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -718,11 +718,6 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>   			goto err;
>   		json_object_object_add(jndns, "uuid", jobj);
>   
> -		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
> -		if (!jobj)
> -			goto err;
> -		json_object_object_add(jndns, "sector_size", jobj);
> -
>   		bdev = ndctl_btt_get_block_device(btt);
>   	} else if (pfn) {
>   		ndctl_pfn_get_uuid(pfn, uuid);
> @@ -774,6 +769,24 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>   	} else
>   		bdev = ndctl_namespace_get_block_device(ndns);
>   
> +	jobj = NULL;
> +	if (btt) {
> +		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
> +		if (!jobj)
> +			goto err;
> +	} else if (!dax) {
> +		unsigned int sector_size = ndctl_namespace_get_sector_size(ndns);
> +
> +		if (!sector_size)
> +			sector_size = 512;
> +
> +		jobj = json_object_new_int(sector_size);
> +		if (!jobj)
> +			goto err;
> +	}
> +	if (jobj)
> +		json_object_object_add(jndns, "sector_size", jobj);
> +
>   	if (bdev && bdev[0]) {
>   		jobj = json_object_new_string(bdev);
>   		if (!jobj)
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [ndctl PATCH v2 2/2] ndctl: add sector_size to 'ndctl list' output
  2018-01-26 20:54   ` [ndctl PATCH v2 " Ross Zwisler
  2018-01-26 20:56     ` Dave Jiang
@ 2018-01-26 21:00     ` Dan Williams
  2018-01-29 21:48       ` [ndctl PATCH v3 " Ross Zwisler
  1 sibling, 1 reply; 9+ messages in thread
From: Dan Williams @ 2018-01-26 21:00 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: linux-nvdimm

On Fri, Jan 26, 2018 at 12:54 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> It used to be that the only PMEM namespaces with a variable sector size
> were ones with a BTT, but that has changed.  sector_size still doesn't make
> sense for device DAX since we don't have a block I/O path.
>
> If we don't have a specified sector size, as happens with namespaces of
> devtype nd_namespace_io and older v1.1 namespace labels, we will display
> the default sector size of 512.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
>  util/json.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/util/json.c b/util/json.c
> index 95beaa2..647bf03 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -718,11 +718,6 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>                         goto err;
>                 json_object_object_add(jndns, "uuid", jobj);
>
> -               jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
> -               if (!jobj)
> -                       goto err;
> -               json_object_object_add(jndns, "sector_size", jobj);
> -
>                 bdev = ndctl_btt_get_block_device(btt);
>         } else if (pfn) {
>                 ndctl_pfn_get_uuid(pfn, uuid);
> @@ -774,6 +769,24 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>         } else
>                 bdev = ndctl_namespace_get_block_device(ndns);
>
> +       jobj = NULL;
> +       if (btt) {
> +               jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
> +               if (!jobj)
> +                       goto err;
> +       } else if (!dax) {
> +               unsigned int sector_size = ndctl_namespace_get_sector_size(ndns);
> +

I'd put a comment here that says on kernel's that do not export a
sector_size attribute, or if userspace fails to select a dynamic
sector_size, the kernel default size is 512.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [ndctl PATCH v3 2/2] ndctl: add sector_size to 'ndctl list' output
  2018-01-26 21:00     ` Dan Williams
@ 2018-01-29 21:48       ` Ross Zwisler
  2018-01-29 22:09         ` Dan Williams
  0 siblings, 1 reply; 9+ messages in thread
From: Ross Zwisler @ 2018-01-29 21:48 UTC (permalink / raw)
  To: Dave Jiang, Dan Williams, linux-nvdimm

It used to be that the only PMEM namespaces with a variable sector size
were ones with a BTT, but that has changed.  sector_size still doesn't make
sense for device DAX since we don't have a block I/O path.

If we don't have a specified sector size, as happens with namespaces of
devtype nd_namespace_io and older v1.1 namespace labels, we will display
the default sector size of 512.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 util/json.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/util/json.c b/util/json.c
index 95beaa2..17d8f13 100644
--- a/util/json.c
+++ b/util/json.c
@@ -718,11 +718,6 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 			goto err;
 		json_object_object_add(jndns, "uuid", jobj);
 
-		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
-		if (!jobj)
-			goto err;
-		json_object_object_add(jndns, "sector_size", jobj);
-
 		bdev = ndctl_btt_get_block_device(btt);
 	} else if (pfn) {
 		ndctl_pfn_get_uuid(pfn, uuid);
@@ -774,6 +769,30 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 	} else
 		bdev = ndctl_namespace_get_block_device(ndns);
 
+	jobj = NULL;
+	if (btt) {
+		jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
+		if (!jobj)
+			goto err;
+	} else if (!dax) {
+		unsigned int sector_size = ndctl_namespace_get_sector_size(ndns);
+
+		/*
+		 * The kernel will default to a 512 byte sector size on PMEM
+		 * namespaces that don't explicitly have a sector size. This
+		 * happens because they use pre-v1.2 labels or because they
+		 * don't have a label space (devtype=nd_namespace_io).
+		 */
+		if (!sector_size)
+			sector_size = 512;
+
+		jobj = json_object_new_int(sector_size);
+		if (!jobj)
+			goto err;
+	}
+	if (jobj)
+		json_object_object_add(jndns, "sector_size", jobj);
+
 	if (bdev && bdev[0]) {
 		jobj = json_object_new_string(bdev);
 		if (!jobj)
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [ndctl PATCH v3 2/2] ndctl: add sector_size to 'ndctl list' output
  2018-01-29 21:48       ` [ndctl PATCH v3 " Ross Zwisler
@ 2018-01-29 22:09         ` Dan Williams
  0 siblings, 0 replies; 9+ messages in thread
From: Dan Williams @ 2018-01-29 22:09 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: linux-nvdimm

On Mon, Jan 29, 2018 at 1:48 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> It used to be that the only PMEM namespaces with a variable sector size
> were ones with a BTT, but that has changed.  sector_size still doesn't make
> sense for device DAX since we don't have a block I/O path.
>
> If we don't have a specified sector size, as happens with namespaces of
> devtype nd_namespace_io and older v1.1 namespace labels, we will display
> the default sector size of 512.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  util/json.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/util/json.c b/util/json.c
> index 95beaa2..17d8f13 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -718,11 +718,6 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>                         goto err;
>                 json_object_object_add(jndns, "uuid", jobj);
>
> -               jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
> -               if (!jobj)
> -                       goto err;
> -               json_object_object_add(jndns, "sector_size", jobj);
> -
>                 bdev = ndctl_btt_get_block_device(btt);
>         } else if (pfn) {
>                 ndctl_pfn_get_uuid(pfn, uuid);
> @@ -774,6 +769,30 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>         } else
>                 bdev = ndctl_namespace_get_block_device(ndns);
>
> +       jobj = NULL;
> +       if (btt) {
> +               jobj = json_object_new_int(ndctl_btt_get_sector_size(btt));
> +               if (!jobj)
> +                       goto err;
> +       } else if (!dax) {
> +               unsigned int sector_size = ndctl_namespace_get_sector_size(ndns);
> +
> +               /*
> +                * The kernel will default to a 512 byte sector size on PMEM
> +                * namespaces that don't explicitly have a sector size. This
> +                * happens because they use pre-v1.2 labels or because they
> +                * don't have a label space (devtype=nd_namespace_io).
> +                */
> +               if (!sector_size)
> +                       sector_size = 512;
> +
> +               jobj = json_object_new_int(sector_size);
> +               if (!jobj)
> +                       goto err;
> +       }
> +       if (jobj)
> +               json_object_object_add(jndns, "sector_size", jobj);
> +
>         if (bdev && bdev[0]) {
>                 jobj = json_object_new_string(bdev);
>                 if (!jobj)
> --
> 2.14.3
>

Looks good to me, you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

end of thread, other threads:[~2018-01-29 22:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 23:26 [ndctl PATCH 1/2] ndctl: update .gitignore Ross Zwisler
2018-01-25 23:26 ` [ndctl PATCH 2/2] ndctl: add sector_size to 'ndctl list' output Ross Zwisler
2018-01-25 23:39   ` Dave Jiang
2018-01-26 20:54   ` [ndctl PATCH v2 " Ross Zwisler
2018-01-26 20:56     ` Dave Jiang
2018-01-26 21:00     ` Dan Williams
2018-01-29 21:48       ` [ndctl PATCH v3 " Ross Zwisler
2018-01-29 22:09         ` Dan Williams
2018-01-25 23:51 ` [ndctl PATCH 1/2] ndctl: update .gitignore Dan Williams

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