[PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Joao Martins]

Hey,

This series builds on top of this one[0] and does the following improvements
to the Soft-Reserved subdivision:

 1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
 Here we add a '-a|--align 4K|2M|1G' option to the existing commands;

 2) Listing improvements for device alignment and mappings;
 Note: Perhaps it is better to hide the mappings by default, and only
       print with -v|--verbose. This would align with ndctl, as the mappings
       info can be quite large.

 3) Allow creating devices from selecting ranges. This allows to keep the
   same GPA->HPA mapping as before we kexec the hypervisor with running guests:

   daxctl list -d dax0.1 > /var/log/dax0.1.json
   kexec -d -l bzImage
   systemctl kexec
   daxctl create -u --restore /var/log/dax0.1.json

   The JSON was what I though it would be easier for an user, given that it is
   the data format daxctl outputs. Alternatives could be adding multiple:
   	--mapping <pgoff>:<start>-<end>

   But that could end up in a gigantic line and a little more
   unmanageable I think.

This series requires this series[0] on top of Dan's patches[1]:

 [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
 [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/

The only TODO here is docs and improving tests to validate mappings, and test
the restore path.

Suggestions/comments are welcome.

	Joao

Joao Martins (8):
  daxctl: add daxctl_dev_{get,set}_align()
  util/json: Print device align
  daxctl: add align support in reconfigure-device
  daxctl: add align support in create-device
  libdaxctl: add mapping iterator APIs
  daxctl: include mappings when listing
  libdaxctl: add daxctl_dev_set_mapping()
  daxctl: Allow restore devices from JSON metadata

 daxctl/device.c                | 154 +++++++++++++++++++++++++++++++++++++++--
 daxctl/lib/libdaxctl-private.h |   9 +++
 daxctl/lib/libdaxctl.c         | 152 +++++++++++++++++++++++++++++++++++++++-
 daxctl/lib/libdaxctl.sym       |   9 +++
 daxctl/libdaxctl.h             |  16 +++++
 util/json.c                    |  63 ++++++++++++++++-
 util/json.h                    |   3 +
 7 files changed, 396 insertions(+), 10 deletions(-)

-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH ndctl v1 1/8] daxctl: add daxctl_dev_{get,set}_align()

[Joao Martins]

Add support for changing devices alignment.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 daxctl/lib/libdaxctl-private.h |  1 +
 daxctl/lib/libdaxctl.c         | 34 ++++++++++++++++++++++++++++++++++
 daxctl/lib/libdaxctl.sym       |  2 ++
 daxctl/libdaxctl.h             |  2 ++
 4 files changed, 39 insertions(+)

diff --git a/daxctl/lib/libdaxctl-private.h b/daxctl/lib/libdaxctl-private.h
index 9f9c70d6024f..b307a8bc9438 100644
--- a/daxctl/lib/libdaxctl-private.h
+++ b/daxctl/lib/libdaxctl-private.h
@@ -99,6 +99,7 @@ struct daxctl_dev {
 	struct list_node list;
 	unsigned long long resource;
 	unsigned long long size;
+	unsigned long align;
 	struct kmod_module *module;
 	struct daxctl_region *region;
 	struct daxctl_memory *mem;
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 9b43b68facfe..c62e04bcdfd1 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -498,6 +498,11 @@ static void *add_dax_dev(void *parent, int id, const char *daxdev_base)
 		goto err_read;
 	dev->size = strtoull(buf, NULL, 0);
 
+	sprintf(path, "%s/align", daxdev_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		goto err_read;
+	dev->align = strtoull(buf, NULL, 0);
+
 	dev->dev_path = strdup(daxdev_base);
 	if (!dev->dev_path)
 		goto err_read;
@@ -1086,6 +1091,35 @@ DAXCTL_EXPORT int daxctl_dev_set_size(struct daxctl_dev *dev, unsigned long long
 	return 0;
 }
 
+DAXCTL_EXPORT unsigned long daxctl_dev_get_align(struct daxctl_dev *dev)
+{
+	return dev->align;
+}
+
+DAXCTL_EXPORT int daxctl_dev_set_align(struct daxctl_dev *dev, unsigned long align)
+{
+	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+	char buf[SYSFS_ATTR_SIZE];
+	char *path = dev->dev_buf;
+	int len = dev->buf_len;
+
+	if (snprintf(path, len, "%s/align", dev->dev_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				daxctl_dev_get_devname(dev));
+		return -ENXIO;
+	}
+
+	sprintf(buf, "%#lx\n", align);
+	if (sysfs_write_attr(ctx, path, buf) < 0) {
+		err(ctx, "%s: failed to set align\n",
+				daxctl_dev_get_devname(dev));
+		return -ENXIO;
+	}
+
+	dev->align = align;
+	return 0;
+}
+
 DAXCTL_EXPORT int daxctl_dev_get_target_node(struct daxctl_dev *dev)
 {
 	return dev->target_node;
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index 33c926411037..c3d08179c9fd 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -81,4 +81,6 @@ global:
 	daxctl_dev_set_size;
 	daxctl_region_create_dev;
 	daxctl_region_destroy_dev;
+	daxctl_dev_get_align;
+	daxctl_dev_set_align;
 } LIBDAXCTL_7;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index 2b14faad1895..b0bb5d78d357 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -71,6 +71,8 @@ int daxctl_dev_get_minor(struct daxctl_dev *dev);
 unsigned long long daxctl_dev_get_resource(struct daxctl_dev *dev);
 unsigned long long daxctl_dev_get_size(struct daxctl_dev *dev);
 int daxctl_dev_set_size(struct daxctl_dev *dev, unsigned long long size);
+unsigned long daxctl_dev_get_align(struct daxctl_dev *dev);
+int daxctl_dev_set_align(struct daxctl_dev *dev, unsigned long align);
 struct daxctl_ctx *daxctl_dev_get_ctx(struct daxctl_dev *dev);
 int daxctl_dev_is_enabled(struct daxctl_dev *dev);
 int daxctl_dev_disable(struct daxctl_dev *dev);
-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH ndctl v1 2/8] util/json: Print device align

[Joao Martins]

Fetch device align and include it on listings.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 util/json.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/util/json.c b/util/json.c
index 21ab25674624..4d9787381d6b 100644
--- a/util/json.c
+++ b/util/json.c
@@ -278,7 +278,7 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev,
 	struct daxctl_memory *mem = daxctl_dev_get_memory(dev);
 	const char *devname = daxctl_dev_get_devname(dev);
 	struct json_object *jdev, *jobj;
-	int node, movable;
+	int node, movable, align;
 
 	jdev = json_object_new_object();
 	if (!devname || !jdev)
@@ -299,6 +299,13 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev,
 			json_object_object_add(jdev, "target_node", jobj);
 	}
 
+	align = daxctl_dev_get_align(dev);
+	if (align > 0) {
+		jobj = util_json_object_size(daxctl_dev_get_align(dev), flags);
+		if (jobj)
+			json_object_object_add(jdev, "align", jobj);
+	}
+
 	if (mem)
 		jobj = json_object_new_string("system-ram");
 	else
-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH ndctl v1 3/8] daxctl: add align support in reconfigure-device

[Joao Martins]

Add an alignment option to reconfigure-device and use the newly added
libdaxctl API to set the alignment of the device prior to setting size.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 daxctl/device.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/daxctl/device.c b/daxctl/device.c
index 05293d6c38ee..9d82ea12aca2 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -22,6 +22,7 @@ static struct {
 	const char *mode;
 	const char *region;
 	const char *size;
+	const char *align;
 	bool no_online;
 	bool no_movable;
 	bool force;
@@ -36,6 +37,7 @@ enum dev_mode {
 };
 
 static enum dev_mode reconfig_mode = DAXCTL_DEV_MODE_UNKNOWN;
+static long long align = -1;
 static long long size = -1;
 static unsigned long flags;
 
@@ -68,7 +70,8 @@ OPT_BOOLEAN('f', "force", &param.force, \
 		"attempt to offline memory sections before reconfiguration")
 
 #define CREATE_OPTIONS() \
-OPT_STRING('s', "size", &param.size, "size", "size to switch the device to")
+OPT_STRING('s', "size", &param.size, "size", "size to switch the device to"), \
+OPT_STRING('a', "align", &param.align, "align", "alignment to switch the device to")
 
 #define DESTROY_OPTIONS() \
 OPT_BOOLEAN('f', "force", &param.force, \
@@ -185,8 +188,10 @@ static const char *parse_device_options(int argc, const char **argv,
 	/* Handle action-specific options */
 	switch (action) {
 	case ACTION_RECONFIG:
-		if (!param.size && !param.mode) {
-			fprintf(stderr, "error: a 'mode' or 'size' option is required\n");
+		if (!param.size &&
+		    !param.align &&
+		    !param.mode) {
+			fprintf(stderr, "error: a 'align', 'mode' or 'size' option is required\n");
 			usage_with_options(u, reconfig_options);
 			rc = -EINVAL;
 		}
@@ -204,6 +209,8 @@ static const char *parse_device_options(int argc, const char **argv,
 				rc =  -EINVAL;
 			}
 		}
+		if (param.align)
+			align = __parse_size64(param.align, &units);
 		break;
 	case ACTION_CREATE:
 		if (param.size)
@@ -558,6 +565,12 @@ static int do_reconfig(struct daxctl_dev *dev, enum dev_mode mode,
 	struct json_object *jdev;
 	int rc = 0;
 
+	if (align > 0) {
+		rc = daxctl_dev_set_align(dev, align);
+		if (rc < 0)
+			return rc;
+	}
+
 	if (size >= 0) {
 		rc = dev_resize(dev, size);
 		return rc;
-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH ndctl v1 4/8] daxctl: add align support in create-device

[Joao Martins]

And thus allow changing devices alignment when creating
a new child device.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 daxctl/device.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/daxctl/device.c b/daxctl/device.c
index 9d82ea12aca2..3a844462829b 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -215,6 +215,8 @@ static const char *parse_device_options(int argc, const char **argv,
 	case ACTION_CREATE:
 		if (param.size)
 			size = __parse_size64(param.size, &units);
+		if (param.align)
+			align = __parse_size64(param.align, &units);
 		/* fall through */
 	case ACTION_ONLINE:
 		if (param.no_movable)
@@ -537,6 +539,12 @@ static int do_create(struct daxctl_region *region, long long val,
 	if (val <= 0)
 		return -ENOSPC;
 
+	if (align > 0) {
+		rc = daxctl_dev_set_align(dev, align);
+		if (rc < 0)
+			return rc;
+	}
+
 	rc = daxctl_dev_set_size(dev, val);
 	if (rc < 0)
 		return rc;
-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH ndctl v1 5/8] libdaxctl: add mapping iterator APIs

[Joao Martins]

Add the following APIs to be able to iterate over a
dynamic device-dax mapping list, as well as fetching
each of the mapping attributes.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 daxctl/lib/libdaxctl-private.h |  8 ++++
 daxctl/lib/libdaxctl.c         | 91 +++++++++++++++++++++++++++++++++++++++++-
 daxctl/lib/libdaxctl.sym       |  6 +++
 daxctl/libdaxctl.h             | 12 ++++++
 4 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/daxctl/lib/libdaxctl-private.h b/daxctl/lib/libdaxctl-private.h
index b307a8bc9438..6d05aefbeda0 100644
--- a/daxctl/lib/libdaxctl-private.h
+++ b/daxctl/lib/libdaxctl-private.h
@@ -91,6 +91,12 @@ struct daxctl_region {
 	struct list_head devices;
 };
 
+struct daxctl_mapping {
+	struct daxctl_dev *dev;
+	unsigned long long pgoff, start, end;
+	struct list_node list;
+};
+
 struct daxctl_dev {
 	int id, major, minor;
 	void *dev_buf;
@@ -104,6 +110,8 @@ struct daxctl_dev {
 	struct daxctl_region *region;
 	struct daxctl_memory *mem;
 	int target_node;
+	int num_mappings;
+	struct list_head mappings;
 };
 
 struct daxctl_memory {
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index c62e04bcdfd1..506cf4b93236 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -524,7 +524,8 @@ static void *add_dax_dev(void *parent, int id, const char *daxdev_base)
 			free(path);
 			return dev_dup;
 		}
-
+	dev->num_mappings = -1;
+	list_head_init(&dev->mappings);
 	list_add(&region->devices, &dev->list);
 	free(path);
 	return dev;
@@ -1148,6 +1149,94 @@ DAXCTL_EXPORT unsigned long daxctl_memory_get_block_size(struct daxctl_memory *m
 	return mem->block_size;
 }
 
+static void mappings_init(struct daxctl_dev *dev)
+{
+	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+	char buf[SYSFS_ATTR_SIZE];
+	char *path = dev->dev_buf;
+	int i;
+
+	if (dev->num_mappings != -1)
+		return;
+
+	dev->num_mappings = 0;
+	for (;;) {
+		struct daxctl_mapping *mapping;
+		unsigned long long pgoff, start, end;
+
+		i = dev->num_mappings;
+		mapping = calloc(1, sizeof(*mapping));
+		if (!mapping) {
+			err(ctx, "%s: mapping%u allocation failure\n",
+				daxctl_dev_get_devname(dev), i);
+			continue;
+		}
+
+		sprintf(path, "%s/mapping%d/start", dev->dev_path, i);
+		if (sysfs_read_attr(ctx, path, buf) < 0) {
+			free(mapping);
+			break;
+		}
+		start = strtoull(buf, NULL, 0);
+
+		sprintf(path, "%s/mapping%d/end", dev->dev_path, i);
+		if (sysfs_read_attr(ctx, path, buf) < 0) {
+			free(mapping);
+			break;
+		}
+		end = strtoull(buf, NULL, 0);
+
+		sprintf(path, "%s/mapping%d/page_offset", dev->dev_path, i);
+		if (sysfs_read_attr(ctx, path, buf) < 0) {
+			free(mapping);
+			break;
+		}
+		pgoff = strtoull(buf, NULL, 0);
+
+		mapping->dev = dev;
+		mapping->start = start;
+		mapping->end = end;
+		mapping->pgoff = pgoff;
+
+		dev->num_mappings++;
+		list_add(&dev->mappings, &mapping->list);
+	}
+}
+
+DAXCTL_EXPORT struct daxctl_mapping *daxctl_mapping_get_first(struct daxctl_dev *dev)
+{
+	mappings_init(dev);
+
+	return list_top(&dev->mappings, struct daxctl_mapping, list);
+}
+
+DAXCTL_EXPORT struct daxctl_mapping *daxctl_mapping_get_next(struct daxctl_mapping *mapping)
+{
+	struct daxctl_dev *dev = mapping->dev;
+
+	return list_next(&dev->mappings, mapping, list);
+}
+
+DAXCTL_EXPORT unsigned long long daxctl_mapping_get_start(struct daxctl_mapping *mapping)
+{
+	return mapping->start;
+}
+
+DAXCTL_EXPORT unsigned long long daxctl_mapping_get_end(struct daxctl_mapping *mapping)
+{
+	return mapping->end;
+}
+
+DAXCTL_EXPORT unsigned long long  daxctl_mapping_get_offset(struct daxctl_mapping *mapping)
+{
+	return mapping->pgoff;
+}
+
+DAXCTL_EXPORT unsigned long long daxctl_mapping_get_size(struct daxctl_mapping *mapping)
+{
+	return mapping->end - mapping->start + 1;
+}
+
 static int memblock_is_online(struct daxctl_memory *mem, char *memblock)
 {
 	struct daxctl_dev *dev = daxctl_memory_get_dev(mem);
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index c3d08179c9fd..08362b683678 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -83,4 +83,10 @@ global:
 	daxctl_region_destroy_dev;
 	daxctl_dev_get_align;
 	daxctl_dev_set_align;
+	daxctl_mapping_get_first;
+	daxctl_mapping_get_next;
+	daxctl_mapping_get_start;
+	daxctl_mapping_get_end;
+	daxctl_mapping_get_offset;
+	daxctl_mapping_get_size;
 } LIBDAXCTL_7;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index b0bb5d78d357..f94a72fed85b 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -103,6 +103,18 @@ int daxctl_memory_online_no_movable(struct daxctl_memory *mem);
              region != NULL; \
              region = daxctl_region_get_next(region))
 
+struct daxctl_mapping;
+struct daxctl_mapping *daxctl_mapping_get_first(struct daxctl_dev *dev);
+struct daxctl_mapping *daxctl_mapping_get_next(struct daxctl_mapping *mapping);
+#define daxctl_mapping_foreach(dev, mapping) \
+        for (mapping = daxctl_mapping_get_first(dev); \
+             mapping != NULL; \
+             mapping = daxctl_mapping_get_next(mapping))
+unsigned long long daxctl_mapping_get_start(struct daxctl_mapping *mapping);
+unsigned long long daxctl_mapping_get_end(struct daxctl_mapping *mapping);
+unsigned long long  daxctl_mapping_get_offset(struct daxctl_mapping *mapping);
+unsigned long long daxctl_mapping_get_size(struct daxctl_mapping *mapping);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH ndctl v1 6/8] daxctl: include mappings when listing

[Joao Martins]

Iterate over the device mappings and print @page_offset,
 @start, @end and a computed size.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
Perhaps hiding it behind a -v|--verbose?
---
 util/json.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 util/json.h |  3 +++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/util/json.c b/util/json.c
index 4d9787381d6b..c814298ad0dc 100644
--- a/util/json.c
+++ b/util/json.c
@@ -277,7 +277,8 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev,
 {
 	struct daxctl_memory *mem = daxctl_dev_get_memory(dev);
 	const char *devname = daxctl_dev_get_devname(dev);
-	struct json_object *jdev, *jobj;
+	struct json_object *jdev, *jobj, *jmappings = NULL;
+	struct daxctl_mapping *mapping = NULL;
 	int node, movable, align;
 
 	jdev = json_object_new_object();
@@ -331,6 +332,22 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev,
 			json_object_object_add(jdev, "state", jobj);
 	}
 
+	daxctl_mapping_foreach(dev, mapping) {
+		struct json_object *jmapping;
+
+		if (!jmappings) {
+			jmappings = json_object_new_array();
+			if (!jmappings)
+				continue;
+
+			json_object_object_add(jdev, "mappings", jmappings);
+		}
+
+		jmapping = util_daxctl_mapping_to_json(mapping, flags);
+		if (!jmapping)
+			continue;
+		json_object_array_add(jmappings, jmapping);
+	}
 	return jdev;
 }
 
@@ -1180,6 +1197,41 @@ struct json_object *util_mapping_to_json(struct ndctl_mapping *mapping,
 	return NULL;
 }
 
+struct json_object *util_daxctl_mapping_to_json(struct daxctl_mapping *mapping,
+		unsigned long flags)
+{
+	struct json_object *jmapping = json_object_new_object();
+	struct json_object *jobj;
+
+	if (!jmapping)
+		return NULL;
+
+	jobj = util_json_object_hex(daxctl_mapping_get_offset(mapping), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jmapping, "page_offset", jobj);
+
+	jobj = util_json_object_hex(daxctl_mapping_get_start(mapping), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jmapping, "start", jobj);
+
+	jobj = util_json_object_hex(daxctl_mapping_get_end(mapping), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jmapping, "end", jobj);
+
+	jobj = util_json_object_size(daxctl_mapping_get_size(mapping), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jmapping, "size", jobj);
+
+	return jmapping;
+ err:
+	json_object_put(jmapping);
+	return NULL;
+}
+
 struct json_object *util_badblock_rec_to_json(u64 block, u64 count,
 		unsigned long flags)
 {
diff --git a/util/json.h b/util/json.h
index 6d39d3aa4693..c32074939068 100644
--- a/util/json.h
+++ b/util/json.h
@@ -15,6 +15,7 @@
 #include <stdio.h>
 #include <stdbool.h>
 #include <ndctl/libndctl.h>
+#include <daxctl/libdaxctl.h>
 #include <ccan/short_types/short_types.h>
 
 enum util_json_flags {
@@ -36,6 +37,8 @@ struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
 		unsigned long flags);
 struct json_object *util_mapping_to_json(struct ndctl_mapping *mapping,
 		unsigned long flags);
+struct json_object *util_daxctl_mapping_to_json(struct daxctl_mapping *mapping,
+		unsigned long flags);
 struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		unsigned long flags);
 struct json_object *util_badblock_rec_to_json(u64 block, u64 count,
-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH ndctl v1 7/8] libdaxctl: add daxctl_dev_set_mapping()

[Joao Martins]

This API adds the ability to manually pick a range within the region
device.  Such routine allows for a admin to restore the mappings of a
device after kexec. This is specially useful for hmem dynamic devdax
which do not persistent ranges allocation through say a e.g. namespace
label storage area. It also allows an userspace application to pick
it's own ranges, should it want to avoid relying on kernel's policy.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 daxctl/lib/libdaxctl.c   | 27 +++++++++++++++++++++++++++
 daxctl/lib/libdaxctl.sym |  1 +
 daxctl/libdaxctl.h       |  2 ++
 3 files changed, 30 insertions(+)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 506cf4b93236..3992ae733b16 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -1121,6 +1121,33 @@ DAXCTL_EXPORT int daxctl_dev_set_align(struct daxctl_dev *dev, unsigned long ali
 	return 0;
 }
 
+DAXCTL_EXPORT int daxctl_dev_set_mapping(struct daxctl_dev *dev,
+					unsigned long long start,
+					unsigned long long end)
+{
+	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+	unsigned long long size = end - start + 1;
+	char buf[SYSFS_ATTR_SIZE];
+	char *path = dev->dev_buf;
+	int len = dev->buf_len;
+
+	if (snprintf(path, len, "%s/mapping", dev->dev_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				daxctl_dev_get_devname(dev));
+		return -ENXIO;
+	}
+
+	sprintf(buf, "%#llx-%#llx\n", start, end);
+	if (sysfs_write_attr(ctx, path, buf) < 0) {
+		err(ctx, "%s: failed to set mapping\n",
+				daxctl_dev_get_devname(dev));
+		return -ENXIO;
+	}
+	dev->size += size;
+
+	return 0;
+}
+
 DAXCTL_EXPORT int daxctl_dev_get_target_node(struct daxctl_dev *dev)
 {
 	return dev->target_node;
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index 08362b683678..a4e16848494b 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -89,4 +89,5 @@ global:
 	daxctl_mapping_get_end;
 	daxctl_mapping_get_offset;
 	daxctl_mapping_get_size;
+	daxctl_dev_set_mapping;
 } LIBDAXCTL_7;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index f94a72fed85b..09439c16d6df 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -73,6 +73,8 @@ unsigned long long daxctl_dev_get_size(struct daxctl_dev *dev);
 int daxctl_dev_set_size(struct daxctl_dev *dev, unsigned long long size);
 unsigned long daxctl_dev_get_align(struct daxctl_dev *dev);
 int daxctl_dev_set_align(struct daxctl_dev *dev, unsigned long align);
+int daxctl_dev_set_mapping(struct daxctl_dev *dev, unsigned long long start,
+			unsigned long long end);
 struct daxctl_ctx *daxctl_dev_get_ctx(struct daxctl_dev *dev);
 int daxctl_dev_is_enabled(struct daxctl_dev *dev);
 int daxctl_dev_disable(struct daxctl_dev *dev);
-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH ndctl v1 8/8] daxctl: Allow restore devices from JSON metadata

[Joao Martins]

Add an option namely --restore which passes a parameter
which is a JSON file path. The JSON file contains the
data usually returned by:

	$ daxctl list -d dax0.1
	{
	  "chardev":"dax0.1",
	  "size":34359738368,
	  "target_node":0,
	  "align":1073741824,
	  "mode":"devdax",
	  "mappings":[
	    {
	      "page_offset":4194304,
	      "start":25769803776,
	      "end":42949672959,
	      "size":17179869184
	    },
	    {
	      "page_offset":0,
	      "start":8589934592,
	      "end":25769803775,
	      "size":17179869184
	    }
	  ]
	}

For purposes of RFC, the input values in the mapping json are decimal
for now. A device can then be created by specifying this same data
to restore it e.g.

	daxctl create-device -u --restore device.json
	{
	  "chardev":"dax0.1",
	  "size":"32.00 GiB (34.36 GB)",
	  "target_node":0,
	  "align":"1024.00 MiB (1073.74 MB)",
	  "mode":"devdax",
	  "mappings":[
	    {
	      "page_offset":"0x400000",
	      "start":"0x600000000",
	      "end":"0x9ffffffff",
	      "size":"16.00 GiB (17.18 GB)"
	    },
	    {
	      "page_offset":"0",
	      "start":"0x200000000",
	      "end":"0x5ffffffff",
	      "size":"16.00 GiB (17.18 GB)"
	    }
	  ]
	}

	created 1 device

This is handy as we are able to kexec and restore previously mappings
that we had established.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 daxctl/device.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 124 insertions(+), 5 deletions(-)

diff --git a/daxctl/device.c b/daxctl/device.c
index 3a844462829b..fe34f0e44569 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -13,6 +13,7 @@
 #include <util/json.h>
 #include <util/filter.h>
 #include <json-c/json.h>
+#include <json-c/json_util.h>
 #include <daxctl/libdaxctl.h>
 #include <util/parse-options.h>
 #include <ccan/array_size/array_size.h>
@@ -23,6 +24,7 @@ static struct {
 	const char *region;
 	const char *size;
 	const char *align;
+	const char *restore;
 	bool no_online;
 	bool no_movable;
 	bool force;
@@ -36,10 +38,16 @@ enum dev_mode {
 	DAXCTL_DEV_MODE_RAM,
 };
 
+struct mapping {
+	unsigned long long start, end, pgoff;
+};
+
 static enum dev_mode reconfig_mode = DAXCTL_DEV_MODE_UNKNOWN;
 static long long align = -1;
 static long long size = -1;
 static unsigned long flags;
+static struct mapping *maps = NULL;
+static long long nmaps = -1;
 
 enum memory_zone {
 	MEM_ZONE_MOVABLE,
@@ -71,7 +79,9 @@ OPT_BOOLEAN('f', "force", &param.force, \
 
 #define CREATE_OPTIONS() \
 OPT_STRING('s', "size", &param.size, "size", "size to switch the device to"), \
-OPT_STRING('a', "align", &param.align, "align", "alignment to switch the device to")
+OPT_STRING('a', "align", &param.align, "align", "alignment to switch the device to"), \
+OPT_STRING('\0', "restore", &param.restore, "restore", \
+		"restore the device from device JSON")
 
 #define DESTROY_OPTIONS() \
 OPT_BOOLEAN('f', "force", &param.force, \
@@ -124,6 +134,94 @@ static const struct option destroy_options[] = {
 	OPT_END(),
 };
 
+static int sort_mappings(const void *a, const void *b)
+{
+	json_object **jsoa, **jsob;
+	struct json_object *va, *vb;
+	unsigned long long pga, pgb;
+
+	jsoa = (json_object **)a;
+	jsob = (json_object **)b;
+	if (!*jsoa && !*jsob)
+		return 0;
+
+	if (!json_object_object_get_ex(*jsoa, "page_offset", &va) ||
+	    !json_object_object_get_ex(*jsob, "page_offset", &vb))
+		return 0;
+
+	pga = json_object_get_int64(va);
+	pgb = json_object_get_int64(vb);
+
+	return pga > pgb;
+}
+
+static int parse_device_file(const char *filename)
+{
+	struct json_object *jobj, *jval = NULL, *jmappings = NULL;
+	int i, len, rc = -EINVAL, region_id, id;
+	const char *chardev;
+	char  *region = NULL;
+	struct mapping *m;
+
+	jobj = json_object_from_file(filename);
+	if (!jobj)
+		return rc;
+
+	if (!json_object_object_get_ex(jobj, "align", &jval))
+		return rc;
+	param.align = json_object_get_string(jval);
+
+	if (!json_object_object_get_ex(jobj, "size", &jval))
+		return rc;
+	param.size = json_object_get_string(jval);
+
+	if (!json_object_object_get_ex(jobj, "chardev", &jval))
+		return rc;
+	chardev = json_object_get_string(jval);
+	if (sscanf(chardev, "dax%u.%u", &region_id, &id) != 2)
+		return rc;
+	if (asprintf(&region, "%u", region_id) < 0)
+		return rc;
+	param.region = region;
+
+	if (!json_object_object_get_ex(jobj, "mappings", &jmappings))
+		return rc;
+	json_object_array_sort(jmappings, sort_mappings);
+
+	len = json_object_array_length(jmappings);
+	m = calloc(len, sizeof(*m));
+	if (!m)
+		return -ENOMEM;
+
+	for (i = 0; i < len; i++) {
+		struct json_object *j, *val;
+
+		j = json_object_array_get_idx(jmappings, i);
+		if (!j)
+			goto err;
+
+		if (!json_object_object_get_ex(j, "start", &val))
+			goto err;
+		m[i].start = json_object_get_int64(val);
+
+		if (!json_object_object_get_ex(j, "end", &val))
+			goto err;
+		m[i].end = json_object_get_int64(val);
+
+		if (!json_object_object_get_ex(j, "page_offset", &val))
+			goto err;
+		m[i].pgoff = json_object_get_int64(val);
+	}
+	maps = m;
+	nmaps = len;
+	rc = 0;
+
+err:
+	if (!maps)
+		free(m);
+	return rc;
+}
+
 static const char *parse_device_options(int argc, const char **argv,
 		enum device_action action, const struct option *options,
 		const char *usage, struct daxctl_ctx *ctx)
@@ -213,6 +311,13 @@ static const char *parse_device_options(int argc, const char **argv,
 			align = __parse_size64(param.align, &units);
 		break;
 	case ACTION_CREATE:
+		if (param.restore &&
+		    (rc = parse_device_file(param.restore)) != 0) {
+			fprintf(stderr,
+				"error: failed to parse device file: %s\n",
+				strerror(-rc));
+			break;
+		}
 		if (param.size)
 			size = __parse_size64(param.size, &units);
 		if (param.align)
@@ -524,7 +629,8 @@ static int do_create(struct daxctl_region *region, long long val,
 {
 	struct json_object *jdev;
 	struct daxctl_dev *dev;
-	int rc = 0;
+	int i, rc = 0;
+	long long alloc = 0;
 
 	if (daxctl_region_create_dev(region))
 		return -ENOSPC;
@@ -545,9 +651,22 @@ static int do_create(struct daxctl_region *region, long long val,
 			return rc;
 	}
 
-	rc = daxctl_dev_set_size(dev, val);
-	if (rc < 0)
-		return rc;
+	/* @maps is ordered by page_offset */
+	for (i = 0; i < nmaps; i++) {
+		rc = daxctl_dev_set_mapping(dev, maps[i].start, maps[i].end);
+		if (rc < 0)
+			return rc;
+		alloc += (maps[i].end - maps[i].start + 1);
+	}
+
+	if (nmaps > 0 && val > 0 && alloc != val) {
+		fprintf(stderr, "%s: allocated %lld but specified size %lld\n",
+			daxctl_dev_get_devname(dev), alloc, val);
+	} else {
+		rc = daxctl_dev_set_size(dev, val);
+		if (rc < 0)
+			return rc;
+	}
 
 	rc = daxctl_dev_enable_devdax(dev);
 	if (rc) {
-- 
1.8.3.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Joao Martins]

On 7/16/20 7:46 PM, Joao Martins wrote:
> Hey,
> 
> This series builds on top of this one[0] and does the following improvements
> to the Soft-Reserved subdivision:
> 
>  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
>  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
> 
>  2) Listing improvements for device alignment and mappings;
>  Note: Perhaps it is better to hide the mappings by default, and only
>        print with -v|--verbose. This would align with ndctl, as the mappings
>        info can be quite large.
> 
>  3) Allow creating devices from selecting ranges. This allows to keep the
>    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
> 
>    daxctl list -d dax0.1 > /var/log/dax0.1.json
>    kexec -d -l bzImage
>    systemctl kexec
>    daxctl create -u --restore /var/log/dax0.1.json
> 
>    The JSON was what I though it would be easier for an user, given that it is
>    the data format daxctl outputs. Alternatives could be adding multiple:
>    	--mapping <pgoff>:<start>-<end>
> 
>    But that could end up in a gigantic line and a little more
>    unmanageable I think.
> 
> This series requires this series[0] on top of Dan's patches[1]:
> 
>  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
>  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
> 
> The only TODO here is docs and improving tests to validate mappings, and test
> the restore path.
> 
> Suggestions/comments are welcome.
> 
There's a couple of issues in this series regarding daxctl-reconfigure options and
breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.

I will fix those and respin, and probably take out the last patch as it's more RFC-ish and
in need of feedback.

	Joao

> Joao Martins (8):
>   daxctl: add daxctl_dev_{get,set}_align()
>   util/json: Print device align
>   daxctl: add align support in reconfigure-device
>   daxctl: add align support in create-device
>   libdaxctl: add mapping iterator APIs
>   daxctl: include mappings when listing
>   libdaxctl: add daxctl_dev_set_mapping()
>   daxctl: Allow restore devices from JSON metadata
> 
>  daxctl/device.c                | 154 +++++++++++++++++++++++++++++++++++++++--
>  daxctl/lib/libdaxctl-private.h |   9 +++
>  daxctl/lib/libdaxctl.c         | 152 +++++++++++++++++++++++++++++++++++++++-
>  daxctl/lib/libdaxctl.sym       |   9 +++
>  daxctl/libdaxctl.h             |  16 +++++
>  util/json.c                    |  63 ++++++++++++++++-
>  util/json.h                    |   3 +
>  7 files changed, 396 insertions(+), 10 deletions(-)
> 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Verma, Vishal L]

On Wed, 2020-12-16 at 11:39 +0000, Joao Martins wrote:
> On 7/16/20 7:46 PM, Joao Martins wrote:
> > Hey,
> > 
> > This series builds on top of this one[0] and does the following improvements
> > to the Soft-Reserved subdivision:
> > 
> >  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
> >  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
> > 
> >  2) Listing improvements for device alignment and mappings;
> >  Note: Perhaps it is better to hide the mappings by default, and only
> >        print with -v|--verbose. This would align with ndctl, as the mappings
> >        info can be quite large.
> > 
> >  3) Allow creating devices from selecting ranges. This allows to keep the
> >    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
> > 
> >    daxctl list -d dax0.1 > /var/log/dax0.1.json
> >    kexec -d -l bzImage
> >    systemctl kexec
> >    daxctl create -u --restore /var/log/dax0.1.json
> > 
> >    The JSON was what I though it would be easier for an user, given that it is
> >    the data format daxctl outputs. Alternatives could be adding multiple:
> >    	--mapping <pgoff>:<start>-<end>
> > 
> >    But that could end up in a gigantic line and a little more
> >    unmanageable I think.
> > 
> > This series requires this series[0] on top of Dan's patches[1]:
> > 
> >  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
> >  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
> > 
> > The only TODO here is docs and improving tests to validate mappings, and test
> > the restore path.
> > 
> > Suggestions/comments are welcome.
> > 
> There's a couple of issues in this series regarding daxctl-reconfigure options and
> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.
> 
> I will fix those and respin, and probably take out the last patch as it's more RFC-ish and
> in need of feedback.

Sounds good. Any objections to releasing v70 with the initial support,
and then adding this series on for the next one? I'm thinking I'll do a
much quicker v72 release say in early January with this and anything
else that missed v71.

> 
> 	Joao
> 
> > Joao Martins (8):
> >   daxctl: add daxctl_dev_{get,set}_align()
> >   util/json: Print device align
> >   daxctl: add align support in reconfigure-device
> >   daxctl: add align support in create-device
> >   libdaxctl: add mapping iterator APIs
> >   daxctl: include mappings when listing
> >   libdaxctl: add daxctl_dev_set_mapping()
> >   daxctl: Allow restore devices from JSON metadata
> > 
> >  daxctl/device.c                | 154 +++++++++++++++++++++++++++++++++++++++--
> >  daxctl/lib/libdaxctl-private.h |   9 +++
> >  daxctl/lib/libdaxctl.c         | 152 +++++++++++++++++++++++++++++++++++++++-
> >  daxctl/lib/libdaxctl.sym       |   9 +++
> >  daxctl/libdaxctl.h             |  16 +++++
> >  util/json.c                    |  63 ++++++++++++++++-
> >  util/json.h                    |   3 +
> >  7 files changed, 396 insertions(+), 10 deletions(-)
> > 

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Dan Williams]

On Wed, Dec 16, 2020 at 3:41 AM Joao Martins <joao.m.martins@oracle.com> wrote:
>
> On 7/16/20 7:46 PM, Joao Martins wrote:
> > Hey,
> >
> > This series builds on top of this one[0] and does the following improvements
> > to the Soft-Reserved subdivision:
> >
> >  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
> >  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
> >
> >  2) Listing improvements for device alignment and mappings;
> >  Note: Perhaps it is better to hide the mappings by default, and only
> >        print with -v|--verbose. This would align with ndctl, as the mappings
> >        info can be quite large.
> >
> >  3) Allow creating devices from selecting ranges. This allows to keep the
> >    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
> >
> >    daxctl list -d dax0.1 > /var/log/dax0.1.json
> >    kexec -d -l bzImage
> >    systemctl kexec
> >    daxctl create -u --restore /var/log/dax0.1.json
> >
> >    The JSON was what I though it would be easier for an user, given that it is
> >    the data format daxctl outputs. Alternatives could be adding multiple:
> >       --mapping <pgoff>:<start>-<end>
> >
> >    But that could end up in a gigantic line and a little more
> >    unmanageable I think.
> >
> > This series requires this series[0] on top of Dan's patches[1]:
> >
> >  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
> >  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
> >
> > The only TODO here is docs and improving tests to validate mappings, and test
> > the restore path.
> >
> > Suggestions/comments are welcome.
> >
> There's a couple of issues in this series regarding daxctl-reconfigure options and
> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.

What's the breakage with older kernels, is it the kernel regressing
old daxctl, or is it new daxctl being incompatible with old kernels?
If it's the latter, it needs a fixup, if it's the former it needs a
kernel compat change.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Joao Martins]

On 12/16/20 7:13 PM, Dan Williams wrote:
> On Wed, Dec 16, 2020 at 3:41 AM Joao Martins <joao.m.martins@oracle.com> wrote:
>>
>> On 7/16/20 7:46 PM, Joao Martins wrote:
>>> Hey,
>>>
>>> This series builds on top of this one[0] and does the following improvements
>>> to the Soft-Reserved subdivision:
>>>
>>>  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
>>>  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
>>>
>>>  2) Listing improvements for device alignment and mappings;
>>>  Note: Perhaps it is better to hide the mappings by default, and only
>>>        print with -v|--verbose. This would align with ndctl, as the mappings
>>>        info can be quite large.
>>>
>>>  3) Allow creating devices from selecting ranges. This allows to keep the
>>>    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
>>>
>>>    daxctl list -d dax0.1 > /var/log/dax0.1.json
>>>    kexec -d -l bzImage
>>>    systemctl kexec
>>>    daxctl create -u --restore /var/log/dax0.1.json
>>>
>>>    The JSON was what I though it would be easier for an user, given that it is
>>>    the data format daxctl outputs. Alternatives could be adding multiple:
>>>       --mapping <pgoff>:<start>-<end>
>>>
>>>    But that could end up in a gigantic line and a little more
>>>    unmanageable I think.
>>>
>>> This series requires this series[0] on top of Dan's patches[1]:
>>>
>>>  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
>>>  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
>>>
>>> The only TODO here is docs and improving tests to validate mappings, and test
>>> the restore path.
>>>
>>> Suggestions/comments are welcome.
>>>
>> There's a couple of issues in this series regarding daxctl-reconfigure options and
>> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
>> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.
> 
> What's the breakage with older kernels, is it the kernel regressing
> old daxctl, or is it new daxctl being incompatible with old kernels?
> If it's the latter, it needs a fixup, if it's the former it needs a
> kernel compat change.

It's the latter i.e. new daxctl being incompatible with old kernels, because of a change
in the first patch. Essentially a wrong assumption of device align being always available
in add_dax_dev().

The fixup would be this snip below to the first patch. But I will respin the first four
patches today or my morning tomorrow, with a test.

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 14bf48dd00bf..b01cc916eb6e 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -498,10 +498,12 @@ static void *add_dax_dev(void *parent, int id, const char *daxdev_base)
                goto err_read;
        dev->size = strtoull(buf, NULL, 0);

+       /* Device align attribute is only available in v5.10 or up */
        sprintf(path, "%s/align", daxdev_base);
-       if (sysfs_read_attr(ctx, path, buf) < 0)
-               goto err_read;
-       dev->align = strtoull(buf, NULL, 0);
+       if (!sysfs_read_attr(ctx, path, buf))
+               dev->align = strtoull(buf, NULL, 0);
+       else
+               dev->align = 0;

        dev->dev_path = strdup(daxdev_base);
        if (!dev->dev_path)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Joao Martins]

On 12/16/20 6:42 PM, Verma, Vishal L wrote:
> On Wed, 2020-12-16 at 11:39 +0000, Joao Martins wrote:
>> On 7/16/20 7:46 PM, Joao Martins wrote:
>>> Hey,
>>>
>>> This series builds on top of this one[0] and does the following improvements
>>> to the Soft-Reserved subdivision:
>>>
>>>  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
>>>  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
>>>
>>>  2) Listing improvements for device alignment and mappings;
>>>  Note: Perhaps it is better to hide the mappings by default, and only
>>>        print with -v|--verbose. This would align with ndctl, as the mappings
>>>        info can be quite large.
>>>
>>>  3) Allow creating devices from selecting ranges. This allows to keep the
>>>    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
>>>
>>>    daxctl list -d dax0.1 > /var/log/dax0.1.json
>>>    kexec -d -l bzImage
>>>    systemctl kexec
>>>    daxctl create -u --restore /var/log/dax0.1.json
>>>
>>>    The JSON was what I though it would be easier for an user, given that it is
>>>    the data format daxctl outputs. Alternatives could be adding multiple:
>>>    	--mapping <pgoff>:<start>-<end>
>>>
>>>    But that could end up in a gigantic line and a little more
>>>    unmanageable I think.
>>>
>>> This series requires this series[0] on top of Dan's patches[1]:
>>>
>>>  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
>>>  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
>>>
>>> The only TODO here is docs and improving tests to validate mappings, and test
>>> the restore path.
>>>
>>> Suggestions/comments are welcome.
>>>
>> There's a couple of issues in this series regarding daxctl-reconfigure options and
>> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
>> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.
>>
>> I will fix those and respin, and probably take out the last patch as it's more RFC-ish and
>> in need of feedback.
> 
> Sounds good. Any objections to releasing v70 with the initial support,
> and then adding this series on for the next one? I'm thinking I'll do a
> much quicker v72 release say in early January with this and anything
> else that missed v71.

If we're able to wait until tomorrow, I could respin these first four patches with the
fixes and include the align support in the initial set. Otherwise, I am also good if you
prefer defering it to v72.

	Joao
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Dan Williams]

On Wed, Dec 16, 2020 at 1:51 PM Joao Martins <joao.m.martins@oracle.com> wrote:
>
> On 12/16/20 6:42 PM, Verma, Vishal L wrote:
> > On Wed, 2020-12-16 at 11:39 +0000, Joao Martins wrote:
> >> On 7/16/20 7:46 PM, Joao Martins wrote:
> >>> Hey,
> >>>
> >>> This series builds on top of this one[0] and does the following improvements
> >>> to the Soft-Reserved subdivision:
> >>>
> >>>  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
> >>>  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
> >>>
> >>>  2) Listing improvements for device alignment and mappings;
> >>>  Note: Perhaps it is better to hide the mappings by default, and only
> >>>        print with -v|--verbose. This would align with ndctl, as the mappings
> >>>        info can be quite large.
> >>>
> >>>  3) Allow creating devices from selecting ranges. This allows to keep the
> >>>    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
> >>>
> >>>    daxctl list -d dax0.1 > /var/log/dax0.1.json
> >>>    kexec -d -l bzImage
> >>>    systemctl kexec
> >>>    daxctl create -u --restore /var/log/dax0.1.json
> >>>
> >>>    The JSON was what I though it would be easier for an user, given that it is
> >>>    the data format daxctl outputs. Alternatives could be adding multiple:
> >>>     --mapping <pgoff>:<start>-<end>
> >>>
> >>>    But that could end up in a gigantic line and a little more
> >>>    unmanageable I think.
> >>>
> >>> This series requires this series[0] on top of Dan's patches[1]:
> >>>
> >>>  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
> >>>  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
> >>>
> >>> The only TODO here is docs and improving tests to validate mappings, and test
> >>> the restore path.
> >>>
> >>> Suggestions/comments are welcome.
> >>>
> >> There's a couple of issues in this series regarding daxctl-reconfigure options and
> >> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
> >> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.
> >>
> >> I will fix those and respin, and probably take out the last patch as it's more RFC-ish and
> >> in need of feedback.
> >
> > Sounds good. Any objections to releasing v70 with the initial support,
> > and then adding this series on for the next one? I'm thinking I'll do a
> > much quicker v72 release say in early January with this and anything
> > else that missed v71.
>
> If we're able to wait until tomorrow, I could respin these first four patches with the
> fixes and include the align support in the initial set. Otherwise, I am also good if you
> prefer defering it to v72.
>

Does this change the JSON output? Might be nice to keep that all in
one update rather than invalidate some testing with the old format
betweem v71 and v72.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Joao Martins]

On 12/16/20 10:31 PM, Dan Williams wrote:
> On Wed, Dec 16, 2020 at 1:51 PM Joao Martins <joao.m.martins@oracle.com> wrote:
>>
>> On 12/16/20 6:42 PM, Verma, Vishal L wrote:
>>> On Wed, 2020-12-16 at 11:39 +0000, Joao Martins wrote:
>>>> On 7/16/20 7:46 PM, Joao Martins wrote:
>>>>> Hey,
>>>>>
>>>>> This series builds on top of this one[0] and does the following improvements
>>>>> to the Soft-Reserved subdivision:
>>>>>
>>>>>  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
>>>>>  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
>>>>>
>>>>>  2) Listing improvements for device alignment and mappings;
>>>>>  Note: Perhaps it is better to hide the mappings by default, and only
>>>>>        print with -v|--verbose. This would align with ndctl, as the mappings
>>>>>        info can be quite large.
>>>>>
>>>>>  3) Allow creating devices from selecting ranges. This allows to keep the
>>>>>    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
>>>>>
>>>>>    daxctl list -d dax0.1 > /var/log/dax0.1.json
>>>>>    kexec -d -l bzImage
>>>>>    systemctl kexec
>>>>>    daxctl create -u --restore /var/log/dax0.1.json
>>>>>
>>>>>    The JSON was what I though it would be easier for an user, given that it is
>>>>>    the data format daxctl outputs. Alternatives could be adding multiple:
>>>>>     --mapping <pgoff>:<start>-<end>
>>>>>
>>>>>    But that could end up in a gigantic line and a little more
>>>>>    unmanageable I think.
>>>>>
>>>>> This series requires this series[0] on top of Dan's patches[1]:
>>>>>
>>>>>  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
>>>>>  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
>>>>>
>>>>> The only TODO here is docs and improving tests to validate mappings, and test
>>>>> the restore path.
>>>>>
>>>>> Suggestions/comments are welcome.
>>>>>
>>>> There's a couple of issues in this series regarding daxctl-reconfigure options and
>>>> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
>>>> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.
>>>>
>>>> I will fix those and respin, and probably take out the last patch as it's more RFC-ish and
>>>> in need of feedback.
>>>
>>> Sounds good. Any objections to releasing v70 with the initial support,
>>> and then adding this series on for the next one? I'm thinking I'll do a
>>> much quicker v72 release say in early January with this and anything
>>> else that missed v71.
>>
>> If we're able to wait until tomorrow, I could respin these first four patches with the
>> fixes and include the align support in the initial set. Otherwise, I am also good if you
>> prefer defering it to v72.
>>
> 
> Does this change the JSON output? Might be nice to keep that all in
> one update rather than invalidate some testing with the old format
> betweem v71 and v72.
> 
Ugh, sent the v2 too early before seeing this.

The only change to the output is on daxctl when listing devices for 5.10+.
It starts displaying an "align" key/value.

	Joao
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Dan Williams]

On Wed, Dec 16, 2020 at 2:54 PM Joao Martins <joao.m.martins@oracle.com> wrote:
>
> On 12/16/20 10:31 PM, Dan Williams wrote:
> > On Wed, Dec 16, 2020 at 1:51 PM Joao Martins <joao.m.martins@oracle.com> wrote:
> >>
> >> On 12/16/20 6:42 PM, Verma, Vishal L wrote:
> >>> On Wed, 2020-12-16 at 11:39 +0000, Joao Martins wrote:
> >>>> On 7/16/20 7:46 PM, Joao Martins wrote:
> >>>>> Hey,
> >>>>>
> >>>>> This series builds on top of this one[0] and does the following improvements
> >>>>> to the Soft-Reserved subdivision:
> >>>>>
> >>>>>  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
> >>>>>  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
> >>>>>
> >>>>>  2) Listing improvements for device alignment and mappings;
> >>>>>  Note: Perhaps it is better to hide the mappings by default, and only
> >>>>>        print with -v|--verbose. This would align with ndctl, as the mappings
> >>>>>        info can be quite large.
> >>>>>
> >>>>>  3) Allow creating devices from selecting ranges. This allows to keep the
> >>>>>    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
> >>>>>
> >>>>>    daxctl list -d dax0.1 > /var/log/dax0.1.json
> >>>>>    kexec -d -l bzImage
> >>>>>    systemctl kexec
> >>>>>    daxctl create -u --restore /var/log/dax0.1.json
> >>>>>
> >>>>>    The JSON was what I though it would be easier for an user, given that it is
> >>>>>    the data format daxctl outputs. Alternatives could be adding multiple:
> >>>>>     --mapping <pgoff>:<start>-<end>
> >>>>>
> >>>>>    But that could end up in a gigantic line and a little more
> >>>>>    unmanageable I think.
> >>>>>
> >>>>> This series requires this series[0] on top of Dan's patches[1]:
> >>>>>
> >>>>>  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
> >>>>>  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
> >>>>>
> >>>>> The only TODO here is docs and improving tests to validate mappings, and test
> >>>>> the restore path.
> >>>>>
> >>>>> Suggestions/comments are welcome.
> >>>>>
> >>>> There's a couple of issues in this series regarding daxctl-reconfigure options and
> >>>> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
> >>>> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.
> >>>>
> >>>> I will fix those and respin, and probably take out the last patch as it's more RFC-ish and
> >>>> in need of feedback.
> >>>
> >>> Sounds good. Any objections to releasing v70 with the initial support,
> >>> and then adding this series on for the next one? I'm thinking I'll do a
> >>> much quicker v72 release say in early January with this and anything
> >>> else that missed v71.
> >>
> >> If we're able to wait until tomorrow, I could respin these first four patches with the
> >> fixes and include the align support in the initial set. Otherwise, I am also good if you
> >> prefer defering it to v72.
> >>
> >
> > Does this change the JSON output? Might be nice to keep that all in
> > one update rather than invalidate some testing with the old format
> > betweem v71 and v72.
> >
> Ugh, sent the v2 too early before seeing this.
>
> The only change to the output is on daxctl when listing devices for 5.10+.
> It starts displaying an "align" key/value.

No worries. Vishal and I chatted and it looks to me like your
improvements to the provisioning flow are worthwhile to sneak into the
v71 release if you want to include those changes as well.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Joao Martins]

On 12/16/20 11:42 PM, Dan Williams wrote:
> On Wed, Dec 16, 2020 at 2:54 PM Joao Martins <joao.m.martins@oracle.com> wrote:
>> On 12/16/20 10:31 PM, Dan Williams wrote:
>>> On Wed, Dec 16, 2020 at 1:51 PM Joao Martins <joao.m.martins@oracle.com> wrote:
>>>> On 12/16/20 6:42 PM, Verma, Vishal L wrote:
>>>>> On Wed, 2020-12-16 at 11:39 +0000, Joao Martins wrote:
>>>>>> On 7/16/20 7:46 PM, Joao Martins wrote:
>>>>>>> Hey,
>>>>>>>
>>>>>>> This series builds on top of this one[0] and does the following improvements
>>>>>>> to the Soft-Reserved subdivision:
>>>>>>>
>>>>>>>  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
>>>>>>>  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
>>>>>>>
>>>>>>>  2) Listing improvements for device alignment and mappings;
>>>>>>>  Note: Perhaps it is better to hide the mappings by default, and only
>>>>>>>        print with -v|--verbose. This would align with ndctl, as the mappings
>>>>>>>        info can be quite large.
>>>>>>>
>>>>>>>  3) Allow creating devices from selecting ranges. This allows to keep the
>>>>>>>    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
>>>>>>>
>>>>>>>    daxctl list -d dax0.1 > /var/log/dax0.1.json
>>>>>>>    kexec -d -l bzImage
>>>>>>>    systemctl kexec
>>>>>>>    daxctl create -u --restore /var/log/dax0.1.json
>>>>>>>
>>>>>>>    The JSON was what I though it would be easier for an user, given that it is
>>>>>>>    the data format daxctl outputs. Alternatives could be adding multiple:
>>>>>>>     --mapping <pgoff>:<start>-<end>
>>>>>>>
>>>>>>>    But that could end up in a gigantic line and a little more
>>>>>>>    unmanageable I think.
>>>>>>>
>>>>>>> This series requires this series[0] on top of Dan's patches[1]:
>>>>>>>
>>>>>>>  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
>>>>>>>  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
>>>>>>>
>>>>>>> The only TODO here is docs and improving tests to validate mappings, and test
>>>>>>> the restore path.
>>>>>>>
>>>>>>> Suggestions/comments are welcome.
>>>>>>>
>>>>>> There's a couple of issues in this series regarding daxctl-reconfigure options and
>>>>>> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
>>>>>> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.
>>>>>>
>>>>>> I will fix those and respin, and probably take out the last patch as it's more RFC-ish and
>>>>>> in need of feedback.
>>>>>
>>>>> Sounds good. Any objections to releasing v70 with the initial support,
>>>>> and then adding this series on for the next one? I'm thinking I'll do a
>>>>> much quicker v72 release say in early January with this and anything
>>>>> else that missed v71.
>>>>
>>>> If we're able to wait until tomorrow, I could respin these first four patches with the
>>>> fixes and include the align support in the initial set. Otherwise, I am also good if you
>>>> prefer defering it to v72.
>>>
>>> Does this change the JSON output? Might be nice to keep that all in
>>> one update rather than invalidate some testing with the old format
>>> betweem v71 and v72.
>>>
>> Ugh, sent the v2 too early before seeing this.
>>
>> The only change to the output is on daxctl when listing devices for 5.10+.
>> It starts displaying an "align" key/value.
> 
> No worries. Vishal and I chatted and it looks to me like your
> improvements to the provisioning flow are worthwhile to sneak into the
> v71 release if you want to include those changes as well.

The provisioning flow additions has some questions open about the daxctl
user interface. To summarize:

1) Should we always list mappings, or only list them with a -v option? Or
maybe instead of -v to use instead a new -M option which enables the
listing of mappings?

The reason being that it can get quite verbose with a device picks a lot
of mappings, hence I would imagine this info isn't necessary for the casual
daxctl listing.

2) Does the '--restore <file.json>' should instead be called it
instead '--device'? I feel the name '--restore' is too tied to one specific
way of using it when the feature can be used by a tool which wants to manage
its own range mappings. Additionally, I was thinking that if one wants to
manually add/fixup ranges more easily that we would add
a --mapping <pgoff>:<start>-<end> sort of syntax. But I suppose this could
be added later if its really desired.

And with these clarified, I could respin it over. Oh and I'm missing a
mappings test as well.

It's worth mentioning that kexec will need fixing, as dax_hmem regions
created with HMAT or manually assigned with efi_fake_mem= get lost on
kexec because we do not pass the EFI_MEMMAP conventional memory ranges
to the second kernel (only runtime code/boot services). I have a
RFC patch for x86 efi handling, but should get that conversation started
after holidays.

	Joao
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Verma, Vishal L]

On Thu, 2020-12-17 at 11:23 +0000, Joao Martins wrote:
> 
> The provisioning flow additions has some questions open about the daxctl
> user interface. To summarize:
> 
> 1) Should we always list mappings, or only list them with a -v option? Or
> maybe instead of -v to use instead a new -M option which enables the
> listing of mappings?
> 
> The reason being that it can get quite verbose with a device picks a lot
> of mappings, hence I would imagine this info isn't necessary for the casual
> daxctl listing.

I think hiding them behind a new option is probably best. And then we
can have different verbosity levels turn on or off. The verbosity levels
stuff is implemented in ndctl, but I don't think it is in daxctl yet, so
we can just do the specific option to display mappings for now, and then
revisit verbosity levels in the future if we feel like the number of
options is getting out of hand.

> 
> 2) Does the '--restore <file.json>' should instead be called it
> instead '--device'? I feel the name '--restore' is too tied to one specific
> way of using it when the feature can be used by a tool which wants to manage

Hm, I looked at other commands that take an input file - write labels
just calls it --input, so there might be value in staying consistent
with that. But write-infoblock just uses stdin - so that could be
another option. I'd be fine with either of those.

> its own range mappings. Additionally, I was thinking that if one wants to
> manually add/fixup ranges more easily that we would add
> a --mapping <pgoff>:<start>-<end> sort of syntax. But I suppose this could
> be added later if its really desired.

Agreed with adding this later if needed.

> 
> And with these clarified, I could respin it over. Oh and I'm missing a
> mappings test as well.

Sounds good I'll wait to get these in.

> 
> It's worth mentioning that kexec will need fixing, as dax_hmem regions
> created with HMAT or manually assigned with efi_fake_mem= get lost on
> kexec because we do not pass the EFI_MEMMAP conventional memory ranges
> to the second kernel (only runtime code/boot services). I have a
> RFC patch for x86 efi handling, but should get that conversation started
> after holidays.
> 
> 	Joao

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org