[ndctl PATCH 1/4] util: mark badblocks helpers static

[ndctl PATCH 1/4] util: mark badblocks helpers static

[Dan Williams]

These helpers are only used internal to util/json.c.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 util/json.c |    6 +++---
 util/json.h |    6 ------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/util/json.c b/util/json.c
index 0092017262bd..b2c84181cadb 100644
--- a/util/json.c
+++ b/util/json.c
@@ -356,7 +356,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 	return NULL;
 }
 
-struct json_object *util_pfn_badblocks_to_json(struct ndctl_pfn *pfn,
+static struct json_object *util_pfn_badblocks_to_json(struct ndctl_pfn *pfn,
 		bool include_media_errors, unsigned int *bb_count)
 {
 	struct ndctl_region *region = ndctl_pfn_get_region(pfn);
@@ -374,7 +374,7 @@ struct json_object *util_pfn_badblocks_to_json(struct ndctl_pfn *pfn,
 			include_media_errors, bb_count);
 }
 
-struct json_object *util_btt_badblocks_to_json(struct ndctl_btt *btt,
+static struct json_object *util_btt_badblocks_to_json(struct ndctl_btt *btt,
 		bool include_media_errors, unsigned int *bb_count)
 {
 	struct ndctl_region *region = ndctl_btt_get_region(btt);
@@ -393,7 +393,7 @@ struct json_object *util_btt_badblocks_to_json(struct ndctl_btt *btt,
 			include_media_errors, bb_count);
 }
 
-struct json_object *util_dax_badblocks_to_json(struct ndctl_dax *dax,
+static struct json_object *util_dax_badblocks_to_json(struct ndctl_dax *dax,
 		bool include_media_errors, unsigned int *bb_count)
 {
 	struct ndctl_region *region = ndctl_dax_get_region(dax);
diff --git a/util/json.h b/util/json.h
index b3bc2084b621..1a6caeeb6e9e 100644
--- a/util/json.h
+++ b/util/json.h
@@ -13,12 +13,6 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		bool include_idle, bool include_dax, bool include_media_errs);
 struct daxctl_region;
 struct daxctl_dev;
-struct json_object *util_dax_badblocks_to_json(struct ndctl_dax *dax,
-		bool include_media_errors, unsigned int *bb_count);
-struct json_object *util_pfn_badblocks_to_json(struct ndctl_pfn *pfn,
-		bool include_media_errors, unsigned int *bb_count);
-struct json_object *util_btt_badblocks_to_json(struct ndctl_btt *btt,
-		bool include_media_errors, unsigned int *bb_count);
 struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 		bool include_media_errors, unsigned int *bb_count);
 struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,

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

[ndctl PATCH 2/4] ndctl, list: clarify dev_badblocks_to_json() usage with btt and documentation

[Dan Williams]

Rather than pass include_media_errors through to dev_badblocks_to_json()
in the btt case only to throw it away, just tell dev_badblocks_to_json()
not to bother. The commentary about why we do not support badblocks
listing is moved internal to the util_btt_badblocks_to_json() helper.

This also takes the opportunity to s/jobj2/jbbs/ for readability, and
s/include_media_err/include_media_errors/ for consistency.

Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl-list.txt |   12 +++++++--
 ndctl/list.c                 |   12 ++++-----
 util/json.c                  |   57 ++++++++++++++++++++----------------------
 3 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/Documentation/ndctl-list.txt b/Documentation/ndctl-list.txt
index 0ac08a8024c0..da90860e061f 100644
--- a/Documentation/ndctl-list.txt
+++ b/Documentation/ndctl-list.txt
@@ -148,9 +148,15 @@ include::xable-region-options.txt[]
 
 -M::
 --media-errors::
-	Include media errors (badblocks) in the listing. badblocks_count
-	may count blocks that are not in the data space of the namespace
-	for sector mode.
+	Include media errors (badblocks) in the listing. Note that the
+	'badblock_count' property is included in the listing by default
+	when the count is non-zero, otherwise it is hidden. Also, if the
+	namespace is in 'sector' mode the 'badblocks' listing is not
+	included and 'badblock_count' property may include blocks that are
+	located in metadata, or unused capacity in the namespace. Convert
+	a 'sector' namespace into 'raw' mode to list precise 'badblocks'
+	offsets.
+
 [verse]
 {
   "dev":"namespace7.0",
diff --git a/ndctl/list.c b/ndctl/list.c
index 8d912e654476..cf26d8c08183 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -120,10 +120,10 @@ static struct json_object *list_namespaces(struct ndctl_region *region,
 }
 
 static struct json_object *region_to_json(struct ndctl_region *region,
-		bool include_media_err)
+		bool include_media_errors)
 {
 	struct json_object *jregion = json_object_new_object();
-	struct json_object *jobj, *jobj2, *jmappings = NULL;
+	struct json_object *jobj, *jbbs, *jmappings = NULL;
 	struct ndctl_interleave_set *iset;
 	struct ndctl_mapping *mapping;
 	unsigned int bb_count;
@@ -207,16 +207,16 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 		json_object_object_add(jregion, "state", jobj);
 	}
 
-	jobj2 = util_region_badblocks_to_json(region, include_media_err,
+	jbbs = util_region_badblocks_to_json(region, include_media_errors,
 			&bb_count);
 	jobj = json_object_new_int(bb_count);
 	if (!jobj) {
-		json_object_put(jobj2);
+		json_object_put(jbbs);
 		goto err;
 	}
 	json_object_object_add(jregion, "badblock_count", jobj);
-	if (include_media_err && jobj2)
-		json_object_object_add(jregion, "badblocks", jobj2);
+	if (include_media_errors && jbbs)
+		json_object_object_add(jregion, "badblocks", jbbs);
 
 	list_namespaces(region, jregion, NULL, false);
 	return jregion;
diff --git a/util/json.c b/util/json.c
index b2c84181cadb..f72ffca910ae 100644
--- a/util/json.c
+++ b/util/json.c
@@ -374,8 +374,8 @@ static struct json_object *util_pfn_badblocks_to_json(struct ndctl_pfn *pfn,
 			include_media_errors, bb_count);
 }
 
-static struct json_object *util_btt_badblocks_to_json(struct ndctl_btt *btt,
-		bool include_media_errors, unsigned int *bb_count)
+static void util_btt_badblocks_to_json(struct ndctl_btt *btt,
+		unsigned int *bb_count)
 {
 	struct ndctl_region *region = ndctl_btt_get_region(btt);
 	struct ndctl_namespace *ndns = ndctl_btt_get_namespace(btt);
@@ -383,14 +383,22 @@ static struct json_object *util_btt_badblocks_to_json(struct ndctl_btt *btt,
 
 	btt_begin = ndctl_namespace_get_resource(ndns);
 	if (btt_begin == ULLONG_MAX)
-		return NULL;
+		return;
 
 	btt_size = ndctl_btt_get_size(btt);
 	if (btt_size == ULLONG_MAX)
-		return NULL;
-
-	return dev_badblocks_to_json(region, btt_begin, btt_size,
-			include_media_errors, bb_count);
+		return;
+
+	/*
+	 * The dev_badblocks_to_json() for BTT is not accurate with
+	 * respect to data vs metadata badblocks, and is only useful for
+	 * a potential bb_count.
+	 *
+	 * FIXME: switch to native BTT badblocks representation
+	 * when / if the kernel provides it.
+	 */
+	dev_badblocks_to_json(region, btt_begin, btt_size,
+			false, bb_count);
 }
 
 static struct json_object *util_dax_badblocks_to_json(struct ndctl_dax *dax,
@@ -416,16 +424,16 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		bool include_media_errors)
 {
 	struct json_object *jndns = json_object_new_object();
+	struct json_object *jobj, *jbbs = NULL;
 	unsigned long long size = ULLONG_MAX;
 	enum ndctl_namespace_mode mode;
-	struct json_object *jobj, *jobj2;
 	const char *bdev = NULL;
+	unsigned int bb_count;
 	struct ndctl_btt *btt;
 	struct ndctl_pfn *pfn;
 	struct ndctl_dax *dax;
 	char buf[40];
 	uuid_t uuid;
-	unsigned int bb_count;
 
 	if (!jndns)
 		return NULL;
@@ -549,38 +557,27 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 	}
 
 	if (pfn)
-		jobj2 = util_pfn_badblocks_to_json(pfn, include_media_errors,
+		jbbs = util_pfn_badblocks_to_json(pfn, include_media_errors,
 				&bb_count);
 	else if (dax)
-		jobj2 = util_dax_badblocks_to_json(dax, include_media_errors,
+		jbbs = util_dax_badblocks_to_json(dax, include_media_errors,
 				&bb_count);
-	else if (btt) {
-		jobj2 = util_btt_badblocks_to_json(btt, include_media_errors,
-				&bb_count);
-		/*
-		 * Discard the jobj2, the badblocks for BTT is not,
-		 * accurate and there will be a good method to caculate
-		 * them later. We just want a bb count and not the specifics
-		 * for now.
-		 */
-		jobj2 = NULL;
-	} else {
-		struct ndctl_region *region =
-			ndctl_namespace_get_region(ndns);
-
-		jobj2 = util_region_badblocks_to_json(region,
+	else if (btt)
+		util_btt_badblocks_to_json(btt, &bb_count);
+	else
+		jbbs = util_region_badblocks_to_json(
+				ndctl_namespace_get_region(ndns),
 				include_media_errors, &bb_count);
-	}
 
 	jobj = json_object_new_int(bb_count);
 	if (!jobj) {
-		json_object_put(jobj2);
+		json_object_put(jbbs);
 		goto err;
 	}
 
 	json_object_object_add(jndns, "badblock_count", jobj);
-	if (include_media_errors && jobj2)
-			json_object_object_add(jndns, "badblocks", jobj2);
+	if (include_media_errors && jbbs)
+			json_object_object_add(jndns, "badblocks", jbbs);
 
 	return jndns;
  err:

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

[ndctl PATCH 3/4] ndctl, list: fix span for btt badblocks check

[Dan Williams]

The size returned by ndctl_btt_get_size() accounts for space consumed by
metadata or otherwise lost to sector size geometry. For the
'badblock_count' property all we can do is report a count of possible
badblocks which may be larger than actual badblocks impacting live data.
Use the raw namespace size for this check.

Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 util/json.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/util/json.c b/util/json.c
index f72ffca910ae..17595267c34c 100644
--- a/util/json.c
+++ b/util/json.c
@@ -379,14 +379,14 @@ static void util_btt_badblocks_to_json(struct ndctl_btt *btt,
 {
 	struct ndctl_region *region = ndctl_btt_get_region(btt);
 	struct ndctl_namespace *ndns = ndctl_btt_get_namespace(btt);
-	unsigned long long btt_begin, btt_size;
+	unsigned long long begin, size;
 
-	btt_begin = ndctl_namespace_get_resource(ndns);
-	if (btt_begin == ULLONG_MAX)
+	begin = ndctl_namespace_get_resource(ndns);
+	if (begin == ULLONG_MAX)
 		return;
 
-	btt_size = ndctl_btt_get_size(btt);
-	if (btt_size == ULLONG_MAX)
+	size = ndctl_namespace_get_size(ndns);
+	if (size == ULLONG_MAX)
 		return;
 
 	/*
@@ -397,8 +397,7 @@ static void util_btt_badblocks_to_json(struct ndctl_btt *btt,
 	 * FIXME: switch to native BTT badblocks representation
 	 * when / if the kernel provides it.
 	 */
-	dev_badblocks_to_json(region, btt_begin, btt_size,
-			false, bb_count);
+	dev_badblocks_to_json(region, begin, size, false, bb_count);
 }
 
 static struct json_object *util_dax_badblocks_to_json(struct ndctl_dax *dax,

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

[ndctl PATCH 4/4] ndctl, list: hide badblock_count by default

[Dan Williams]

In order to keep the default listing as compact as possible, hide
badblock_count when it is zero.

Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/list.c |   12 +++++++-----
 util/json.c  |   14 ++++++++------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/ndctl/list.c b/ndctl/list.c
index cf26d8c08183..befb3cfc6ffd 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -209,12 +209,14 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 
 	jbbs = util_region_badblocks_to_json(region, include_media_errors,
 			&bb_count);
-	jobj = json_object_new_int(bb_count);
-	if (!jobj) {
-		json_object_put(jbbs);
-		goto err;
+	if (bb_count) {
+		jobj = json_object_new_int(bb_count);
+		if (!jobj) {
+			json_object_put(jbbs);
+			goto err;
+		}
+		json_object_object_add(jregion, "badblock_count", jobj);
 	}
-	json_object_object_add(jregion, "badblock_count", jobj);
 	if (include_media_errors && jbbs)
 		json_object_object_add(jregion, "badblocks", jbbs);
 
diff --git a/util/json.c b/util/json.c
index 17595267c34c..4ca33a628381 100644
--- a/util/json.c
+++ b/util/json.c
@@ -568,15 +568,17 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 				ndctl_namespace_get_region(ndns),
 				include_media_errors, &bb_count);
 
-	jobj = json_object_new_int(bb_count);
-	if (!jobj) {
-		json_object_put(jbbs);
-		goto err;
+	if (bb_count) {
+		jobj = json_object_new_int(bb_count);
+		if (!jobj) {
+			json_object_put(jbbs);
+			goto err;
+		}
+		json_object_object_add(jndns, "badblock_count", jobj);
 	}
 
-	json_object_object_add(jndns, "badblock_count", jobj);
 	if (include_media_errors && jbbs)
-			json_object_object_add(jndns, "badblocks", jbbs);
+		json_object_object_add(jndns, "badblocks", jbbs);
 
 	return jndns;
  err:

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

Re: [ndctl PATCH 4/4] ndctl, list: hide badblock_count by default

[Kani, Toshimitsu]

On Thu, 2017-05-11 at 15:01 -0700, Dan Williams wrote:
> In order to keep the default listing as compact as possible, hide
> badblock_count when it is zero.
> 
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Works as expected.  For the series:

Tested-by: Toshi Kani <toshi.kani@hpe.com>

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