[ndctl PATCH 0/8] Improve support + testing for labels + info-blocks

[ndctl PATCH 0/8] Improve support + testing for labels + info-blocks

[Dan Williams]

As noted in the kernel patches for this issue:

    Lately Linux has encountered platforms that collide Persistent
    Memory regions between each other, specifically cases where
    ->start_pad needed to be non-zero. This lead to commit ae86cbfef381
    "libnvdimm, pfn: Pad pfn namespaces relative to other regions". That
    commit allowed namespaces to be mapped with devm_memremap_pages().
    However dax operations on those configurations currently fail if
    attempted within the ->start_pad range because
    pmem_device->data_offset was still relative to raw resource base not
    relative to the section aligned resource range mapped by
    devm_memremap_pages().

    Luckily __bdev_dax_supported() caught these failures and simply
    disabled dax. However, to fix this situation a non-backwards
    compatible change needs to be made to the interpretation of the
    nd_pfn info-block.  ->start_pad needs to be accounted in
    ->map.map_offset (formerly ->data_offset), and ->map.map_base
    (formerly ->phys_addr) needs to be adjusted to the section aligned
    resource base used to establish ->map.map formerly (formerly
    ->virt_addr).

Towards preventing similar bugs in this area introduce a regression
test "test/collide.sh" to validate support for pre- and post-fixed
kernels. In the course of developing this test a few missing
capabilities and fixes also surfaced.

---

Dan Williams (8):
      ndctl/dimm: Add 'flags' field to read-labels output
      ndctl/dimm: Add --human support to read-labels
      ndctl/build: Drop -Wpointer-arith
      ndctl/namespace: Add read-info-block command
      ndctl/test: Update dax-dev to handle multiple e820 ranges
      ndctl/test: Make dax.sh more robust vs small namespaces
      ndctl/namespace: Always zero info-blocks
      ndctl/test: Test inter-region collision handling


 configure.ac         |    1 
 ndctl/action.h       |    1 
 ndctl/builtin.h      |    1 
 ndctl/check.c        |   20 --
 ndctl/dimm.c         |   21 ++-
 ndctl/namespace.c    |  416 +++++++++++++++++++++++++++++++++++++++++++++++++-
 ndctl/namespace.h    |   51 ++++++
 ndctl/ndctl.c        |    1 
 test/Makefile.am     |    1 
 test/collide.sh      |  226 +++++++++++++++++++++++++++
 test/dax-dev.c       |   17 ++
 test/dax.sh          |    4 
 test/fsdax-info0.xxd |   11 +
 test/fsdax-info1.xxd |   11 +
 test/fsdax-info2.xxd |   11 +
 test/fsdax-info3.xxd |   11 +
 util/fletcher.h      |    1 
 util/size.h          |    1 
 18 files changed, 763 insertions(+), 43 deletions(-)
 create mode 100755 test/collide.sh
 create mode 100644 test/fsdax-info0.xxd
 create mode 100644 test/fsdax-info1.xxd
 create mode 100644 test/fsdax-info2.xxd
 create mode 100644 test/fsdax-info3.xxd
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 1/8] ndctl/dimm: Add 'flags' field to read-labels output

[Dan Williams]

Recent discussions on some platform implementations setting the LOCAL
bit in namespace labels makes it apparent that this field should be
decoded in the json representation.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/dimm.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index cc0bec04c3b2..292beebf9734 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -140,6 +140,11 @@ static struct json_object *dump_label_json(struct ndctl_dimm *dimm,
 			break;
 		json_object_object_add(jlabel, "nlabel", jobj);
 
+		jobj = json_object_new_int64(le64_to_cpu(nslabel.flags));
+		if (!jobj)
+			break;
+		json_object_object_add(jlabel, "flags", jobj);
+
 		jobj = json_object_new_int64(le64_to_cpu(nslabel.isetcookie));
 		if (!jobj)
 			break;

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

[ndctl PATCH 2/8] ndctl/dimm: Add --human support to read-labels

[Dan Williams]

Allow for easier comparisons between 'read-labels' output and list.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/dimm.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 292beebf9734..5ff730f42a45 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -50,6 +50,7 @@ static struct parameters {
 	bool crypto_erase;
 	bool overwrite;
 	bool master_pass;
+	bool human;
 	bool force;
 	bool json;
 	bool verbose;
@@ -79,7 +80,7 @@ static int action_zero(struct ndctl_dimm *dimm, struct action_context *actx)
 }
 
 static struct json_object *dump_label_json(struct ndctl_dimm *dimm,
-		struct ndctl_cmd *cmd_read, ssize_t size)
+		struct ndctl_cmd *cmd_read, ssize_t size, unsigned long flags)
 {
 	struct json_object *jarray = json_object_new_array();
 	struct json_object *jlabel = NULL;
@@ -140,12 +141,13 @@ static struct json_object *dump_label_json(struct ndctl_dimm *dimm,
 			break;
 		json_object_object_add(jlabel, "nlabel", jobj);
 
-		jobj = json_object_new_int64(le64_to_cpu(nslabel.flags));
+		jobj = util_json_object_hex(le32_to_cpu(nslabel.flags), flags);
 		if (!jobj)
 			break;
 		json_object_object_add(jlabel, "flags", jobj);
 
-		jobj = json_object_new_int64(le64_to_cpu(nslabel.isetcookie));
+		jobj = util_json_object_hex(le64_to_cpu(nslabel.isetcookie),
+				flags);
 		if (!jobj)
 			break;
 		json_object_object_add(jlabel, "isetcookie", jobj);
@@ -155,12 +157,12 @@ static struct json_object *dump_label_json(struct ndctl_dimm *dimm,
 			break;
 		json_object_object_add(jlabel, "lbasize", jobj);
 
-		jobj = json_object_new_int64(le64_to_cpu(nslabel.dpa));
+		jobj = util_json_object_hex(le64_to_cpu(nslabel.dpa), flags);
 		if (!jobj)
 			break;
 		json_object_object_add(jlabel, "dpa", jobj);
 
-		jobj = json_object_new_int64(le64_to_cpu(nslabel.rawsize));
+		jobj = util_json_object_size(le64_to_cpu(nslabel.rawsize), flags);
 		if (!jobj)
 			break;
 		json_object_object_add(jlabel, "rawsize", jobj);
@@ -265,6 +267,7 @@ static struct json_object *dump_index_json(struct ndctl_cmd *cmd_read, ssize_t s
 static struct json_object *dump_json(struct ndctl_dimm *dimm,
 		struct ndctl_cmd *cmd_read, ssize_t size)
 {
+	unsigned long flags = param.human ? UTIL_JSON_HUMAN : 0;
 	struct json_object *jdimm = json_object_new_object();
 	struct json_object *jlabel, *jobj, *jindex;
 
@@ -273,7 +276,7 @@ static struct json_object *dump_json(struct ndctl_dimm *dimm,
 	jindex = dump_index_json(cmd_read, size);
 	if (!jindex)
 		goto err_jindex;
-	jlabel = dump_label_json(dimm, cmd_read, size);
+	jlabel = dump_label_json(dimm, cmd_read, size, flags);
 	if (!jlabel)
 		goto err_jlabel;
 
@@ -1038,7 +1041,8 @@ OPT_BOOLEAN('v',"verbose", &param.verbose, "turn on debug")
 #define READ_OPTIONS() \
 OPT_STRING('o', "output", &param.outfile, "output-file", \
 	"filename to write label area contents"), \
-OPT_BOOLEAN('j', "json", &param.json, "parse label data into json")
+OPT_BOOLEAN('j', "json", &param.json, "parse label data into json"), \
+OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats ")
 
 #define WRITE_OPTIONS() \
 OPT_STRING('i', "input", &param.infile, "input-file", \

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

[ndctl PATCH 3/8] ndctl/build: Drop -Wpointer-arith

[Dan Williams]

It is common kernel practice to do pointer arithmetic on (void *). Drop
this warning for ndctl.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 configure.ac |    1 -
 1 file changed, 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index d27a2b1cf5a2..25b785f9d04f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -190,7 +190,6 @@ my_CFLAGS="\
 -Wmissing-declarations \
 -Wmissing-prototypes \
 -Wnested-externs \
--Wpointer-arith \
 -Wshadow \
 -Wsign-compare \
 -Wstrict-prototypes \

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

[ndctl PATCH 4/8] ndctl/namespace: Add read-info-block command

[Dan Williams]

Namespaces may contain an info-block that correlates with the possible
modes of a namespace: 'sector', 'fsdax', or 'devdax'. Provide a command
that can temporarily convert the namespace into 'raw' mode to read the
info-block.

Also, similar to 'read-labels' provide a facility to decode the
info-block into json.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/action.h    |    1 
 ndctl/builtin.h   |    1 
 ndctl/check.c     |   20 ---
 ndctl/namespace.c |  401 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ndctl/namespace.h |   51 +++++++
 ndctl/ndctl.c     |    1 
 util/fletcher.h   |    1 
 util/size.h       |    1 
 8 files changed, 456 insertions(+), 21 deletions(-)

diff --git a/ndctl/action.h b/ndctl/action.h
index 1ecad49530d5..01cfce511250 100644
--- a/ndctl/action.h
+++ b/ndctl/action.h
@@ -13,5 +13,6 @@ enum device_action {
 	ACTION_CHECK,
 	ACTION_WAIT,
 	ACTION_START,
+	ACTION_READ_INFOBLOCK,
 };
 #endif /* __NDCTL_ACTION_H__ */
diff --git a/ndctl/builtin.h b/ndctl/builtin.h
index 681a69ff3770..985b7937d884 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -8,6 +8,7 @@ int cmd_create_nfit(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_enable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_destroy_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_read_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_disable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_check_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_enable_region(int argc, const char **argv, struct ndctl_ctx *ctx);
diff --git a/ndctl/check.c b/ndctl/check.c
index 8a7125053865..3b384c091af5 100644
--- a/ndctl/check.c
+++ b/ndctl/check.c
@@ -297,24 +297,6 @@ static int btt_log_read(struct arena_info *a, u32 lane, struct log_entry *ent)
 	return 0;
 }
 
-static int btt_checksum_verify(struct btt_sb *btt_sb)
-{
-	uint64_t sum;
-	le64 sum_save;
-
-	BUILD_BUG_ON(sizeof(struct btt_sb) != SZ_4K);
-
-	sum_save = btt_sb->checksum;
-	btt_sb->checksum = 0;
-	sum = fletcher64(btt_sb, sizeof(*btt_sb), 1);
-	if (sum != sum_save)
-		return 1;
-	/* restore the checksum in the buffer */
-	btt_sb->checksum = sum_save;
-
-	return 0;
-}
-
 /*
  * Never pass a mmapped buffer to this as it will attempt to write to
  * the buffer, and we want writes to only happened in a controlled fashion.
@@ -330,7 +312,7 @@ static int btt_info_verify(struct btt_chk *bttc, struct btt_sb *btt_sb)
 		if (uuid_compare(bttc->parent_uuid, btt_sb->parent_uuid) != 0)
 			return -ENXIO;
 
-	if (btt_checksum_verify(btt_sb))
+	if (!verify_infoblock_checksum((union info_block *) btt_sb))
 		return -ENXIO;
 
 	return 0;
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 03d805a69ce4..6e42acb695aa 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -21,6 +21,7 @@
 
 #include <ndctl.h>
 #include "action.h"
+#include "namespace.h"
 #include <sys/stat.h>
 #include <uuid/uuid.h>
 #include <sys/types.h>
@@ -40,6 +41,9 @@ static struct parameters {
 	bool do_scan;
 	bool mode_default;
 	bool autolabel;
+	bool no_verify;
+	bool human;
+	bool json;
 	const char *bus;
 	const char *map;
 	const char *type;
@@ -51,6 +55,8 @@ static struct parameters {
 	const char *reconfig;
 	const char *sector_size;
 	const char *align;
+	const char *outfile;
+	const char *infile;
 } param = {
 	.autolabel = true,
 };
@@ -79,6 +85,13 @@ struct parsed_parameters {
 	bool autolabel;
 };
 
+#define pr_verbose(fmt, ...) \
+	({if (verbose) { \
+		fprintf(stderr, fmt, ##__VA_ARGS__); \
+	} else { \
+		do { } while (0); \
+	}})
+
 #define debug(fmt, ...) \
 	({if (verbose) { \
 		fprintf(stderr, "%s:%d: " fmt, __func__, __LINE__, ##__VA_ARGS__); \
@@ -120,6 +133,16 @@ OPT_BOOLEAN('R', "repair", &repair, "perform metadata repairs"), \
 OPT_BOOLEAN('L', "rewrite-log", &logfix, "regenerate the log"), \
 OPT_BOOLEAN('f', "force", &force, "check namespace even if currently active")
 
+#define READ_INFOBLOCK_OPTIONS() \
+OPT_FILENAME('o', "output", &param.outfile, "output-file", \
+	"filename to write label area contents"), \
+OPT_FILENAME('f', "file", &param.infile, "input-file", \
+	"filename to readinfo block instead of a namespace"), \
+OPT_BOOLEAN('n', "no-verify", &param.no_verify, \
+	"skip parent uuid, and checksum validation"), \
+OPT_BOOLEAN('j', "json", &param.json, "parse label data into json"), \
+OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats ")
+
 static const struct option base_options[] = {
 	BASE_OPTIONS(),
 	OPT_END(),
@@ -144,6 +167,12 @@ static const struct option check_options[] = {
 	OPT_END(),
 };
 
+static const struct option read_infoblock_options[] = {
+	BASE_OPTIONS(),
+	READ_INFOBLOCK_OPTIONS(),
+	OPT_END(),
+};
+
 static int set_defaults(enum device_action mode)
 {
 	int rc = 0;
@@ -285,18 +314,30 @@ static const char *parse_namespace_options(int argc, const char **argv,
 			case ACTION_CHECK:
 				action_string = "check";
 				break;
+			case ACTION_READ_INFOBLOCK:
+				action_string = "read-infoblock";
+				break;
 			default:
 				action_string = "<>";
 				break;
 		}
-		error("specify a namespace to %s, or \"all\"\n", action_string);
-		rc = -EINVAL;
+
+		if ((mode == ACTION_READ_INFOBLOCK && !param.infile)
+				|| mode != ACTION_READ_INFOBLOCK) {
+			error("specify a namespace to %s, or \"all\"\n", action_string);
+			rc = -EINVAL;
+		}
 	}
 	for (i = mode == ACTION_CREATE ? 0 : 1; i < argc; i++) {
 		error("unknown extra parameter \"%s\"\n", argv[i]);
 		rc = -EINVAL;
 	}
 
+	if (mode == ACTION_READ_INFOBLOCK && param.infile && argc) {
+		error("specify a namespace, or --file, not both\n");
+		rc = -EINVAL;
+	}
+
 	if (rc) {
 		usage_with_options(u, options);
 		return NULL; /* we won't return from usage_with_options() */
@@ -1051,10 +1092,319 @@ static int namespace_reconfig(struct ndctl_region *region,
 int namespace_check(struct ndctl_namespace *ndns, bool verbose, bool force,
 		bool repair, bool logfix);
 
+struct read_infoblock_ctx {
+	struct json_object *jblocks;
+	FILE *f_out;
+};
+
+#define parse_field(sb, field)						\
+	jobj = json_object_new_int(le32_to_cpu((sb)->field));		\
+	if (!jobj)							\
+		goto err;						\
+	json_object_object_add(jblock, #field, jobj);
+
+#define parse_hex(sb, field, sz)						\
+	jobj = util_json_object_hex(le##sz##_to_cpu((sb)->field), flags);	\
+	if (!jobj)								\
+		goto err;							\
+	json_object_object_add(jblock, #field, jobj);
+
+
+static json_object *btt_parse(struct btt_sb *btt_sb, struct ndctl_namespace *ndns,
+		const char *path, unsigned long flags)
+{
+	uuid_t uuid;
+	char str[40];
+	struct json_object *jblock, *jobj;
+	const char *cmd = "read-info-block";
+	const bool verify = !param.no_verify;
+
+	if (verify && !verify_infoblock_checksum((union info_block *) btt_sb)) {
+		pr_verbose("%s: %s checksum verification failed\n", cmd, __func__);
+		return NULL;
+	}
+
+	if (ndns) {
+		ndctl_namespace_get_uuid(ndns, uuid);
+		if (verify && !uuid_is_null(uuid) && memcmp(uuid, btt_sb->parent_uuid,
+					sizeof(uuid) != 0)) {
+			pr_verbose("%s: %s uuid verification failed\n", cmd, __func__);
+			return NULL;
+		}
+	}
+
+	jblock = json_object_new_object();
+	if (!jblock)
+		return NULL;
+
+	if (ndns) {
+		jobj = json_object_new_string(ndctl_namespace_get_devname(ndns));
+		if (!jobj)
+			goto err;
+		json_object_object_add(jblock, "dev", jobj);
+	} else {
+		jobj = json_object_new_string(path);
+		if (!jobj)
+			goto err;
+		json_object_object_add(jblock, "file", jobj);
+	}
+
+	jobj = json_object_new_string((char *) btt_sb->signature);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "signature", jobj);
+
+	uuid_unparse((void *) btt_sb->uuid, str);
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "uuid", jobj);
+
+	uuid_unparse((void *) btt_sb->parent_uuid, str);
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "parent_uuid", jobj);
+
+	jobj = util_json_object_hex(le32_to_cpu(btt_sb->flags), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "flags", jobj);
+
+	if (snprintf(str, 4, "%d.%d", btt_sb->version_major,
+				btt_sb->version_minor) >= 4)
+		goto err;
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "version", jobj);
+
+	parse_field(btt_sb, external_lbasize);
+	parse_field(btt_sb, external_nlba);
+	parse_field(btt_sb, internal_lbasize);
+	parse_field(btt_sb, internal_nlba);
+	parse_field(btt_sb, nfree);
+	parse_field(btt_sb, infosize);
+	parse_hex(btt_sb, nextoff, 64);
+	parse_hex(btt_sb, dataoff, 64);
+	parse_hex(btt_sb, mapoff, 64);
+	parse_hex(btt_sb, logoff, 64);
+	parse_hex(btt_sb, info2off, 64);
+
+	return jblock;
+err:
+	pr_verbose("%s: failed to create json representation\n", cmd);
+	json_object_put(jblock);
+	return NULL;
+}
+
+static json_object *pfn_parse(struct pfn_sb *pfn_sb, struct ndctl_namespace *ndns,
+		const char *path, unsigned long flags)
+{
+	uuid_t uuid;
+	char str[40];
+	struct json_object *jblock, *jobj;
+	const char *cmd = "read-info-block";
+	const bool verify = !param.no_verify;
+
+	if (verify && !verify_infoblock_checksum((union info_block *) pfn_sb)) {
+		pr_verbose("%s: %s checksum verification failed\n", cmd, __func__);
+		return NULL;
+	}
+
+	if (ndns) {
+		ndctl_namespace_get_uuid(ndns, uuid);
+		if (verify && !uuid_is_null(uuid) && memcmp(uuid, pfn_sb->parent_uuid,
+					sizeof(uuid) != 0)) {
+			pr_verbose("%s: %s uuid verification failed\n", cmd, __func__);
+			return NULL;
+		}
+	}
+
+	jblock = json_object_new_object();
+	if (!jblock)
+		return NULL;
+
+	if (ndns) {
+		jobj = json_object_new_string(ndctl_namespace_get_devname(ndns));
+		if (!jobj)
+			goto err;
+		json_object_object_add(jblock, "dev", jobj);
+	} else {
+		jobj = json_object_new_string(path);
+		if (!jobj)
+			goto err;
+		json_object_object_add(jblock, "file", jobj);
+	}
+
+	jobj = json_object_new_string((char *) pfn_sb->signature);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "signature", jobj);
+
+	uuid_unparse((void *) pfn_sb->uuid, str);
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "uuid", jobj);
+
+	uuid_unparse((void *) pfn_sb->parent_uuid, str);
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "parent_uuid", jobj);
+
+	jobj = util_json_object_hex(le32_to_cpu(pfn_sb->flags), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "flags", jobj);
+
+	if (snprintf(str, 4, "%d.%d", pfn_sb->version_major,
+				pfn_sb->version_minor) >= 4)
+		goto err;
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "version", jobj);
+
+	parse_hex(pfn_sb, dataoff, 64);
+	parse_hex(pfn_sb, npfns, 64);
+	parse_field(pfn_sb, mode);
+	parse_hex(pfn_sb, start_pad, 32);
+	parse_hex(pfn_sb, end_trunc, 32);
+	parse_hex(pfn_sb, align, 32);
+
+	return jblock;
+err:
+	pr_verbose("%s: failed to create json representation\n", cmd);
+	json_object_put(jblock);
+	return NULL;
+}
+
+#define INFOBLOCK_SZ SZ_8K
+
+static int parse_namespace_infoblock(char *_buf, struct ndctl_namespace *ndns,
+		const char *path, struct read_infoblock_ctx *ri_ctx)
+{
+	int rc;
+	void *buf = _buf;
+	unsigned long flags = param.human ? UTIL_JSON_HUMAN : 0;
+	struct btt_sb *btt1_sb = buf + SZ_4K, *btt2_sb = buf;
+	struct json_object *jblock = NULL, *jblocks = ri_ctx->jblocks;
+	struct pfn_sb *pfn_sb = buf + SZ_4K, *dax_sb = buf + SZ_4K;
+
+	if (!param.json) {
+		rc = fwrite(buf, 1, INFOBLOCK_SZ, ri_ctx->f_out);
+		if (rc != INFOBLOCK_SZ)
+			return -EIO;
+		fflush(ri_ctx->f_out);
+		return 0;
+	}
+
+	if (!jblocks) {
+		jblocks = json_object_new_array();
+		if (!jblocks)
+			return -ENOMEM;
+		ri_ctx->jblocks = jblocks;
+	}
+
+	if (memcmp(btt1_sb->signature, BTT_SIG, BTT_SIG_LEN) == 0) {
+		jblock = btt_parse(btt1_sb, ndns, path, flags);
+		if (jblock)
+			json_object_array_add(jblocks, jblock);
+	}
+
+	if (memcmp(btt2_sb->signature, BTT_SIG, BTT_SIG_LEN) == 0) {
+		jblock = btt_parse(btt2_sb, ndns, path, flags);
+		if (jblock)
+			json_object_array_add(jblocks, jblock);
+	}
+
+	if (memcmp(pfn_sb->signature, PFN_SIG, PFN_SIG_LEN) == 0) {
+		jblock = pfn_parse(pfn_sb, ndns, path, flags);
+		if (jblock)
+			json_object_array_add(jblocks, jblock);
+	}
+
+	if (memcmp(dax_sb->signature, DAX_SIG, PFN_SIG_LEN) == 0) {
+		jblock = pfn_parse(dax_sb, ndns, path, flags);
+		if (jblock)
+			json_object_array_add(jblocks, jblock);
+	}
+
+	return 0;
+}
+
+static int file_read_infoblock(const char *path, struct ndctl_namespace *ndns,
+		struct read_infoblock_ctx *ri_ctx)
+{
+	const char *devname = ndns ? ndctl_namespace_get_devname(ndns) : "";
+	const char *cmd = "read-info-block";
+	void *buf = NULL;
+	int fd = -1, rc;
+
+	buf = calloc(1, INFOBLOCK_SZ);
+	if (!buf)
+		return -ENOMEM;
+
+	fd = open(path, O_RDONLY|O_EXCL);
+	if (fd < 0) {
+		pr_verbose("%s: %s failed to open %s: %s\n",
+				cmd, devname, path, strerror(errno));
+		rc = -errno;
+		goto out;
+	}
+
+	rc = read(fd, buf, INFOBLOCK_SZ);
+	if (rc < 0) {
+		pr_verbose("%s: %s failed to read %s: %s\n",
+				cmd, devname, path, strerror(errno));
+		rc = -errno;
+		goto out;
+	}
+
+	rc = parse_namespace_infoblock(buf, ndns, path, ri_ctx);
+out:
+	free(buf);
+	if (fd >= 0)
+		close(fd);
+	return rc;
+}
+
+static int namespace_read_infoblock(struct ndctl_namespace *ndns,
+		struct read_infoblock_ctx *ri_ctx)
+{
+	int rc;
+	char path[50];
+	const char *cmd = "read-info-block";
+	const char *devname = ndctl_namespace_get_devname(ndns);
+
+	if (ndctl_namespace_is_active(ndns)) {
+		pr_verbose("%s: %s enabled, must be disabled\n", cmd, devname);
+		return -EBUSY;
+	}
+
+	ndctl_namespace_set_raw_mode(ndns, 1);
+	rc = ndctl_namespace_enable(ndns);
+	if (rc < 0) {
+		pr_verbose("%s: %s failed to enable\n", cmd, devname);
+		goto out;
+	}
+
+	sprintf(path, "/dev/%s", ndctl_namespace_get_block_device(ndns));
+	rc = file_read_infoblock(path, ndns, ri_ctx);
+
+out:
+	ndctl_namespace_set_raw_mode(ndns, 0);
+	ndctl_namespace_disable_invalidate(ndns);
+	return rc;
+}
+
 static int do_xaction_namespace(const char *namespace,
 		enum device_action action, struct ndctl_ctx *ctx,
 		int *processed)
 {
+	struct read_infoblock_ctx ri_ctx = { 0 };
 	struct ndctl_namespace *ndns, *_n;
 	struct ndctl_region *region;
 	const char *ndns_name;
@@ -1063,6 +1413,26 @@ static int do_xaction_namespace(const char *namespace,
 
 	*processed = 0;
 
+	if (action == ACTION_READ_INFOBLOCK) {
+		if (!param.outfile)
+			ri_ctx.f_out = stdout;
+		else {
+			ri_ctx.f_out = fopen(param.outfile, "w+");
+			if (!ri_ctx.f_out) {
+				fprintf(stderr, "failed to open: %s: (%s)\n",
+						param.outfile, strerror(errno));
+				return -errno;
+			}
+		}
+
+		if (param.infile) {
+			rc = file_read_infoblock(param.infile, NULL, &ri_ctx);
+			if (ri_ctx.jblocks)
+				util_display_json_array(ri_ctx.f_out, ri_ctx.jblocks, 0);
+			return rc;
+		}
+	}
+
 	if (!namespace && action != ACTION_CREATE)
 		return rc;
 
@@ -1136,6 +1506,11 @@ static int do_xaction_namespace(const char *namespace,
 					if (rc == 0)
 						*processed = 1;
 					return rc;
+				case ACTION_READ_INFOBLOCK:
+					rc = namespace_read_infoblock(ndns, &ri_ctx);
+					if (rc == 0)
+						(*processed)++;
+					break;
 				default:
 					rc = -EINVAL;
 					break;
@@ -1144,6 +1519,12 @@ static int do_xaction_namespace(const char *namespace,
 		}
 	}
 
+	if (ri_ctx.jblocks)
+		util_display_json_array(ri_ctx.f_out, ri_ctx.jblocks, 0);
+
+	if (ri_ctx.f_out && ri_ctx.f_out != stdout)
+		fclose(ri_ctx.f_out);
+
 	return rc;
 }
 
@@ -1240,3 +1621,19 @@ int cmd_check_namespace(int argc , const char **argv, struct ndctl_ctx *ctx)
 			checked == 1 ? "" : "s");
 	return rc;
 }
+
+int cmd_read_infoblock(int argc , const char **argv, struct ndctl_ctx *ctx)
+{
+	char *xable_usage = "ndctl read-info-block <namespace> [<options>]";
+	const char *namespace = parse_namespace_options(argc, argv,
+			ACTION_READ_INFOBLOCK, read_infoblock_options,
+			xable_usage);
+	int read, rc;
+
+	rc = do_xaction_namespace(namespace, ACTION_READ_INFOBLOCK, ctx, &read);
+	if (rc < 0)
+		fprintf(stderr, "error checking namespaces: %s\n",
+				strerror(-rc));
+	fprintf(stderr, "read %d namespace%s\n", read, read == 1 ? "" : "s");
+	return rc;
+}
diff --git a/ndctl/namespace.h b/ndctl/namespace.h
index bc210857642f..861dfbfa5127 100644
--- a/ndctl/namespace.h
+++ b/ndctl/namespace.h
@@ -13,7 +13,9 @@
 #ifndef __NDCTL_NAMESPACE_H__
 #define __NDCTL_NAMESPACE_H__
 #include <sys/types.h>
+#include <util/util.h>
 #include <util/size.h>
+#include <util/fletcher.h>
 #include <ccan/endian/endian.h>
 #include <ccan/short_types/short_types.h>
 
@@ -202,4 +204,53 @@ struct arena_map {
 	struct btt_sb *info2;
 	size_t info2_len;
 };
+
+#define PFN_SIG_LEN 16
+#define PFN_SIG "NVDIMM_PFN_INFO\0"
+#define DAX_SIG "NVDIMM_DAX_INFO\0"
+
+struct pfn_sb {
+	u8 signature[PFN_SIG_LEN];
+	u8 uuid[16];
+	u8 parent_uuid[16];
+	le32 flags;
+	le16 version_major;
+	le16 version_minor;
+	le64 dataoff; /* relative to namespace_base + start_pad */
+	le64 npfns;
+	le32 mode;
+	/* minor-version-1 additions for section alignment */
+	le32 start_pad;
+	le32 end_trunc;
+	/* minor-version-2 record the base alignment of the mapping */
+	le32 align;
+	u8 padding[4000];
+	le64 checksum;
+};
+
+union info_block {
+	struct pfn_sb pfn_sb;
+	struct btt_sb btt_sb;
+};
+
+static inline bool verify_infoblock_checksum(union info_block *sb)
+{
+	uint64_t sum;
+	le64 sum_save;
+
+	BUILD_BUG_ON(sizeof(union info_block) != SZ_4K);
+
+	/* all infoblocks share the btt_sb layout for checksum */
+	sum_save = sb->btt_sb.checksum;
+	sb->btt_sb.checksum = 0;
+	sum = fletcher64(&sb->btt_sb, sizeof(*sb), 1);
+	if (sum != sum_save)
+		return false;
+	/* restore the checksum in the buffer */
+	sb->btt_sb.checksum = sum_save;
+
+	return true;
+}
+
+
 #endif /* __NDCTL_NAMESPACE_H__ */
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index bd333b292743..5018d486e2c1 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -73,6 +73,7 @@ static struct cmd_struct commands[] = {
 	{ "disable-namespace", { cmd_disable_namespace } },
 	{ "create-namespace", { cmd_create_namespace } },
 	{ "destroy-namespace", { cmd_destroy_namespace } },
+	{ "read-infoblock",  { cmd_read_infoblock } },
 	{ "check-namespace", { cmd_check_namespace } },
 	{ "enable-region", { cmd_enable_region } },
 	{ "disable-region", { cmd_disable_region } },
diff --git a/util/fletcher.h b/util/fletcher.h
index 54e2ecf5d6ed..8fccac4ec758 100644
--- a/util/fletcher.h
+++ b/util/fletcher.h
@@ -1,6 +1,7 @@
 #ifndef _NDCTL_FLETCHER_H_
 #define _NDCTL_FLETCHER_H_
 
+#include <stdbool.h>
 #include <ccan/endian/endian.h>
 #include <ccan/short_types/short_types.h>
 
diff --git a/util/size.h b/util/size.h
index 34fac58d6945..2426fef74b3c 100644
--- a/util/size.h
+++ b/util/size.h
@@ -16,6 +16,7 @@
 
 #define SZ_1K     0x00000400
 #define SZ_4K     0x00001000
+#define SZ_8K     0x00002000
 #define SZ_1M     0x00100000
 #define SZ_2M     0x00200000
 #define SZ_4M     0x00400000

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

[ndctl PATCH 5/8] ndctl/test: Update dax-dev to handle multiple e820 ranges

[Dan Williams]

Establish a convention that the first, lowest-index, region on the e820
bus always enables in fsdax mode without need for an nd_pfn instance.
This is in preparation for a defining a collision test that requires
multiple section-mis-aligned regions defined by memmap=nn!ss.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/dax-dev.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/test/dax-dev.c b/test/dax-dev.c
index 0183b7af2052..49ccaa334e31 100644
--- a/test/dax-dev.c
+++ b/test/dax-dev.c
@@ -33,17 +33,28 @@ struct ndctl_namespace *ndctl_get_test_dev(struct ndctl_ctx *ctx)
 	struct ndctl_bus *bus;
 	struct ndctl_dax *dax;
 	struct ndctl_pfn *pfn;
-	struct ndctl_region *region;
 	struct ndctl_namespace *ndns;
 	enum ndctl_namespace_mode mode;
+	struct ndctl_region *region, *min = NULL;
 
 	bus = ndctl_bus_get_by_provider(ctx, "e820");
 	if (!bus)
 		goto out;
 
-	region = ndctl_region_get_first(bus);
-	if (!region)
+	ndctl_region_foreach(bus, region) {
+		if (!min) {
+			min = region;
+			continue;
+		}
+		if (ndctl_region_get_id(region) < ndctl_region_get_id(min))
+			min = region;
+	}
+	if (!min)
 		goto out;
+	region = min;
+
+	/* attempt to re-enable the region if a previous test disabled it */
+	ndctl_region_enable(region);
 
 	ndns = ndctl_namespace_get_first(region);
 	if (!ndns)

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

[ndctl PATCH 6/8] ndctl/test: Make dax.sh more robust vs small namespaces

[Dan Williams]

If the namespace returned by test/dax-dev is too small ext4 may default
to a 1K block-size. A 1K block-size precludes dax operation, so force a
4K block-size in all cases.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/dax.sh |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/dax.sh b/test/dax.sh
index d38fd01acb07..e703e1222dee 100755
--- a/test/dax.sh
+++ b/test/dax.sh
@@ -47,7 +47,7 @@ json=$($NDCTL list -N -n $dev)
 eval $(json2var <<< "$json")
 rc=1
 
-mkfs.ext4 /dev/$blockdev
+mkfs.ext4 -b 4096 /dev/$blockdev
 mount /dev/$blockdev $MNT -o dax
 fallocate -l 1GiB $MNT/$FILE
 run_test
@@ -59,7 +59,7 @@ eval $(json2var <<< "$json")
 [ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
 
 #note the blockdev returned from ndctl create-namespace lacks the /dev prefix
-mkfs.ext4 /dev/$blockdev
+mkfs.ext4 -b 4096 /dev/$blockdev
 mount /dev/$blockdev $MNT -o dax
 fallocate -l 1GiB $MNT/$FILE
 run_test

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

[ndctl PATCH 7/8] ndctl/namespace: Always zero info-blocks

[Dan Williams]

Do not gate zeroing on whether a namespace is claimed by a personality.
The namespace might not have been able to be claimed due to info-block
corruption.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 6e42acb695aa..63b870ff7a6d 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -917,9 +917,6 @@ static int namespace_destroy(struct ndctl_region *region,
 		struct ndctl_namespace *ndns)
 {
 	const char *devname = ndctl_namespace_get_devname(ndns);
-	struct ndctl_pfn *pfn = ndctl_namespace_get_pfn(ndns);
-	struct ndctl_dax *dax = ndctl_namespace_get_dax(ndns);
-	struct ndctl_btt *btt = ndctl_namespace_get_btt(ndns);
 	bool did_zero = false;
 	int rc;
 
@@ -941,13 +938,11 @@ static int namespace_destroy(struct ndctl_region *region,
 
 	ndctl_namespace_set_enforce_mode(ndns, NDCTL_NS_MODE_RAW);
 
-	if (pfn || btt || dax) {
-		rc = zero_info_block(ndns);
-		if (rc < 0)
-			return rc;
-		if (rc == 0)
-			did_zero = true;
-	}
+	rc = zero_info_block(ndns);
+	if (rc < 0)
+		return rc;
+	if (rc == 0)
+		did_zero = true;
 
 	switch (ndctl_namespace_get_type(ndns)) {
         case ND_DEVICE_NAMESPACE_PMEM:

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

[ndctl PATCH 8/8] ndctl/test: Test inter-region collision handling

[Dan Williams]

Given the discovery of a broken padding implementation in the kernel
introduce a regression test to validate collision handling and dax
operation. It is known to fail as of v5.0-rc1.

The test relies on a custom memmap= configuration to generate the
problematic physical alignment condition. Details of the required memmap
configuration are listed / validated in the test script.

The test also validates that the kernel implementation preserves the
pre-v1.3 info block behavior while implementing the new v1.3+ behavior.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/Makefile.am     |    1 
 test/collide.sh      |  226 ++++++++++++++++++++++++++++++++++++++++++++++++++
 test/fsdax-info0.xxd |   11 ++
 test/fsdax-info1.xxd |   11 ++
 test/fsdax-info2.xxd |   11 ++
 test/fsdax-info3.xxd |   11 ++
 6 files changed, 271 insertions(+)
 create mode 100755 test/collide.sh
 create mode 100644 test/fsdax-info0.xxd
 create mode 100644 test/fsdax-info1.xxd
 create mode 100644 test/fsdax-info2.xxd
 create mode 100644 test/fsdax-info3.xxd

diff --git a/test/Makefile.am b/test/Makefile.am
index 42009c310b14..2f7cf5f24afd 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -53,6 +53,7 @@ TESTS +=\
 	dax.sh \
 	device-dax \
 	device-dax-fio.sh \
+	collide.sh \
 	mmap.sh
 
 check_PROGRAMS +=\
diff --git a/test/collide.sh b/test/collide.sh
new file mode 100755
index 000000000000..65b2664bbcc3
--- /dev/null
+++ b/test/collide.sh
@@ -0,0 +1,226 @@
+#!/bin/bash -x
+
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2015-2019 Intel Corporation. All rights reserved.
+
+set -e
+
+SKIP=77
+FAIL=1
+SUCCESS=0
+
+. ./common
+
+check_min_kver "5.0" || do_skip "may lack align collision fixes"
+check_prereq xxd
+
+MNT=test_dax_mnt
+mkdir -p $MNT
+
+rc=$FAIL
+cleanup() {
+	if [ $rc -ne $SUCCESS ]; then
+		echo "test/collide.sh: failed at line $1"
+	fi
+	if mountpoint -q $MNT; then
+		umount $MNT
+	fi
+	rmdir $MNT
+	# opportunistic cleanup, not fatal if these fail
+	for i in $regions
+	do
+		if ! $NDCTL destroy-namespace -f -r $i all; then
+			echo "test/collide.sh: cleanup() failed to destroy $i"
+		fi
+		if ! $NDCTL enable-namespace -r $i all; then
+			echo "test/collide.sh: cleanup() failed to re-enable $i"
+		fi
+	done
+	exit $rc
+}
+
+
+trap 'err $LINENO cleanup' ERR
+
+modprobe nd_e820
+
+# Require TEST_NUM consecutive regions of TEST_SIZE in size, where
+# TEST_SIZE causes every other namespace to collide within a 128MB
+# boundary. I.e. trigger the kernel to allocate a 'start_pad' area. As
+# of writing, an example memmap= configuration this test expects is:
+#    memmap=$((4      << 30))!$((( 7 << 30)))
+#    memmap=$((64 * 3 << 20))!$(((11 << 30)))
+#    memmap=$((64 * 3 << 20))!$(((11 << 30) + (64 * 3 << 20)))
+#    memmap=$((64 * 3 << 20))!$(((11 << 30) + (64 * 6 << 20)))
+#    memmap=$((64 * 3 << 20))!$(((11 << 30) + (64 * 9 << 20)))
+# ...where the first region is the area for tests that depend on the
+# namespace identified by test/dax-dev, and the rest are four namespace
+# that collide per the expectations of this test.
+TEST_NUM=4
+TEST_SIZE=$((64*3 << 20))
+json=$($NDCTL list -Ri -b e820)
+len=$(echo $json | jq length)
+if [ $len -lt $TEST_NUM ]; then
+	exit $SKIP
+fi
+
+found=0
+prev_resource=0
+regions=$(echo $json | jq -r ". | sort_by(.dev) | .[].dev")
+first_region=""
+for i in $regions
+do
+	resource=$(cat /sys/bus/nd/devices/$i/resource)
+	size=$(cat /sys/bus/nd/devices/$i/size)
+	if [ $((size)) -ne $TEST_SIZE ]; then
+		prev_resource=0
+		found=0
+		continue
+	fi
+	if [ $((prev_resource)) -gt 0 ]; then
+		if [ $((prev_resource + TEST_SIZE)) -ne $((resource)) ]; then
+			prev_resource=0
+			found=0
+		else
+			prev_resource=$resource
+			found=$((found+1))
+		fi
+	else
+		first_region=$i
+		prev_resource=$resource
+		found=1
+	fi
+done
+if [ $found -lt $TEST_NUM ]; then
+	echo "failed to find $TEST_NUM regions of size $((TEST_SIZE >> 20))MB"
+	exit $FAIL
+fi
+
+region_id() {
+	echo ${1:6}
+}
+
+reset_region() {
+	id=$(region_id $1)
+	idx=$2
+	ns="namespace${id}.0"
+	# clear out any existing info blocks
+	$NDCTL enable-region $1
+	$NDCTL destroy-namespace $ns -f
+
+	# check that every other namespace fails to auto-enable due to
+	# dev_pagemap collisions
+	if $NDCTL enable-namespace $ns; then
+		if [ $((idx & 1)) -eq 1 ]; then
+			echo "expected enable region$id (idx: $idx) failure"
+			exit $FAIL
+		fi
+	else
+		if [ $((idx & 1)) -eq 0 ]; then
+			echo "expected enable region$id (idx: $idx) success"
+			exit $FAIL
+		fi
+	fi
+}
+
+restore_region() {
+	id=$(region_id $1)
+	idx=$2
+	ns="namespace${id}.0"
+	# restore saved v1.2 info block
+	$NDCTL enable-region $1
+	$NDCTL destroy-namespace $ns -f
+	echo 1 > /sys/bus/nd/devices/$ns/force_raw
+	$NDCTL enable-namespace $ns
+	xxd -r fsdax-info$idx.xxd /dev/pmem$id
+	blockdev --flushbufs /dev/pmem$id
+	echo 0 > /sys/bus/nd/devices/$ns/force_raw
+	$NDCTL disable-namespace $ns
+
+	# check size
+	$NDCTL enable-namespace $ns
+	size=$($NDCTL list -n $ns | jq -r '.[].size')
+	if [ $size -ne 130023424 ]; then
+		false
+	fi
+
+	# check for every other @idx dax-mount fails
+	mkfs.ext4 -b 4096 /dev/pmem$id
+	fail=0
+	mount -o dax /dev/pmem$id $MNT || fail=1
+	if [ $fail -eq 0 ]; then
+		umount $MNT
+		if [ $((idx & 1)) -eq 1 ]; then
+			echo "expected dax-mount region$id (idx: $idx) failure"
+			false
+		fi
+	else
+		if [ $((idx & 1)) -eq 0 ]; then
+			echo "expected dax-mount region$id (idx: $idx) success"
+			false
+		fi
+	fi
+}
+
+# validate pre-5.1 info block handling
+idx=0
+$NDCTL disable-region all -b e820
+for i in $regions
+do
+	if [ $(region_id $i) -lt $(region_id $first_region) ]; then
+		continue
+	fi
+	restore_region $i $idx
+	idx=$((idx+1))
+done
+
+# validate alignment collision assumptions
+idx=0
+$NDCTL disable-region -b e820 all
+for i in $regions
+do
+	if [ $(region_id $i) -lt $(region_id $first_region) ]; then
+		continue
+	fi
+	reset_region $i $idx
+	idx=$((idx+1))
+done
+
+# use ext4 to check for successful dax mount
+check_dax() {
+	id=$(region_id $1)
+	idx=$2
+	ns="namespace${id}.0"
+
+	$NDCTL create-namespace -e $ns -m fsdax -M dev -f
+	mkfs.ext4 -b 4096 /dev/pmem$id
+	mount -o dax /dev/pmem$id $MNT
+	umount $MNT
+}
+
+check_size() {
+	id=$(region_id $1)
+	ns="namespace${id}.0"
+	pfn_size=$($NDCTL list -n $ns | jq -r ".[] | .size")
+	bdev_size=$(blockdev --getsize64 /dev/pmem$id)
+
+	if [ $pfn_size -ne $bdev_size ]; then
+		false
+	fi
+}
+
+for i in $regions
+do
+	if [ $(region_id $i) -lt $(region_id $first_region) ]; then
+		continue
+	fi
+	check_dax $i
+	check_size $i
+done
+
+# check probe
+$NDCTL disable-namespace -b e820 all
+$NDCTL enable-namespace -b e820 all
+
+rc=$SUCCESS
+cleanup $LINENO
diff --git a/test/fsdax-info0.xxd b/test/fsdax-info0.xxd
new file mode 100644
index 000000000000..a89f581ffd7e
--- /dev/null
+++ b/test/fsdax-info0.xxd
@@ -0,0 +1,11 @@
+00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+*
+00001000: 4e56 4449 4d4d 5f50 464e 5f49 4e46 4f00  NVDIMM_PFN_INFO.
+00001010: 2406 11fa 696a 40e7 b85f b9b9 e326 4a3a  $...ij@.._...&J:
+00001020: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+00001030: 0000 0000 0100 0200 0000 4000 0000 0000  ..........@.....
+00001040: 007c 0000 0000 0000 0200 0000 0000 0000  .|..............
+00001050: 0000 0004 0000 2000 0200 0000 0000 0000  ...... .........
+00001060: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+*
+00001ff0: 0000 0000 0000 0000 5cab 09bd be77 7a8d  ........\....wz.
diff --git a/test/fsdax-info1.xxd b/test/fsdax-info1.xxd
new file mode 100644
index 000000000000..36dd36d897bc
--- /dev/null
+++ b/test/fsdax-info1.xxd
@@ -0,0 +1,11 @@
+00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+*
+00001000: 4e56 4449 4d4d 5f50 464e 5f49 4e46 4f00  NVDIMM_PFN_INFO.
+00001010: af1d b9a0 b422 4188 8b3f 67bc 610c 546b  ....."A..?g.a.Tk
+00001020: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+00001030: 0000 0000 0100 0200 0000 4000 0000 0000  ..........@.....
+00001040: 007c 0000 0000 0000 0200 0000 0000 0004  .|..............
+00001050: 0000 0000 0000 2000 0200 0000 0000 0000  ...... .........
+00001060: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+*
+00001ff0: 0000 0000 0000 0000 8340 6a38 b796 d2ec  .........@j8....
diff --git a/test/fsdax-info2.xxd b/test/fsdax-info2.xxd
new file mode 100644
index 000000000000..dc19f7a89342
--- /dev/null
+++ b/test/fsdax-info2.xxd
@@ -0,0 +1,11 @@
+00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+*
+00001000: 4e56 4449 4d4d 5f50 464e 5f49 4e46 4f00  NVDIMM_PFN_INFO.
+00001010: 4c1e 026f 2939 4910 bcf7 0f8e 4dbd ab56  L..o)9I.....M..V
+00001020: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+00001030: 0000 0000 0100 0200 0000 4000 0000 0000  ..........@.....
+00001040: 007c 0000 0000 0000 0200 0000 0000 0000  .|..............
+00001050: 0000 0004 0000 2000 0200 0000 0000 0000  ...... .........
+00001060: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+*
+00001ff0: 0000 0000 0000 0000 b2c0 bb4b 60b8 2cf4  ...........K`.,.
diff --git a/test/fsdax-info3.xxd b/test/fsdax-info3.xxd
new file mode 100644
index 000000000000..468fea13b78a
--- /dev/null
+++ b/test/fsdax-info3.xxd
@@ -0,0 +1,11 @@
+00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+*
+00001000: 4e56 4449 4d4d 5f50 464e 5f49 4e46 4f00  NVDIMM_PFN_INFO.
+00001010: 80ee 77a1 321b 4938 80f2 74a5 234d 2468  ..w.2.I8..t.#M$h
+00001020: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+00001030: 0000 0000 0100 0200 0000 4000 0000 0000  ..........@.....
+00001040: 007c 0000 0000 0000 0200 0000 0000 0004  .|..............
+00001050: 0000 0000 0000 2000 0200 0000 0000 0000  ...... .........
+00001060: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+*
+00001ff0: 0000 0000 0000 0000 89fd 0ecf f199 9fac  ................

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

Re: [ndctl PATCH 2/8] ndctl/dimm: Add --human support to read-labels

[Verma, Vishal L]

On Tue, 2019-02-12 at 13:28 -0800, Dan Williams wrote:
> Allow for easier comparisons between 'read-labels' output and list.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  ndctl/dimm.c |   18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/ndctl/dimm.c b/ndctl/dimm.c
> index 292beebf9734..5ff730f42a45 100644
> --- a/ndctl/dimm.c
> +++ b/ndctl/dimm.c
> @@ -50,6 +50,7 @@ static struct parameters {
>  	bool crypto_erase;
>  	bool overwrite;
>  	bool master_pass;
> +	bool human;

Should --human also imply --json? It doesn't make sense to have human
for binary data, so it might be surprising if --human doesn't output
json.

Also I think this is missing the man page update to add --human (and
that would be a good place to clarify the human/json behavior).

Looks good otherwise.

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

Re: [ndctl PATCH 2/8] ndctl/dimm: Add --human support to read-labels

[Dan Williams]

On Tue, Mar 19, 2019 at 3:39 PM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
>
> On Tue, 2019-02-12 at 13:28 -0800, Dan Williams wrote:
> > Allow for easier comparisons between 'read-labels' output and list.
> >
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  ndctl/dimm.c |   18 +++++++++++-------
> >  1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/ndctl/dimm.c b/ndctl/dimm.c
> > index 292beebf9734..5ff730f42a45 100644
> > --- a/ndctl/dimm.c
> > +++ b/ndctl/dimm.c
> > @@ -50,6 +50,7 @@ static struct parameters {
> >       bool crypto_erase;
> >       bool overwrite;
> >       bool master_pass;
> > +     bool human;
>
> Should --human also imply --json? It doesn't make sense to have human
> for binary data, so it might be surprising if --human doesn't output
> json.
>
> Also I think this is missing the man page update to add --human (and
> that would be a good place to clarify the human/json behavior).
>

Agree with both.

> Looks good otherwise.

Will respin, thanks.
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 4/8] ndctl/namespace: Add read-info-block command

[Verma, Vishal L]

On Tue, 2019-02-12 at 13:29 -0800, Dan Williams wrote:
> Namespaces may contain an info-block that correlates with the possible
> modes of a namespace: 'sector', 'fsdax', or 'devdax'. Provide a command
> that can temporarily convert the namespace into 'raw' mode to read the
> info-block.
> 
> Also, similar to 'read-labels' provide a facility to decode the
> info-block into json.

Nice, I like this!

> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  ndctl/action.h    |    1 
>  ndctl/builtin.h   |    1 
>  ndctl/check.c     |   20 ---
>  ndctl/namespace.c |  401 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ndctl/namespace.h |   51 +++++++
>  ndctl/ndctl.c     |    1 
>  util/fletcher.h   |    1 
>  util/size.h       |    1 
>  8 files changed, 456 insertions(+), 21 deletions(-)

[..]

>  
> +#define READ_INFOBLOCK_OPTIONS() \
> +OPT_FILENAME('o', "output", &param.outfile, "output-file", \
> +	"filename to write label area contents"), \

copy/paste staleness, should be "write infoblock contents"

> +OPT_FILENAME('f', "file", &param.infile, "input-file", \
> +	"filename to readinfo block instead of a namespace"), \

Perhaps make this consistent with the label commands, and call the file
option -i/--input. This also frees up -f for --force, which might be
nice to have since the namespaces are expected to be disabled (That's
only a minor convenience thing though - I'm equally OK leaving out  the
--force for now)

> +OPT_BOOLEAN('n', "no-verify", &param.no_verify, \
> +	"skip parent uuid, and checksum validation"), \

Since the option handling framework automatically provides a no-* option
for boolean options, should we call this --verify, default to on, and
then document the availability of the automatic --no-verify option?

Currently, this ends up in an automatic --no-no-verify option :)

> +OPT_BOOLEAN('j', "json", &param.json, "parse label data into json"), \
> +OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats ")
> +

[..]

> +	if (mode == ACTION_READ_INFOBLOCK && param.infile && argc) {
> +		error("specify a namespace, or --file, not both\n");
> +		rc = -EINVAL;
> +	}
> +

Similar comment as read-labels about auto enabling --json if --human
supplied standalone.



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