[ndctl PATCH 0/5] ndctl: Optimize label operations

[ndctl PATCH 0/5] ndctl: Optimize label operations

[Dan Williams]

In almost all use cases ndctl was reading more label data than necessary
to carry out tasks like "init-labels" and the auto-label capability of
"create-namespace".

Outside of index-block scoped operations above, there is utility in
being able to specify an extent range smaller than the full capacity to
"{read,write,zero}-labels".

Deploy optimizations to only operate on the index blocks when possible,
and allow for custom extents for the generic operations.

---

Dan Williams (5):
      ndctl/dimm: Support small label reads/writes
      ndctl/dimm: Minimize data-transfer for init-labels
      ndctl/dimm: Add offset and size options to {read,write,zero}-labels
      ndctl/dimm: Limit read-labels with --index option
      ndctl/namespace: Minimize label data transfer for autolabel


 Documentation/ndctl/labels-options.txt |    9 +++
 ndctl/dimm.c                           |   88 ++++++++++++++++++--------
 ndctl/lib/dimm.c                       |   85 +++++++++++++++++++++++--
 ndctl/lib/libndctl.c                   |  107 ++++++++++++++++++++++++++++----
 ndctl/lib/libndctl.sym                 |    5 +
 ndctl/lib/private.h                    |    4 -
 ndctl/libndctl.h                       |    9 +++
 ndctl/namespace.c                      |    2 -
 util/util.h                            |    4 +
 9 files changed, 260 insertions(+), 53 deletions(-)
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 1/5] ndctl/dimm: Support small label reads/writes

[Dan Williams]

The initial ndctl label read/write implementation assumed that label
read / writes were relatively inexpensive, but that assumption is
invalid in practice. The process of reading a full label area can take
10s of seconds. Implement ndctl_cmd_cfg_{read,write}_set_extent() to
trim the label read/write commands to a range smaller than the full
label area.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/libndctl.c   |  107 +++++++++++++++++++++++++++++++++++++++++-------
 ndctl/lib/libndctl.sym |    2 +
 ndctl/lib/private.h    |    1 
 ndctl/libndctl.h       |    4 ++
 4 files changed, 99 insertions(+), 15 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index fd36aa0662f4..5a5279f0fdb4 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2572,6 +2572,7 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_read(struct ndctl_cmd *cfg
 	cmd->get_data->in_offset = 0;
 	cmd->get_data->in_length = cfg_size->get_size->max_xfer;
 	cmd->firmware_status = &cmd->get_data->status;
+	cmd->iter.init_offset = 0;
 	cmd->iter.offset = &cmd->get_data->in_offset;
 	cmd->iter.xfer = &cmd->get_data->in_length;
 	cmd->iter.max_xfer = cfg_size->get_size->max_xfer;
@@ -2583,10 +2584,43 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_read(struct ndctl_cmd *cfg
 		free(cmd);
 		return NULL;
 	}
+	cmd->source = cfg_size;
+	ndctl_cmd_ref(cfg_size);
 
 	return cmd;
 }
 
+static void iter_set_extent(struct ndctl_cmd_iter *iter, unsigned int len,
+		unsigned int offset)
+{
+	iter->init_offset = offset;
+	*iter->offset = offset;
+	*iter->xfer = min(iter->max_xfer, len);
+	iter->total_xfer = len;
+}
+
+NDCTL_EXPORT int ndctl_cmd_cfg_read_set_extent(struct ndctl_cmd *cfg_read,
+		unsigned int len, unsigned int offset)
+{
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(cmd_to_bus(cfg_read));
+	struct ndctl_cmd *cfg_size = cfg_read->source;
+
+	if (cfg_read->type != ND_CMD_GET_CONFIG_DATA
+			|| cfg_read->status <= 0) {
+		dbg(ctx, "expected unsubmitted cfg_read command\n");
+		return -EINVAL;
+	}
+
+	if (offset + len > cfg_size->get_size->config_size) {
+		dbg(ctx, "read %d from %d exceeds %d\n", len, offset,
+				cfg_size->get_size->config_size);
+		return -EINVAL;
+	}
+
+	iter_set_extent(&cfg_read->iter, len, offset);
+	return 0;
+}
+
 NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_write(struct ndctl_cmd *cfg_read)
 {
 	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(cmd_to_bus(cfg_read));
@@ -2622,10 +2656,11 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_write(struct ndctl_cmd *cf
 	cmd->type = ND_CMD_SET_CONFIG_DATA;
 	cmd->size = size;
 	cmd->status = 1;
-	cmd->set_data->in_offset = 0;
+	cmd->set_data->in_offset = cfg_read->iter.init_offset;
 	cmd->set_data->in_length = cfg_read->iter.max_xfer;
 	cmd->firmware_status = (u32 *) (cmd->cmd_buf
 		+ sizeof(struct nd_cmd_set_config_hdr) + cfg_read->iter.max_xfer);
+	cmd->iter.init_offset = cfg_read->iter.init_offset;
 	cmd->iter.offset = &cmd->set_data->in_offset;
 	cmd->iter.xfer = &cmd->set_data->in_length;
 	cmd->iter.max_xfer = cfg_read->iter.max_xfer;
@@ -2647,18 +2682,33 @@ NDCTL_EXPORT unsigned int ndctl_cmd_cfg_size_get_size(struct ndctl_cmd *cfg_size
 	return 0;
 }
 
+static ssize_t iter_access(struct ndctl_cmd_iter *iter, unsigned int len,
+		unsigned int offset)
+{
+	if (offset < iter->init_offset
+			|| offset > iter->init_offset + iter->total_xfer
+			|| len + offset < len)
+		return -EINVAL;
+	if (len + offset > iter->init_offset + iter->total_xfer)
+		len = iter->total_xfer - offset;
+	return len;
+}
+
 NDCTL_EXPORT ssize_t ndctl_cmd_cfg_read_get_data(struct ndctl_cmd *cfg_read,
-		void *buf, unsigned int len, unsigned int offset)
+		void *buf, unsigned int _len, unsigned int offset)
 {
+	struct ndctl_cmd_iter *iter;
+	ssize_t len;
+
 	if (cfg_read->type != ND_CMD_GET_CONFIG_DATA || cfg_read->status > 0)
 		return -EINVAL;
 	if (cfg_read->status < 0)
 		return cfg_read->status;
-	if (offset > cfg_read->iter.total_xfer || len + offset < len)
-		return -EINVAL;
-	if (len + offset > cfg_read->iter.total_xfer)
-		len = cfg_read->iter.total_xfer - offset;
-	memcpy(buf, cfg_read->iter.total_buf + offset, len);
+
+	iter = &cfg_read->iter;
+	len = iter_access(&cfg_read->iter, _len, offset);
+	if (len >= 0)
+		memcpy(buf, iter->total_buf + offset, len);
 	return len;
 }
 
@@ -2671,29 +2721,56 @@ NDCTL_EXPORT ssize_t ndctl_cmd_cfg_read_get_size(struct ndctl_cmd *cfg_read)
 	return cfg_read->iter.total_xfer;
 }
 
+NDCTL_EXPORT int ndctl_cmd_cfg_write_set_extent(struct ndctl_cmd *cfg_write,
+		unsigned int len, unsigned int offset)
+{
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(cmd_to_bus(cfg_write));
+	struct ndctl_cmd *cfg_size, *cfg_read;
+
+	if (cfg_write->type != ND_CMD_SET_CONFIG_DATA
+			|| cfg_write->status <= 0) {
+		dbg(ctx, "expected unsubmitted cfg_write command\n");
+		return -EINVAL;
+	}
+
+	cfg_read = cfg_write->source;
+	cfg_size = cfg_read->source;
+
+	if (offset + len > cfg_size->get_size->config_size) {
+		dbg(ctx, "write %d from %d exceeds %d\n", len, offset,
+				cfg_size->get_size->config_size);
+		return -EINVAL;
+	}
+
+	iter_set_extent(&cfg_write->iter, len, offset);
+	return 0;
+}
+
 NDCTL_EXPORT ssize_t ndctl_cmd_cfg_write_set_data(struct ndctl_cmd *cfg_write,
-		void *buf, unsigned int len, unsigned int offset)
+		void *buf, unsigned int _len, unsigned int offset)
 {
+	ssize_t len;
+
 	if (cfg_write->type != ND_CMD_SET_CONFIG_DATA || cfg_write->status < 1)
 		return -EINVAL;
 	if (cfg_write->status < 0)
 		return cfg_write->status;
-	if (offset > cfg_write->iter.total_xfer || len + offset < len)
-		return -EINVAL;
-	if (len + offset > cfg_write->iter.total_xfer)
-		len = cfg_write->iter.total_xfer - offset;
-	memcpy(cfg_write->iter.total_buf + offset, buf, len);
+	len = iter_access(&cfg_write->iter, _len, offset);
+	if (len >= 0)
+		memcpy(cfg_write->iter.total_buf + offset, buf, len);
 	return len;
 }
 
 NDCTL_EXPORT ssize_t ndctl_cmd_cfg_write_zero_data(struct ndctl_cmd *cfg_write)
 {
+	struct ndctl_cmd_iter *iter = &cfg_write->iter;
+
 	if (cfg_write->type != ND_CMD_SET_CONFIG_DATA || cfg_write->status < 1)
 		return -EINVAL;
 	if (cfg_write->status < 0)
 		return cfg_write->status;
-	memset(cfg_write->iter.total_buf, 0, cfg_write->iter.total_xfer);
-	return cfg_write->iter.total_xfer;
+	memset(iter->total_buf + iter->init_offset, 0, iter->total_xfer);
+	return iter->total_xfer;
 }
 
 NDCTL_EXPORT void ndctl_cmd_unref(struct ndctl_cmd *cmd)
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 297f03d7ae39..e79b31c71ae6 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -409,4 +409,6 @@ global:
 LIBNDCTL_20 {
 global:
 	ndctl_bus_poll_scrub_completion;
+	ndctl_cmd_cfg_read_set_extent;
+	ndctl_cmd_cfg_write_set_extent;
 } LIBNDCTL_19;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index a387b0b50442..5a17f0b63b80 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -253,6 +253,7 @@ struct ndctl_cmd {
 	int status;
 	u32 *firmware_status;
 	struct ndctl_cmd_iter {
+		u32 init_offset;
 		u32 *offset;
 		u32 *xfer; /* pointer to xfer length in cmd */
 		u8 *data; /* pointer to the data buffer location in cmd */
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index e378802ee4c1..310814fe924c 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -321,6 +321,10 @@ unsigned int ndctl_cmd_cfg_size_get_size(struct ndctl_cmd *cfg_size);
 ssize_t ndctl_cmd_cfg_read_get_data(struct ndctl_cmd *cfg_read, void *buf,
 		unsigned int len, unsigned int offset);
 ssize_t ndctl_cmd_cfg_read_get_size(struct ndctl_cmd *cfg_read);
+int ndctl_cmd_cfg_read_set_extent(struct ndctl_cmd *cfg_read,
+		unsigned int len, unsigned int offset);
+int ndctl_cmd_cfg_write_set_extent(struct ndctl_cmd *cfg_write,
+		unsigned int len, unsigned int offset);
 ssize_t ndctl_cmd_cfg_write_set_data(struct ndctl_cmd *cfg_write, void *buf,
 		unsigned int len, unsigned int offset);
 ssize_t ndctl_cmd_cfg_write_zero_data(struct ndctl_cmd *cfg_write);

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

[ndctl PATCH 2/5] ndctl/dimm: Minimize data-transfer for init-labels

[Dan Williams]

Currently init-labels implementation reads the entire namespace-label
capacity, initializes just the namespace index, and then writes the
entire label capacity. It turns out that DIMM label-area access methods
can be exceedingly slow.

For example, the time to read the entire label area on a single dimm:
2s, but the time to just read the index block space: 45ms.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/dimm.c           |    2 +-
 ndctl/lib/dimm.c       |   53 +++++++++++++++++++++++++++++++++++++++++++++---
 ndctl/lib/libndctl.sym |    1 +
 ndctl/libndctl.h       |    1 +
 4 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 5ff730f42a45..5edada678c9c 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -974,7 +974,7 @@ static int __action_init(struct ndctl_dimm *dimm,
 	struct ndctl_cmd *cmd_read;
 	int rc;
 
-	cmd_read = ndctl_dimm_read_labels(dimm);
+	cmd_read = ndctl_dimm_read_label_index(dimm);
 	if (!cmd_read)
 		return -ENXIO;
 
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 22cf4e10b56c..9c5a34e542c3 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -370,14 +370,15 @@ static struct namespace_label *label_base(struct nvdimm_data *ndd)
 	return (struct namespace_label *) base;
 }
 
-static void init_ndd(struct nvdimm_data *ndd, struct ndctl_cmd *cmd_read)
+static void init_ndd(struct nvdimm_data *ndd, struct ndctl_cmd *cmd_read,
+		struct ndctl_cmd *cmd_size)
 {
 	ndctl_cmd_unref(ndd->cmd_read);
 	memset(ndd, 0, sizeof(*ndd));
 	ndd->cmd_read = cmd_read;
 	ndctl_cmd_ref(cmd_read);
 	ndd->data = cmd_read->iter.total_buf;
-	ndd->config_size = cmd_read->iter.total_xfer;
+	ndd->config_size = cmd_size->get_size->config_size;
 	ndd->ns_current = -1;
 	ndd->ns_next = -1;
 }
@@ -490,6 +491,52 @@ NDCTL_EXPORT int ndctl_dimm_validate_labels(struct ndctl_dimm *dimm)
 	return label_validate(&dimm->ndd);
 }
 
+NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_label_index(struct ndctl_dimm *dimm)
+{
+        struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
+        struct ndctl_cmd *cmd_size, *cmd_read;
+	struct nvdimm_data *ndd = &dimm->ndd;
+        int rc;
+
+        rc = ndctl_bus_wait_probe(bus);
+        if (rc < 0)
+                return NULL;
+
+        cmd_size = ndctl_dimm_cmd_new_cfg_size(dimm);
+        if (!cmd_size)
+                return NULL;
+        rc = ndctl_cmd_submit_xlat(cmd_size);
+        if (rc < 0)
+                goto out_size;
+
+        cmd_read = ndctl_dimm_cmd_new_cfg_read(cmd_size);
+        if (!cmd_read)
+                goto out_size;
+	/*
+	 * To calc the namespace index size use the minimum label
+	 * size which corresponds to the maximum namespace index size.
+	 */
+	init_ndd(ndd, cmd_read, cmd_size);
+	ndd->nslabel_size = 128;
+	rc = ndctl_cmd_cfg_read_set_extent(cmd_read,
+			sizeof_namespace_index(ndd) * 2, 0);
+	if (rc < 0)
+		goto out_size;
+
+        rc = ndctl_cmd_submit_xlat(cmd_read);
+        if (rc < 0)
+                goto out_read;
+	ndctl_cmd_unref(cmd_size);
+
+	return cmd_read;
+
+ out_read:
+        ndctl_cmd_unref(cmd_read);
+ out_size:
+        ndctl_cmd_unref(cmd_size);
+        return NULL;
+}
+
 NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm)
 {
         struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
@@ -515,7 +562,7 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm)
                 goto out_read;
 	ndctl_cmd_unref(cmd_size);
 
-	init_ndd(&dimm->ndd, cmd_read);
+	init_ndd(&dimm->ndd, cmd_read, cmd_size);
 
 	return cmd_read;
 
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index e79b31c71ae6..3cd431a90e55 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -411,4 +411,5 @@ global:
 	ndctl_bus_poll_scrub_completion;
 	ndctl_cmd_cfg_read_set_extent;
 	ndctl_cmd_cfg_write_set_extent;
+	ndctl_dimm_read_label_index;
 } LIBNDCTL_19;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 310814fe924c..8aa4b8bbe6c2 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -308,6 +308,7 @@ struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_read(struct ndctl_cmd *cfg_size);
 struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_write(struct ndctl_cmd *cfg_read);
 int ndctl_dimm_zero_labels(struct ndctl_dimm *dimm);
 struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm);
+struct ndctl_cmd *ndctl_dimm_read_label_index(struct ndctl_dimm *dimm);
 int ndctl_dimm_validate_labels(struct ndctl_dimm *dimm);
 enum ndctl_namespace_version {
 	NDCTL_NS_VERSION_1_1,

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

[ndctl PATCH 3/5] ndctl/dimm: Add offset and size options to {read, write, zero}-labels

[Dan Williams]

Allow for more precision in label utilities, i.e. stop operating over
the entire label area.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/labels-options.txt |    9 ++++++
 ndctl/dimm.c                           |   45 ++++++++++++++++++++++++--------
 ndctl/lib/dimm.c                       |   36 +++++++++++++++++++++-----
 ndctl/lib/libndctl.sym                 |    2 +
 ndctl/lib/private.h                    |    3 --
 ndctl/libndctl.h                       |    4 +++
 util/util.h                            |    4 +++
 7 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/Documentation/ndctl/labels-options.txt b/Documentation/ndctl/labels-options.txt
index 539ace079557..4aee37969fd5 100644
--- a/Documentation/ndctl/labels-options.txt
+++ b/Documentation/ndctl/labels-options.txt
@@ -5,6 +5,15 @@
 	operate on every dimm in the system, optionally filtered by bus id (see
         --bus= option).
 
+-s::
+--size=::
+	Limit the operation to the given number of bytes. A size of 0
+	indicates to operate over the entire label capacity.
+
+-O::
+--offset=::
+	Begin the operation at the given offset into the label area.
+
 -b::
 --bus=::
 	Limit operation to memory devices (dimms) that are on the given bus.
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 5edada678c9c..478e0fea3afc 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -47,6 +47,8 @@ static struct parameters {
 	const char *infile;
 	const char *labelversion;
 	const char *kek;
+	unsigned len;
+	unsigned offset;
 	bool crypto_erase;
 	bool overwrite;
 	bool master_pass;
@@ -76,7 +78,7 @@ static int action_enable(struct ndctl_dimm *dimm, struct action_context *actx)
 
 static int action_zero(struct ndctl_dimm *dimm, struct action_context *actx)
 {
-	return ndctl_dimm_zero_labels(dimm);
+	return ndctl_dimm_zero_label_extent(dimm, param.len, param.offset);
 }
 
 static struct json_object *dump_label_json(struct ndctl_dimm *dimm,
@@ -298,15 +300,17 @@ static struct json_object *dump_json(struct ndctl_dimm *dimm,
 	return NULL;
 }
 
-static int rw_bin(FILE *f, struct ndctl_cmd *cmd, ssize_t size, int rw)
+static int rw_bin(FILE *f, struct ndctl_cmd *cmd, ssize_t size,
+		unsigned int start_offset, int rw)
 {
 	char buf[4096];
 	ssize_t offset, write = 0;
 
-	for (offset = 0; offset < size; offset += sizeof(buf)) {
+	for (offset = start_offset; offset < start_offset + size;
+			offset += sizeof(buf)) {
 		ssize_t len = min_t(ssize_t, sizeof(buf), size - offset), rc;
 
-		if (rw) {
+		if (rw == WRITE) {
 			len = fread(buf, 1, len, f);
 			if (len == 0)
 				break;
@@ -342,9 +346,9 @@ static int action_write(struct ndctl_dimm *dimm, struct action_context *actx)
 		return -EBUSY;
 	}
 
-	cmd_read = ndctl_dimm_read_labels(dimm);
+	cmd_read = ndctl_dimm_read_label_extent(dimm, param.len, param.offset);
 	if (!cmd_read)
-		return -ENXIO;
+		return -EINVAL;
 
 	cmd_write = ndctl_dimm_cmd_new_cfg_write(cmd_read);
 	if (!cmd_write) {
@@ -353,7 +357,7 @@ static int action_write(struct ndctl_dimm *dimm, struct action_context *actx)
 	}
 
 	size = ndctl_cmd_cfg_read_get_size(cmd_read);
-	rc = rw_bin(actx->f_in, cmd_write, size, 1);
+	rc = rw_bin(actx->f_in, cmd_write, size, param.offset, WRITE);
 
 	/*
 	 * If the dimm is already disabled the kernel is not holding a cached
@@ -380,9 +384,9 @@ static int action_read(struct ndctl_dimm *dimm, struct action_context *actx)
 	ssize_t size;
 	int rc = 0;
 
-	cmd_read = ndctl_dimm_read_labels(dimm);
+	cmd_read = ndctl_dimm_read_label_extent(dimm, param.len, param.offset);
 	if (!cmd_read)
-		return -ENXIO;
+		return -EINVAL;
 
 	size = ndctl_cmd_cfg_read_get_size(cmd_read);
 	if (actx->jdimms) {
@@ -393,7 +397,7 @@ static int action_read(struct ndctl_dimm *dimm, struct action_context *actx)
 		else
 			rc = -ENOMEM;
 	} else
-		rc = rw_bin(actx->f_out, cmd_read, size, 0);
+		rc = rw_bin(actx->f_out, cmd_read, size, param.offset, READ);
 
 	ndctl_cmd_unref(cmd_read);
 
@@ -1072,18 +1076,31 @@ OPT_BOOLEAN('o', "overwrite", &param.overwrite, \
 OPT_BOOLEAN('m', "master-passphrase", &param.master_pass, \
 		"use master passphrase")
 
+#define LABEL_OPTIONS() \
+OPT_UINTEGER('s', "size", &param.len, "number of label bytes to operate"), \
+OPT_UINTEGER('O', "offset", &param.offset, \
+	"offset into the label area to start operation")
+
 static const struct option read_options[] = {
 	BASE_OPTIONS(),
+	LABEL_OPTIONS(),
 	READ_OPTIONS(),
 	OPT_END(),
 };
 
 static const struct option write_options[] = {
 	BASE_OPTIONS(),
+	LABEL_OPTIONS(),
 	WRITE_OPTIONS(),
 	OPT_END(),
 };
 
+static const struct option zero_options[] = {
+	BASE_OPTIONS(),
+	LABEL_OPTIONS(),
+	OPT_END(),
+};
+
 static const struct option update_options[] = {
 	BASE_OPTIONS(),
 	UPDATE_OPTIONS(),
@@ -1150,6 +1167,12 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx,
 		return -EINVAL;
 	}
 
+	if (action == action_read && param.json && (param.len || param.offset)) {
+		fprintf(stderr, "--size and --offset are incompatible with --json\n");
+		usage_with_options(u, options);
+		return -EINVAL;
+	}
+
 	if (param.json) {
 		actx.jdimms = json_object_new_array();
 		if (!actx.jdimms)
@@ -1284,7 +1307,7 @@ int cmd_read_labels(int argc, const char **argv, struct ndctl_ctx *ctx)
 
 int cmd_zero_labels(int argc, const char **argv, struct ndctl_ctx *ctx)
 {
-	int count = dimm_action(argc, argv, ctx, action_zero, base_options,
+	int count = dimm_action(argc, argv, ctx, action_zero, zero_options,
 			"ndctl zero-labels <nmem0> [<nmem1>..<nmemN>] [<options>]");
 
 	fprintf(stderr, "zeroed %d nmem%s\n", count >= 0 ? count : 0,
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 9c5a34e542c3..28b1dfb0bdfa 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -537,7 +537,8 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_label_index(struct ndctl_dimm *di
         return NULL;
 }
 
-NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm)
+NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_label_extent(
+		struct ndctl_dimm *dimm, unsigned int len, unsigned int offset)
 {
         struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
         struct ndctl_cmd *cmd_size, *cmd_read;
@@ -557,13 +558,25 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm)
         cmd_read = ndctl_dimm_cmd_new_cfg_read(cmd_size);
         if (!cmd_read)
                 goto out_size;
+
+	/*
+	 * For ndctl_read_labels() compat, enable subsequent calls that
+	 * will manipulate labels
+	 */
+	if (len == 0 && offset == 0)
+		init_ndd(&dimm->ndd, cmd_read, cmd_size);
+
+	if (len == 0)
+		len = cmd_size->get_size->config_size;
+	rc = ndctl_cmd_cfg_read_set_extent(cmd_read, len, offset);
+	if (rc < 0)
+		goto out_size;
+
         rc = ndctl_cmd_submit_xlat(cmd_read);
         if (rc < 0)
                 goto out_read;
 	ndctl_cmd_unref(cmd_size);
 
-	init_ndd(&dimm->ndd, cmd_read, cmd_size);
-
 	return cmd_read;
 
  out_read:
@@ -573,15 +586,21 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm)
         return NULL;
 }
 
-NDCTL_EXPORT int ndctl_dimm_zero_labels(struct ndctl_dimm *dimm)
+NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm)
+{
+	return ndctl_dimm_read_label_extent(dimm, 0, 0);
+}
+
+NDCTL_EXPORT int ndctl_dimm_zero_label_extent(struct ndctl_dimm *dimm,
+		unsigned int len, unsigned int offset)
 {
 	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
 	struct ndctl_cmd *cmd_read, *cmd_write;
 	int rc;
 
-	cmd_read = ndctl_dimm_read_labels(dimm);
+	cmd_read = ndctl_dimm_read_label_extent(dimm, len, offset);
 	if (!cmd_read)
-		return -ENXIO;
+		return -EINVAL;
 
 	if (ndctl_dimm_is_active(dimm)) {
 		dbg(ctx, "%s: regions active, abort label write\n",
@@ -623,6 +642,11 @@ NDCTL_EXPORT int ndctl_dimm_zero_labels(struct ndctl_dimm *dimm)
 	return rc;
 }
 
+NDCTL_EXPORT int ndctl_dimm_zero_labels(struct ndctl_dimm *dimm)
+{
+	return ndctl_dimm_zero_label_extent(dimm, 0, 0);
+}
+
 NDCTL_EXPORT unsigned long ndctl_dimm_get_available_labels(
 		struct ndctl_dimm *dimm)
 {
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 3cd431a90e55..895fb358c43a 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -412,4 +412,6 @@ global:
 	ndctl_cmd_cfg_read_set_extent;
 	ndctl_cmd_cfg_write_set_extent;
 	ndctl_dimm_read_label_index;
+	ndctl_dimm_read_label_extent;
+	ndctl_dimm_zero_label_extent;
 } LIBNDCTL_19;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 5a17f0b63b80..1304017d5e3b 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -241,9 +241,6 @@ struct ndctl_namespace {
  *
  * A command may only specify one of @source, or @iter.total_buf, not both.
  */
-enum {
-	READ, WRITE,
-};
 struct ndctl_cmd {
 	struct ndctl_dimm *dimm;
 	struct ndctl_bus *bus;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 8aa4b8bbe6c2..c9d0dc120d3b 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -307,8 +307,12 @@ struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_size(struct ndctl_dimm *dimm);
 struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_read(struct ndctl_cmd *cfg_size);
 struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_write(struct ndctl_cmd *cfg_read);
 int ndctl_dimm_zero_labels(struct ndctl_dimm *dimm);
+int ndctl_dimm_zero_label_extent(struct ndctl_dimm *dimm,
+		unsigned int len, unsigned int offset);
 struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm);
 struct ndctl_cmd *ndctl_dimm_read_label_index(struct ndctl_dimm *dimm);
+struct ndctl_cmd *ndctl_dimm_read_label_extent(struct ndctl_dimm *dimm,
+		unsigned int len, unsigned int offset);
 int ndctl_dimm_validate_labels(struct ndctl_dimm *dimm);
 enum ndctl_namespace_version {
 	NDCTL_NS_VERSION_1_1,
diff --git a/util/util.h b/util/util.h
index 001707e8b159..54c6ef18b6d7 100644
--- a/util/util.h
+++ b/util/util.h
@@ -73,6 +73,10 @@
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
 #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
 
+enum {
+	READ, WRITE,
+};
+
 static inline const char *skip_prefix(const char *str, const char *prefix)
 {
         size_t len = strlen(prefix);

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

[ndctl PATCH 4/5] ndctl/dimm: Limit read-labels with --index option

[Dan Williams]

Provide a capability to limit the read-labels payload to just the
index-block data space.

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

diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 478e0fea3afc..a4b9999d9c67 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -54,6 +54,7 @@ static struct parameters {
 	bool master_pass;
 	bool human;
 	bool force;
+	bool index;
 	bool json;
 	bool verbose;
 } param = {
@@ -275,27 +276,26 @@ static struct json_object *dump_json(struct ndctl_dimm *dimm,
 
 	if (!jdimm)
 		return NULL;
-	jindex = dump_index_json(cmd_read, size);
-	if (!jindex)
-		goto err_jindex;
-	jlabel = dump_label_json(dimm, cmd_read, size, flags);
-	if (!jlabel)
-		goto err_jlabel;
 
 	jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
 	if (!jobj)
-		goto err_jobj;
-
+		goto err;
 	json_object_object_add(jdimm, "dev", jobj);
+
+	jindex = dump_index_json(cmd_read, size);
+	if (!jindex)
+		goto err;
 	json_object_object_add(jdimm, "index", jindex);
+	if (param.index)
+		return jdimm;
+
+	jlabel = dump_label_json(dimm, cmd_read, size, flags);
+	if (!jlabel)
+		goto err;
 	json_object_object_add(jdimm, "label", jlabel);
-	return jdimm;
 
- err_jobj:
-	json_object_put(jlabel);
- err_jlabel:
-	json_object_put(jindex);
- err_jindex:
+	return jdimm;
+err:
 	json_object_put(jdimm);
 	return NULL;
 }
@@ -384,7 +384,11 @@ static int action_read(struct ndctl_dimm *dimm, struct action_context *actx)
 	ssize_t size;
 	int rc = 0;
 
-	cmd_read = ndctl_dimm_read_label_extent(dimm, param.len, param.offset);
+	if (param.index)
+		cmd_read = ndctl_dimm_read_label_index(dimm);
+	else
+		cmd_read = ndctl_dimm_read_label_extent(dimm, param.len,
+				param.offset);
 	if (!cmd_read)
 		return -EINVAL;
 
@@ -1046,7 +1050,8 @@ OPT_BOOLEAN('v',"verbose", &param.verbose, "turn on debug")
 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('u', "human", &param.human, "use human friendly number formats ")
+OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats "), \
+OPT_BOOLEAN('I', "index", &param.index, "limit read to the index block area")
 
 #define WRITE_OPTIONS() \
 OPT_STRING('i', "input", &param.infile, "input-file", \
@@ -1173,6 +1178,12 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx,
 		return -EINVAL;
 	}
 
+	if (param.index && param.len) {
+		fprintf(stderr, "pick either --size, or --index, not both\n");
+		usage_with_options(u, options);
+		return -EINVAL;
+	}
+
 	if (param.json) {
 		actx.jdimms = json_object_new_array();
 		if (!actx.jdimms)

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

[ndctl PATCH 5/5] ndctl/namespace: Minimize label data transfer for autolabel

[Dan Williams]

Use the new ndctl_dimm_read_label_index() helper to minimize the amount
of label I/O needed to execute an autolabel event.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 63b870ff7a6d..05e5ab792941 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -1016,7 +1016,7 @@ retry:
 		int num_labels, avail;
 
 		ndctl_cmd_unref(cmd_read);
-		cmd_read = ndctl_dimm_read_labels(dimm);
+		cmd_read = ndctl_dimm_read_label_index(dimm);
 		if (!cmd_read)
 			continue;
 

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

Re: [ndctl PATCH 3/5] ndctl/dimm: Add offset and size options to {read, write, zero}-labels

[Verma, Vishal L]

On Tue, 2019-03-05 at 17:34 -0800, Dan Williams wrote:
> Allow for more precision in label utilities, i.e. stop operating over
> the entire label area.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  Documentation/ndctl/labels-options.txt |    9 ++++++
>  ndctl/dimm.c                           |   45 ++++++++++++++++++++++++--------
>  ndctl/lib/dimm.c                       |   36 +++++++++++++++++++++-----
>  ndctl/lib/libndctl.sym                 |    2 +
>  ndctl/lib/private.h                    |    3 --
>  ndctl/libndctl.h                       |    4 +++
>  util/util.h                            |    4 +++
>  7 files changed, 83 insertions(+), 20 deletions(-)
> 

[..]

> diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
> index 5a17f0b63b80..1304017d5e3b 100644
> --- a/ndctl/lib/private.h
> +++ b/ndctl/lib/private.h
> @@ -241,9 +241,6 @@ struct ndctl_namespace {
>   *
>   * A command may only specify one of @source, or @iter.total_buf, not both.
>   */
> -enum {
> -	READ, WRITE,
> -};

This fails to build as libndctl.c now needs to include util/util.h..
Stale version perhaps? (Patch 4 in the series also fails to apply
cleanly)

>  struct ndctl_cmd {
>  	struct ndctl_dimm *dimm;
>  	struct ndctl_bus *bus;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index 8aa4b8bbe6c2..c9d0dc120d3b 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -307,8 +307,12 @@ struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_size(struct ndctl_dimm *dimm);
>  struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_read(struct ndctl_cmd *cfg_size);
>  struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_write(struct ndctl_cmd *cfg_read);
>  int ndctl_dimm_zero_labels(struct ndctl_dimm *dimm);
> +int ndctl_dimm_zero_label_extent(struct ndctl_dimm *dimm,
> +		unsigned int len, unsigned int offset);
>  struct ndctl_cmd *ndctl_dimm_read_labels(struct ndctl_dimm *dimm);
>  struct ndctl_cmd *ndctl_dimm_read_label_index(struct ndctl_dimm *dimm);
> +struct ndctl_cmd *ndctl_dimm_read_label_extent(struct ndctl_dimm *dimm,
> +		unsigned int len, unsigned int offset);
>  int ndctl_dimm_validate_labels(struct ndctl_dimm *dimm);
>  enum ndctl_namespace_version {
>  	NDCTL_NS_VERSION_1_1,
> diff --git a/util/util.h b/util/util.h
> index 001707e8b159..54c6ef18b6d7 100644
> --- a/util/util.h
> +++ b/util/util.h
> @@ -73,6 +73,10 @@
>  #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>  #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>  
> +enum {
> +	READ, WRITE,
> +};
> +
>  static inline const char *skip_prefix(const char *str, const char *prefix)
>  {
>          size_t len = strlen(prefix);
> 

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

Re: [ndctl PATCH 3/5] ndctl/dimm: Add offset and size options to {read, write, zero}-labels

[Dan Williams]

On Fri, Mar 22, 2019 at 2:49 PM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
>
> On Tue, 2019-03-05 at 17:34 -0800, Dan Williams wrote:
> > Allow for more precision in label utilities, i.e. stop operating over
> > the entire label area.
> >
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  Documentation/ndctl/labels-options.txt |    9 ++++++
> >  ndctl/dimm.c                           |   45 ++++++++++++++++++++++++--------
> >  ndctl/lib/dimm.c                       |   36 +++++++++++++++++++++-----
> >  ndctl/lib/libndctl.sym                 |    2 +
> >  ndctl/lib/private.h                    |    3 --
> >  ndctl/libndctl.h                       |    4 +++
> >  util/util.h                            |    4 +++
> >  7 files changed, 83 insertions(+), 20 deletions(-)
> >
>
> [..]
>
> > diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
> > index 5a17f0b63b80..1304017d5e3b 100644
> > --- a/ndctl/lib/private.h
> > +++ b/ndctl/lib/private.h
> > @@ -241,9 +241,6 @@ struct ndctl_namespace {
> >   *
> >   * A command may only specify one of @source, or @iter.total_buf, not both.
> >   */
> > -enum {
> > -     READ, WRITE,
> > -};
>
> This fails to build as libndctl.c now needs to include util/util.h..
> Stale version perhaps? (Patch 4 in the series also fails to apply
> cleanly)

Hmm, ok I'll resend. It probably depends on something in the
read-infoblock patch that needs to be fixed up.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm