[PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions

linux-nvdimm.lists.01.org archive mirror
 help / color / mirror / Atom feed

* [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions
@ 2020-07-13 16:08 Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 01/10] daxctl: Cleanup whitespace Joao Martins
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Changes since v1:

* Add a Documentation/daxctl/ entry for each patch that adds commands or new
option.

* Fix functional test suite to only change region 0 and not touch others

* Fix reconfigure-device -s changes (third patch) for better bisection.

v1: https://lore.kernel.org/linux-nvdimm/20200403205900.18035-1-joao.m.martins@oracle.com/

---

This series introduces the daxctl support for sub-dividing soft-reserved 
regions created by EFI/HMAT/efi_fake_mem. It's the userspace counterpart
of this recent patch series [0].

These new 'dynamic' regions can be partitioned into multiple different devices
which its subdivisions can consist of one or more ranges. This
is in contrast to static dax regions -- created with ndctl-create-namespace
 -m devdax -- which can't be subdivided neither discontiguous. 
 See also cover-letter of [0].

The daxctl changes in these patches are depicted as:

 * {create,destroy,disable,enable}-device:
   
   These orchestrate/manage the sub-division devices.
   It mimmics the same as namespaces equivalent commands.

 * Allow reconfigure-device to change the size of an existing *dynamic* dax
 device.

 * Add test coverage (Tried to cover all range allocation code paths).
 v2 of kernel patches now passes this test suite.

 * Documentation regarding the new command additions.

[0] "device-dax: Support sub-dividing soft-reserved ranges",
https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/

Dan Williams (1):
  daxctl: Cleanup whitespace

Joao Martins (9):
  libdaxctl: add daxctl_dev_set_size()
  daxctl: add resize support in reconfigure-device
  daxctl: add command to disable devdax device
  daxctl: add command to enable devdax device
  libdaxctl: add daxctl_region_create_dev()
  daxctl: add command to create device
  libdaxctl: add daxctl_region_destroy_dev()
  daxctl: add command to destroy device
  daxctl/test: Add tests for dynamic dax regions

 Documentation/daxctl/Makefile.am                   |   6 +-
 Documentation/daxctl/daxctl-create-device.txt      | 105 +++++++
 Documentation/daxctl/daxctl-destroy-device.txt     |  63 +++++
 Documentation/daxctl/daxctl-disable-device.txt     |  58 ++++
 Documentation/daxctl/daxctl-enable-device.txt      |  59 ++++
 Documentation/daxctl/daxctl-reconfigure-device.txt |  16 ++
 daxctl/builtin.h                                   |   4 +
 daxctl/daxctl.c                                    |   4 +
 daxctl/device.c                                    | 310 ++++++++++++++++++++-
 daxctl/lib/libdaxctl.c                             |  67 +++++
 daxctl/lib/libdaxctl.sym                           |   7 +
 daxctl/libdaxctl.h                                 |   3 +
 test/Makefile.am                                   |   1 +
 test/daxctl-create.sh                              | 294 +++++++++++++++++++
 util/filter.c                                      |   2 +-
 15 files changed, 993 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/daxctl/daxctl-create-device.txt
 create mode 100644 Documentation/daxctl/daxctl-destroy-device.txt
 create mode 100644 Documentation/daxctl/daxctl-disable-device.txt
 create mode 100644 Documentation/daxctl/daxctl-enable-device.txt
 create mode 100755 test/daxctl-create.sh

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

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

* [PATCH ndctl v2 01/10] daxctl: Cleanup whitespace
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 02/10] libdaxctl: add daxctl_dev_set_size() Joao Martins
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

From: Dan Williams <dan.j.williams@intel.com>

Re-indent util_daxctl_region_filter() to match expectations.

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

diff --git a/util/filter.c b/util/filter.c
index af72793929e2..7d0159f8cef6 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -344,7 +344,7 @@ struct daxctl_region *util_daxctl_region_filter(struct daxctl_region *region,
 		return region;
 
 	if ((sscanf(ident, "%d", &region_id) == 1
-       || sscanf(ident, "region%d", &region_id) == 1)
+				|| sscanf(ident, "region%d", &region_id) == 1)
 			&& daxctl_region_get_id(region) == region_id)
 		return region;
 
-- 
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

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

* [PATCH ndctl v2 02/10] libdaxctl: add daxctl_dev_set_size()
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 01/10] daxctl: Cleanup whitespace Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 03/10] daxctl: add resize support in reconfigure-device Joao Martins
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add an API for setting the size of a dax device.

This sysfs attribute is only writeable for a dynamic
dax device such as the one exported by hmem.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 daxctl/lib/libdaxctl.c   | 24 ++++++++++++++++++++++++
 daxctl/lib/libdaxctl.sym |  5 +++++
 daxctl/libdaxctl.h       |  1 +
 3 files changed, 30 insertions(+)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index ee4a069eb463..00d5f7fe61ad 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -1019,6 +1019,30 @@ DAXCTL_EXPORT unsigned long long daxctl_dev_get_size(struct daxctl_dev *dev)
 	return dev->size;
 }
 
+DAXCTL_EXPORT int daxctl_dev_set_size(struct daxctl_dev *dev, unsigned long long size)
+{
+	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/size", dev->dev_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				daxctl_dev_get_devname(dev));
+		return -ENXIO;
+	}
+
+	sprintf(buf, "%#llx\n", size);
+	if (sysfs_write_attr(ctx, path, buf) < 0) {
+		err(ctx, "%s: failed to set size\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 87d02366a917..d8b4137c76b7 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -75,3 +75,8 @@ global:
 	daxctl_memory_is_movable;
 	daxctl_memory_online_no_movable;
 } LIBDAXCTL_6;
+
+LIBDAXCTL_8 {
+global:
+	daxctl_dev_set_size;
+} LIBDAXCTL_7;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index 6c545e1f3055..e3d482743cc6 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -68,6 +68,7 @@ int daxctl_dev_get_major(struct daxctl_dev *dev);
 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);
 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

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

* [PATCH ndctl v2 03/10] daxctl: add resize support in reconfigure-device
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 01/10] daxctl: Cleanup whitespace Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 02/10] libdaxctl: add daxctl_dev_set_size() Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 04/10] daxctl: add command to disable devdax device Joao Martins
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add the ability to change the size of an inactive dax
device. Uses of this includes adjusting existing devices
or enterily freeing the region device to make the space
available.

	$ daxctl disable-device dax0.0
	disabled 1 device
	$ daxctl reconfigure-device -s 0 dax0.0
	reconfigured 1 device
	$ daxctl reconfigure-device -s 4G dax0.0
	reconfigured 1 device

@size (-s) and @mode (-m) are mutually exclusive as the latter relates
to assigning memory to System-RAM through kmem as opposed
to reconfiguring dynamic dax devices.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/daxctl/daxctl-reconfigure-device.txt | 16 +++++++++++
 daxctl/device.c                                    | 32 ++++++++++++++++++++--
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/daxctl/daxctl-reconfigure-device.txt b/Documentation/daxctl/daxctl-reconfigure-device.txt
index cb28fed24e52..8caae436faae 100644
--- a/Documentation/daxctl/daxctl-reconfigure-device.txt
+++ b/Documentation/daxctl/daxctl-reconfigure-device.txt
@@ -75,6 +75,14 @@ daxctl_dev_get_target_node() or 'daxctl list')
 # numactl --cpunodebind=0-1 --membind=2 -- some-service --opt1 --opt2
 ----
 
+* Change the size of a dax device
+----
+# daxctl reconfigure-device dax0.1 -s 16G
+reconfigured 1 device
+# daxctl reconfigure-device dax0.1 -s 0
+reconfigured 1 device
+----
+
 DESCRIPTION
 -----------
 
@@ -120,6 +128,14 @@ OPTIONS
 	more /dev/daxX.Y devices, where X is the region id and Y is the device
 	instance id.
 
+-s::
+--size=::
+	For regions that support dax device creation, change the device size
+	in bytes. This option supports the suffixes "k" or "K" for KiB, "m" or
+	"M" for MiB, "g" or "G" for GiB and "t" or "T" for TiB.
+
+	The size must be a multiple of the region alignment.
+
 -m::
 --mode=::
 	Specify the mode to which the dax device(s) should be reconfigured.
diff --git a/daxctl/device.c b/daxctl/device.c
index 705f1f8ff7f6..033e098eafe0 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -9,6 +9,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/sysmacros.h>
+#include <util/size.h>
 #include <util/json.h>
 #include <util/filter.h>
 #include <json-c/json.h>
@@ -20,6 +21,7 @@ static struct {
 	const char *dev;
 	const char *mode;
 	const char *region;
+	const char *size;
 	bool no_online;
 	bool no_movable;
 	bool force;
@@ -34,6 +36,7 @@ enum dev_mode {
 };
 
 static enum dev_mode reconfig_mode = DAXCTL_DEV_MODE_UNKNOWN;
+static long long size = -1;
 static unsigned long flags;
 
 enum memory_zone {
@@ -60,12 +63,16 @@ OPT_BOOLEAN('N', "no-online", &param.no_online, \
 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")
+
 #define ZONE_OPTIONS() \
 OPT_BOOLEAN('\0', "no-movable", &param.no_movable, \
 		"online memory in ZONE_NORMAL")
 
 static const struct option reconfig_options[] = {
 	BASE_OPTIONS(),
+	CREATE_OPTIONS(),
 	RECONFIG_OPTIONS(),
 	ZONE_OPTIONS(),
 	OPT_END(),
@@ -90,6 +97,7 @@ static const char *parse_device_options(int argc, const char **argv,
 		usage,
 		NULL
 	};
+	unsigned long long units = 1;
 	int i, rc = 0;
 
 	argc = parse_options(argc, argv, options, u, 0);
@@ -135,12 +143,14 @@ static const char *parse_device_options(int argc, const char **argv,
 	/* Handle action-specific options */
 	switch (action) {
 	case ACTION_RECONFIG:
-		if (!param.mode) {
-			fprintf(stderr, "error: a 'mode' option is required\n");
+		if (!param.size && !param.mode) {
+			fprintf(stderr, "error: a 'mode' or 'size' option is required\n");
 			usage_with_options(u, reconfig_options);
 			rc = -EINVAL;
 		}
-		if (strcmp(param.mode, "system-ram") == 0) {
+		if (param.size) {
+			size = __parse_size64(param.size, &units);
+		} else if (strcmp(param.mode, "system-ram") == 0) {
 			reconfig_mode = DAXCTL_DEV_MODE_RAM;
 			if (param.no_movable)
 				mem_zone = MEM_ZONE_NORMAL;
@@ -309,6 +319,17 @@ static int dev_offline_memory(struct daxctl_dev *dev)
 	return 0;
 }
 
+static int dev_resize(struct daxctl_dev *dev, unsigned long long val)
+{
+	int rc;
+
+	rc = daxctl_dev_set_size(dev, val);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
 static int disable_devdax_device(struct daxctl_dev *dev)
 {
 	struct daxctl_memory *mem = daxctl_dev_get_memory(dev);
@@ -418,6 +439,11 @@ static int do_reconfig(struct daxctl_dev *dev, enum dev_mode mode,
 	struct json_object *jdev;
 	int rc = 0;
 
+	if (size >= 0) {
+		rc = dev_resize(dev, size);
+		return rc;
+	}
+
 	switch (mode) {
 	case DAXCTL_DEV_MODE_RAM:
 		rc = reconfig_mode_system_ram(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

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

* [PATCH ndctl v2 04/10] daxctl: add command to disable devdax device
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
                   ` (2 preceding siblings ...)
  2020-07-13 16:08 ` [PATCH ndctl v2 03/10] daxctl: add resize support in reconfigure-device Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 05/10] daxctl: add command to enable " Joao Martins
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add a 'disable-device' command, required prior to
reconfiguration or destruction of the dax device.

Mimmics the same functionality as seen in
ndctl-disable-namespace.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/daxctl/Makefile.am               |  3 +-
 Documentation/daxctl/daxctl-disable-device.txt | 58 ++++++++++++++++++++++++
 daxctl/builtin.h                               |  1 +
 daxctl/daxctl.c                                |  1 +
 daxctl/device.c                                | 61 ++++++++++++++++++++++++++
 5 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/daxctl/daxctl-disable-device.txt

diff --git a/Documentation/daxctl/Makefile.am b/Documentation/daxctl/Makefile.am
index 7696e23cc9c0..1f070771cd95 100644
--- a/Documentation/daxctl/Makefile.am
+++ b/Documentation/daxctl/Makefile.am
@@ -31,7 +31,8 @@ man1_MANS = \
 	daxctl-migrate-device-model.1 \
 	daxctl-reconfigure-device.1 \
 	daxctl-online-memory.1 \
-	daxctl-offline-memory.1
+	daxctl-offline-memory.1 \
+	daxctl-disable-device.1
 
 EXTRA_DIST = $(man1_MANS)
 
diff --git a/Documentation/daxctl/daxctl-disable-device.txt b/Documentation/daxctl/daxctl-disable-device.txt
new file mode 100644
index 000000000000..383aeeb58150
--- /dev/null
+++ b/Documentation/daxctl/daxctl-disable-device.txt
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+
+daxctl-disable-device(1)
+========================
+
+NAME
+----
+daxctl-disable-device - Disables a devdax device
+
+SYNOPSIS
+--------
+[verse]
+'daxctl disable-device' [<options>]
+
+EXAMPLES
+--------
+
+* Disables dax0.1
+----
+# daxctl disable-device dax0.1
+----
+
+* Disables all devices in region id 0
+----
+# daxctl disable-device -r 0 all
+disabled 3 devices
+----
+
+DESCRIPTION
+-----------
+
+Disables a dax device in 'devdax' mode.
+
+OPTIONS
+-------
+-r::
+--region=::
+	Restrict the operation to devices belonging to the specified region(s).
+	A device-dax region is a contiguous range of memory that hosts one or
+	more /dev/daxX.Y devices, where X is the region id and Y is the device
+	instance id.
+
+-u::
+--human::
+	By default the command will output machine-friendly raw-integer
+	data. Instead, with this flag, numbers representing storage size
+	will be formatted as human readable strings with units, other
+	fields are converted to hexadecimal strings.
+
+-v::
+--verbose::
+	Emit more debug messages
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkdaxctl:daxctl-list[1],daxctl-reconfigure-device[1],daxctl-create-device[1]
diff --git a/daxctl/builtin.h b/daxctl/builtin.h
index f5a0147f0e11..c9848953bbd8 100644
--- a/daxctl/builtin.h
+++ b/daxctl/builtin.h
@@ -7,6 +7,7 @@ struct daxctl_ctx;
 int cmd_list(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx);
+int cmd_disable_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_online_memory(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_offline_memory(int argc, const char **argv, struct daxctl_ctx *ctx);
 #endif /* _DAXCTL_BUILTIN_H_ */
diff --git a/daxctl/daxctl.c b/daxctl/daxctl.c
index 1ab073200313..1707a9ff0791 100644
--- a/daxctl/daxctl.c
+++ b/daxctl/daxctl.c
@@ -74,6 +74,7 @@ static struct cmd_struct commands[] = {
 	{ "reconfigure-device", .d_fn = cmd_reconfig_device },
 	{ "online-memory", .d_fn = cmd_online_memory },
 	{ "offline-memory", .d_fn = cmd_offline_memory },
+	{ "disable-device", .d_fn = cmd_disable_device },
 };
 
 int main(int argc, const char **argv)
diff --git a/daxctl/device.c b/daxctl/device.c
index 033e098eafe0..20df2b844774 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -49,6 +49,7 @@ enum device_action {
 	ACTION_RECONFIG,
 	ACTION_ONLINE,
 	ACTION_OFFLINE,
+	ACTION_DISABLE,
 };
 
 #define BASE_OPTIONS() \
@@ -89,6 +90,11 @@ static const struct option offline_options[] = {
 	OPT_END(),
 };
 
+static const struct option disable_options[] = {
+	BASE_OPTIONS(),
+	OPT_END(),
+};
+
 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)
@@ -116,6 +122,9 @@ static const char *parse_device_options(int argc, const char **argv,
 		case ACTION_OFFLINE:
 			action_string = "offline memory for";
 			break;
+		case ACTION_DISABLE:
+			action_string = "disable";
+			break;
 		default:
 			action_string = "<>";
 			break;
@@ -168,6 +177,7 @@ static const char *parse_device_options(int argc, const char **argv,
 			mem_zone = MEM_ZONE_NORMAL;
 		/* fall through */
 	case ACTION_OFFLINE:
+	case ACTION_DISABLE:
 		/* nothing special */
 		break;
 	}
@@ -497,6 +507,35 @@ static int do_xline(struct daxctl_dev *dev, enum device_action action)
 	return rc;
 }
 
+static int do_xble(struct daxctl_dev *dev, enum device_action action)
+{
+	struct daxctl_memory *mem = daxctl_dev_get_memory(dev);
+	const char *devname = daxctl_dev_get_devname(dev);
+	int rc;
+
+	if (mem) {
+		fprintf(stderr,
+			"%s: status operations are only applicable in devdax mode\n",
+			devname);
+		return -ENXIO;
+	}
+
+	switch (action) {
+	case ACTION_DISABLE:
+		rc = daxctl_dev_disable(dev);
+		if (rc) {
+			fprintf(stderr, "%s: disable failed: %s\n",
+				daxctl_dev_get_devname(dev), strerror(-rc));
+			return rc;
+		}
+		break;
+	default:
+		fprintf(stderr, "%s: invalid action: %d\n", devname, action);
+		rc = -EINVAL;
+	}
+	return rc;
+}
+
 static int do_xaction_device(const char *device, enum device_action action,
 		struct daxctl_ctx *ctx, int *processed)
 {
@@ -531,6 +570,11 @@ static int do_xaction_device(const char *device, enum device_action action,
 				if (rc == 0)
 					(*processed)++;
 				break;
+			case ACTION_DISABLE:
+				rc = do_xble(dev, action);
+				if (rc == 0)
+					(*processed)++;
+				break;
 			default:
 				rc = -EINVAL;
 				break;
@@ -566,6 +610,23 @@ int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx)
 	return rc;
 }
 
+int cmd_disable_device(int argc, const char **argv, struct daxctl_ctx *ctx)
+{
+	char *usage = "daxctl disable-device <device>";
+	const char *device = parse_device_options(argc, argv, ACTION_DISABLE,
+			disable_options, usage, ctx);
+	int processed, rc;
+
+	rc = do_xaction_device(device, ACTION_DISABLE, ctx, &processed);
+	if (rc < 0)
+		fprintf(stderr, "error disabling device: %s\n",
+				strerror(-rc));
+
+	fprintf(stderr, "disabled %d device%s\n", processed,
+			processed == 1 ? "" : "s");
+	return rc;
+}
+
 int cmd_online_memory(int argc, const char **argv, struct daxctl_ctx *ctx)
 {
 	char *usage = "daxctl online-memory <device> [<options>]";
-- 
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

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

* [PATCH ndctl v2 05/10] daxctl: add command to enable devdax device
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
                   ` (3 preceding siblings ...)
  2020-07-13 16:08 ` [PATCH ndctl v2 04/10] daxctl: add command to disable devdax device Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 06/10] libdaxctl: add daxctl_region_create_dev() Joao Martins
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add a 'enable-device' command, required prior to
reconfiguration of the dax device.

Mimics the same functionality as seen in
ndctl-enable-namespace.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/daxctl/Makefile.am              |  3 +-
 Documentation/daxctl/daxctl-enable-device.txt | 59 +++++++++++++++++++++++++++
 daxctl/builtin.h                              |  1 +
 daxctl/daxctl.c                               |  1 +
 daxctl/device.c                               | 40 ++++++++++++++++++
 5 files changed, 103 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/daxctl/daxctl-enable-device.txt

diff --git a/Documentation/daxctl/Makefile.am b/Documentation/daxctl/Makefile.am
index 1f070771cd95..e43d34183142 100644
--- a/Documentation/daxctl/Makefile.am
+++ b/Documentation/daxctl/Makefile.am
@@ -32,7 +32,8 @@ man1_MANS = \
 	daxctl-reconfigure-device.1 \
 	daxctl-online-memory.1 \
 	daxctl-offline-memory.1 \
-	daxctl-disable-device.1
+	daxctl-disable-device.1 \
+	daxctl-enable-device.1
 
 EXTRA_DIST = $(man1_MANS)
 
diff --git a/Documentation/daxctl/daxctl-enable-device.txt b/Documentation/daxctl/daxctl-enable-device.txt
new file mode 100644
index 000000000000..6410d92cafe9
--- /dev/null
+++ b/Documentation/daxctl/daxctl-enable-device.txt
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0
+
+daxctl-enable-device(1)
+=======================
+
+NAME
+----
+daxctl-enable-device - Enable a devdax device
+
+SYNOPSIS
+--------
+[verse]
+'daxctl enable-device' <dax0.0> [<dax1.0>...<daxY.Z>] [<options>]
+
+EXAMPLES
+--------
+
+* Enables dax0.1
+----
+# daxctl enable-device dax0.1
+enabled 1 device
+----
+
+* Enables all devices in region id 0
+----
+# daxctl enable-device -r 0 all
+enabled 3 devices
+----
+
+DESCRIPTION
+-----------
+
+Enables a dax device in 'devdax' mode.
+
+OPTIONS
+-------
+-r::
+--region=::
+	Restrict the operation to devices belonging to the specified region(s).
+	A device-dax region is a contiguous range of memory that hosts one or
+	more /dev/daxX.Y devices, where X is the region id and Y is the device
+	instance id.
+
+-u::
+--human::
+	By default the command will output machine-friendly raw-integer
+	data. Instead, with this flag, numbers representing storage size
+	will be formatted as human readable strings with units, other
+	fields are converted to hexadecimal strings.
+
+-v::
+--verbose::
+	Emit more debug messages
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkdaxctl:daxctl-list[1],daxctl-reconfigure-device[1],daxctl-create-device[1]
diff --git a/daxctl/builtin.h b/daxctl/builtin.h
index c9848953bbd8..8f344f86ad20 100644
--- a/daxctl/builtin.h
+++ b/daxctl/builtin.h
@@ -8,6 +8,7 @@ int cmd_list(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_disable_device(int argc, const char **argv, struct daxctl_ctx *ctx);
+int cmd_enable_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_online_memory(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_offline_memory(int argc, const char **argv, struct daxctl_ctx *ctx);
 #endif /* _DAXCTL_BUILTIN_H_ */
diff --git a/daxctl/daxctl.c b/daxctl/daxctl.c
index 1707a9ff0791..a4699b3780bd 100644
--- a/daxctl/daxctl.c
+++ b/daxctl/daxctl.c
@@ -75,6 +75,7 @@ static struct cmd_struct commands[] = {
 	{ "online-memory", .d_fn = cmd_online_memory },
 	{ "offline-memory", .d_fn = cmd_offline_memory },
 	{ "disable-device", .d_fn = cmd_disable_device },
+	{ "enable-device", .d_fn = cmd_enable_device },
 };
 
 int main(int argc, const char **argv)
diff --git a/daxctl/device.c b/daxctl/device.c
index 20df2b844774..05a9247ecfde 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -50,6 +50,7 @@ enum device_action {
 	ACTION_ONLINE,
 	ACTION_OFFLINE,
 	ACTION_DISABLE,
+	ACTION_ENABLE,
 };
 
 #define BASE_OPTIONS() \
@@ -95,6 +96,11 @@ static const struct option disable_options[] = {
 	OPT_END(),
 };
 
+static const struct option enable_options[] = {
+	BASE_OPTIONS(),
+	OPT_END(),
+};
+
 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)
@@ -125,6 +131,9 @@ static const char *parse_device_options(int argc, const char **argv,
 		case ACTION_DISABLE:
 			action_string = "disable";
 			break;
+		case ACTION_ENABLE:
+			action_string = "enable";
+			break;
 		default:
 			action_string = "<>";
 			break;
@@ -178,6 +187,7 @@ static const char *parse_device_options(int argc, const char **argv,
 		/* fall through */
 	case ACTION_OFFLINE:
 	case ACTION_DISABLE:
+	case ACTION_ENABLE:
 		/* nothing special */
 		break;
 	}
@@ -521,6 +531,14 @@ static int do_xble(struct daxctl_dev *dev, enum device_action action)
 	}
 
 	switch (action) {
+	case ACTION_ENABLE:
+		rc = daxctl_dev_enable_devdax(dev);
+		if (rc) {
+			fprintf(stderr, "%s: enable failed: %s\n",
+				daxctl_dev_get_devname(dev), strerror(-rc));
+			return rc;
+		}
+		break;
 	case ACTION_DISABLE:
 		rc = daxctl_dev_disable(dev);
 		if (rc) {
@@ -570,6 +588,11 @@ static int do_xaction_device(const char *device, enum device_action action,
 				if (rc == 0)
 					(*processed)++;
 				break;
+			case ACTION_ENABLE:
+				rc = do_xble(dev, action);
+				if (rc == 0)
+					(*processed)++;
+				break;
 			case ACTION_DISABLE:
 				rc = do_xble(dev, action);
 				if (rc == 0)
@@ -627,6 +650,23 @@ int cmd_disable_device(int argc, const char **argv, struct daxctl_ctx *ctx)
 	return rc;
 }
 
+int cmd_enable_device(int argc, const char **argv, struct daxctl_ctx *ctx)
+{
+	char *usage = "daxctl enable-device <device>";
+	const char *device = parse_device_options(argc, argv, ACTION_DISABLE,
+			enable_options, usage, ctx);
+	int processed, rc;
+
+	rc = do_xaction_device(device, ACTION_ENABLE, ctx, &processed);
+	if (rc < 0)
+		fprintf(stderr, "error enabling device: %s\n",
+				strerror(-rc));
+
+	fprintf(stderr, "enabled %d device%s\n", processed,
+			processed == 1 ? "" : "s");
+	return rc;
+}
+
 int cmd_online_memory(int argc, const char **argv, struct daxctl_ctx *ctx)
 {
 	char *usage = "daxctl online-memory <device> [<options>]";
-- 
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

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

* [PATCH ndctl v2 06/10] libdaxctl: add daxctl_region_create_dev()
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
                   ` (4 preceding siblings ...)
  2020-07-13 16:08 ` [PATCH ndctl v2 05/10] daxctl: add command to enable " Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 07/10] daxctl: add command to create device Joao Martins
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add an API to create an empty seed device.

This sysfs attribute is only writeable for a dynamic
dax device such as the one exported by hmem.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 daxctl/lib/libdaxctl.c   | 26 ++++++++++++++++++++++++++
 daxctl/lib/libdaxctl.sym |  1 +
 daxctl/libdaxctl.h       |  1 +
 3 files changed, 28 insertions(+)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 00d5f7fe61ad..d17ff7a02bad 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -583,6 +583,32 @@ DAXCTL_EXPORT unsigned long long daxctl_region_get_available_size(
 	return 0;
 }
 
+DAXCTL_EXPORT int daxctl_region_create_dev(struct daxctl_region *region)
+{
+	struct daxctl_ctx *ctx = daxctl_region_get_ctx(region);
+	char *path = region->region_buf;
+	int rc, len = region->buf_len;
+	char *num_devices;
+
+	if (snprintf(path, len, "%s/%s/create", region->region_path, attrs) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				daxctl_region_get_devname(region));
+		return -EFAULT;
+	}
+
+	if (asprintf(&num_devices, "%d", 1) < 0) {
+		err(ctx, "%s: buffer too small!\n",
+				daxctl_region_get_devname(region));
+		return -EFAULT;
+	}
+
+	rc = sysfs_write_attr(ctx, path, num_devices);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
 DAXCTL_EXPORT struct daxctl_dev *daxctl_region_get_dev_seed(
 		struct daxctl_region *region)
 {
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index d8b4137c76b7..26987ba021ab 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -79,4 +79,5 @@ global:
 LIBDAXCTL_8 {
 global:
 	daxctl_dev_set_size;
+	daxctl_region_create_dev;
 } LIBDAXCTL_7;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index e3d482743cc6..a579ddd1d43c 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -56,6 +56,7 @@ unsigned long daxctl_region_get_align(struct daxctl_region *region);
 const char *daxctl_region_get_devname(struct daxctl_region *region);
 const char *daxctl_region_get_path(struct daxctl_region *region);
 
+int daxctl_region_create_dev(struct daxctl_region *region);
 struct daxctl_dev *daxctl_region_get_dev_seed(struct daxctl_region *region);
 
 struct daxctl_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

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

* [PATCH ndctl v2 07/10] daxctl: add command to create device
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
                   ` (5 preceding siblings ...)
  2020-07-13 16:08 ` [PATCH ndctl v2 06/10] libdaxctl: add daxctl_region_create_dev() Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 08/10] libdaxctl: add daxctl_region_destroy_dev() Joao Martins
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add a 'create-device' command which creates a devdax
region. So far the only new option is size, which when
omitted uses the entire available space. Example usage:

	$ daxctl reconfigure-device -s 0 dax0.0
	reconfigured 1 device

	$ daxctl create-device -s 4G -r 0
	[
	  {
	    "chardev":"dax0.1",
	    "size":4294967296,
	    "target_node":0,
	    "mode":"devdax"
	  }
	]
	created 1 device

Or using the whole available space like default behaviour
or ndctl-create-namespace:

	$ daxctl create-device
	[
	  {
	    "chardev":"dax0.1",
	    "size":120259084288,
	    "target_node":0,
	    "mode":"devdax"
	  }
	]
	created 1 device

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/daxctl/Makefile.am              |   3 +-
 Documentation/daxctl/daxctl-create-device.txt | 105 ++++++++++++++++++++++++
 daxctl/builtin.h                              |   1 +
 daxctl/daxctl.c                               |   1 +
 daxctl/device.c                               | 111 +++++++++++++++++++++++++-
 5 files changed, 219 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/daxctl/daxctl-create-device.txt

diff --git a/Documentation/daxctl/Makefile.am b/Documentation/daxctl/Makefile.am
index e43d34183142..27e201dfc254 100644
--- a/Documentation/daxctl/Makefile.am
+++ b/Documentation/daxctl/Makefile.am
@@ -33,7 +33,8 @@ man1_MANS = \
 	daxctl-online-memory.1 \
 	daxctl-offline-memory.1 \
 	daxctl-disable-device.1 \
-	daxctl-enable-device.1
+	daxctl-enable-device.1 \
+	daxctl-create-device.1
 
 EXTRA_DIST = $(man1_MANS)
 
diff --git a/Documentation/daxctl/daxctl-create-device.txt b/Documentation/daxctl/daxctl-create-device.txt
new file mode 100644
index 000000000000..648d2541f833
--- /dev/null
+++ b/Documentation/daxctl/daxctl-create-device.txt
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+
+daxctl-create-device(1)
+=======================
+
+NAME
+----
+daxctl-create-device - Create a devdax device
+
+SYNOPSIS
+--------
+[verse]
+'daxctl create-device' [<options>]
+
+EXAMPLES
+--------
+
+* Creates dax0.1 with 4G of size
+----
+# daxctl create-device -s 4G
+[
+  {
+    "chardev":"dax0.1",
+    "size":4294967296,
+    "target_node":0,
+    "mode":"devdax"
+  }
+]
+----
+
+* Creates devices with fully available size on all regions
+----
+# daxctl create-device -u
+[
+  {
+    "chardev":"dax0.1",
+    "size":"15.63 GiB (16.78 GB)",
+    "target_node":0,
+    "mode":"devdax"
+  },
+  {
+    "chardev":"dax1.1",
+    "size":"15.63 GiB (16.78 GB)",
+    "target_node":1,
+    "mode":"devdax"
+  }
+]
+----
+
+* Creates dax0.1 with fully available size on region id 0
+----
+# daxctl create-device -r 0 -u
+{
+  "chardev":"dax0.1",
+  "size":"15.63 GiB (16.78 GB)",
+  "target_node":0,
+  "mode":"devdax"
+}
+----
+
+DESCRIPTION
+-----------
+
+Creates dax device in 'devdax' mode in dynamic regions. The resultant can also
+be convereted to the 'system-ram' mode which arranges for the dax range to be
+hot-plugged into the system as regular memory.
+
+'daxctl create-device' expects that the BIOS or kernel defines a range in the
+EFI memory map with EFI_MEMORY_SP. The resultant ranges mean that it's
+100% capacity is reserved for applications.
+
+OPTIONS
+-------
+-r::
+--region=::
+	Restrict the operation to devices belonging to the specified region(s).
+	A device-dax region is a contiguous range of memory that hosts one or
+	more /dev/daxX.Y devices, where X is the region id and Y is the device
+	instance id.
+
+-s::
+--size=::
+	For regions that support dax device cretion, set the device size
+	in bytes.  Otherwise it defaults to the maximum size specified by
+	region.  This option supports the suffixes "k" or "K" for KiB, "m" or
+	"M" for MiB, "g" or "G" for GiB and "t" or "T" for TiB.
+
+	The size must be a multiple of the region alignment.
+
+-u::
+--human::
+	By default the command will output machine-friendly raw-integer
+	data. Instead, with this flag, numbers representing storage size
+	will be formatted as human readable strings with units, other
+	fields are converted to hexadecimal strings.
+
+-v::
+--verbose::
+	Emit more debug messages
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkdaxctl:daxctl-list[1],daxctl-reconfigure-device[1],daxctl-destroy-device[1]
diff --git a/daxctl/builtin.h b/daxctl/builtin.h
index 8f344f86ad20..19b33933b91b 100644
--- a/daxctl/builtin.h
+++ b/daxctl/builtin.h
@@ -6,6 +6,7 @@
 struct daxctl_ctx;
 int cmd_list(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx);
+int cmd_create_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_disable_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_enable_device(int argc, const char **argv, struct daxctl_ctx *ctx);
diff --git a/daxctl/daxctl.c b/daxctl/daxctl.c
index a4699b3780bd..1f315168c513 100644
--- a/daxctl/daxctl.c
+++ b/daxctl/daxctl.c
@@ -71,6 +71,7 @@ static struct cmd_struct commands[] = {
 	{ "list", .d_fn = cmd_list },
 	{ "help", .d_fn = cmd_help },
 	{ "migrate-device-model", .d_fn = cmd_migrate },
+	{ "create-device", .d_fn = cmd_create_device },
 	{ "reconfigure-device", .d_fn = cmd_reconfig_device },
 	{ "online-memory", .d_fn = cmd_online_memory },
 	{ "offline-memory", .d_fn = cmd_offline_memory },
diff --git a/daxctl/device.c b/daxctl/device.c
index 05a9247ecfde..c038abba8063 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -49,6 +49,7 @@ enum device_action {
 	ACTION_RECONFIG,
 	ACTION_ONLINE,
 	ACTION_OFFLINE,
+	ACTION_CREATE,
 	ACTION_DISABLE,
 	ACTION_ENABLE,
 };
@@ -72,6 +73,14 @@ OPT_STRING('s', "size", &param.size, "size", "size to switch the device to")
 OPT_BOOLEAN('\0', "no-movable", &param.no_movable, \
 		"online memory in ZONE_NORMAL")
 
+static const struct option create_options[] = {
+	BASE_OPTIONS(),
+	CREATE_OPTIONS(),
+	RECONFIG_OPTIONS(),
+	ZONE_OPTIONS(),
+	OPT_END(),
+};
+
 static const struct option reconfig_options[] = {
 	BASE_OPTIONS(),
 	CREATE_OPTIONS(),
@@ -115,7 +124,8 @@ static const char *parse_device_options(int argc, const char **argv,
 	argc = parse_options(argc, argv, options, u, 0);
 
 	/* Handle action-agnostic non-option arguments */
-	if (argc == 0) {
+	if (argc == 0 &&
+	    action != ACTION_CREATE) {
 		char *action_string;
 
 		switch (action) {
@@ -181,6 +191,10 @@ static const char *parse_device_options(int argc, const char **argv,
 			}
 		}
 		break;
+	case ACTION_CREATE:
+		if (param.size)
+			size = __parse_size64(param.size, &units);
+		/* fall through */
 	case ACTION_ONLINE:
 		if (param.no_movable)
 			mem_zone = MEM_ZONE_NORMAL;
@@ -452,6 +466,47 @@ static int reconfig_mode_devdax(struct daxctl_dev *dev)
 	return 0;
 }
 
+static int do_create(struct daxctl_region *region, long long val,
+		     struct json_object **jdevs)
+{
+	struct json_object *jdev;
+	struct daxctl_dev *dev;
+	int rc = 0;
+
+	if (daxctl_region_create_dev(region))
+		return -ENOSPC;
+
+	dev = daxctl_region_get_dev_seed(region);
+	if (!dev)
+		return -ENOSPC;
+
+	if (val == -1)
+		val = daxctl_region_get_available_size(region);
+
+	if (val <= 0)
+		return -ENOSPC;
+
+	rc = daxctl_dev_set_size(dev, val);
+	if (rc < 0)
+		return rc;
+
+	rc = daxctl_dev_enable_devdax(dev);
+	if (rc) {
+		fprintf(stderr, "%s: enable failed: %s\n",
+			daxctl_dev_get_devname(dev), strerror(-rc));
+		return rc;
+	}
+
+	*jdevs = json_object_new_array();
+	if (*jdevs) {
+		jdev = util_daxctl_dev_to_json(dev, flags);
+		if (jdev)
+			json_object_array_add(*jdevs, jdev);
+	}
+
+	return 0;
+}
+
 static int do_reconfig(struct daxctl_dev *dev, enum dev_mode mode,
 		struct json_object **jdevs)
 {
@@ -554,6 +609,42 @@ static int do_xble(struct daxctl_dev *dev, enum device_action action)
 	return rc;
 }
 
+static int do_xaction_region(enum device_action action,
+		struct daxctl_ctx *ctx, int *processed)
+{
+	struct json_object *jdevs = NULL;
+	struct daxctl_region *region;
+	int rc = -ENXIO;
+
+	*processed = 0;
+
+	daxctl_region_foreach(ctx, region) {
+		if (!util_daxctl_region_filter(region, param.region))
+			continue;
+
+		switch (action) {
+		case ACTION_CREATE:
+			rc = do_create(region, size, &jdevs);
+			if (rc == 0)
+				(*processed)++;
+			break;
+		default:
+			rc = -EINVAL;
+			break;
+		}
+	}
+
+	/*
+	 * jdevs is the containing json array for all devices we are reporting
+	 * on. It therefore needs to be outside the region/device iterators,
+	 * and passed in to the do_<action> functions to add their objects to
+	 */
+	if (jdevs)
+		util_display_json_array(stdout, jdevs, flags);
+
+	return rc;
+}
+
 static int do_xaction_device(const char *device, enum device_action action,
 		struct daxctl_ctx *ctx, int *processed)
 {
@@ -616,6 +707,24 @@ static int do_xaction_device(const char *device, enum device_action action,
 	return rc;
 }
 
+int cmd_create_device(int argc, const char **argv, struct daxctl_ctx *ctx)
+{
+	char *usage = "daxctl create-device [<options>]";
+	int processed, rc;
+
+	parse_device_options(argc, argv, ACTION_CREATE,
+			create_options, usage, ctx);
+
+	rc = do_xaction_region(ACTION_CREATE, ctx, &processed);
+	if (rc < 0)
+		fprintf(stderr, "error creating devices: %s\n",
+				strerror(-rc));
+
+	fprintf(stderr, "created %d device%s\n", processed,
+			processed == 1 ? "" : "s");
+	return rc;
+}
+
 int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx)
 {
 	char *usage = "daxctl reconfigure-device <device> [<options>]";
-- 
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

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

* [PATCH ndctl v2 08/10] libdaxctl: add daxctl_region_destroy_dev()
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
                   ` (6 preceding siblings ...)
  2020-07-13 16:08 ` [PATCH ndctl v2 07/10] daxctl: add command to create device Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 09/10] daxctl: add command to destroy device Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions Joao Martins
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add an API to destroy an empty device.

This 'delete' sysfs attribute is only writeable for a dynamic
dax device such as the one exported by hmem.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 daxctl/lib/libdaxctl.c   | 17 +++++++++++++++++
 daxctl/lib/libdaxctl.sym |  1 +
 daxctl/libdaxctl.h       |  1 +
 3 files changed, 19 insertions(+)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index d17ff7a02bad..9b43b68facfe 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -609,6 +609,23 @@ DAXCTL_EXPORT int daxctl_region_create_dev(struct daxctl_region *region)
 	return 0;
 }
 
+DAXCTL_EXPORT int daxctl_region_destroy_dev(struct daxctl_region *region,
+					    struct daxctl_dev *dev)
+{
+	struct daxctl_ctx *ctx = daxctl_region_get_ctx(region);
+	char *path = region->region_buf;
+	int rc, len = region->buf_len;
+
+	if (snprintf(path, len, "%s/%s/delete", region->region_path, attrs) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				daxctl_region_get_devname(region));
+		return -EFAULT;
+	}
+
+	rc = sysfs_write_attr(ctx, path, daxctl_dev_get_devname(dev));
+	return rc;
+}
+
 DAXCTL_EXPORT struct daxctl_dev *daxctl_region_get_dev_seed(
 		struct daxctl_region *region)
 {
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index 26987ba021ab..33c926411037 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -80,4 +80,5 @@ LIBDAXCTL_8 {
 global:
 	daxctl_dev_set_size;
 	daxctl_region_create_dev;
+	daxctl_region_destroy_dev;
 } LIBDAXCTL_7;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index a579ddd1d43c..2b14faad1895 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -61,6 +61,7 @@ struct daxctl_dev *daxctl_region_get_dev_seed(struct daxctl_region *region);
 
 struct daxctl_dev;
 struct daxctl_dev *daxctl_dev_get_first(struct daxctl_region *region);
+int daxctl_region_destroy_dev(struct daxctl_region *region, struct daxctl_dev *dev);
 struct daxctl_dev *daxctl_dev_get_next(struct daxctl_dev *dev);
 struct daxctl_region *daxctl_dev_get_region(struct daxctl_dev *dev);
 int daxctl_dev_get_id(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

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

* [PATCH ndctl v2 09/10] daxctl: add command to destroy device
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
                   ` (7 preceding siblings ...)
  2020-07-13 16:08 ` [PATCH ndctl v2 08/10] libdaxctl: add daxctl_region_destroy_dev() Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-13 16:08 ` [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions Joao Martins
  9 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add a 'destroy-device' command which destroys a
dax device. Like namespaces, the device needs to
be disabled in order to be destroyed. Example usage:

	$ daxctl disable-device dax0.1
	disabled 1 device
	$ daxctl destroy-device dax0.1
	destroyed 1 device

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/daxctl/Makefile.am               |  3 +-
 Documentation/daxctl/daxctl-destroy-device.txt | 63 ++++++++++++++++++++++++
 daxctl/builtin.h                               |  1 +
 daxctl/daxctl.c                                |  1 +
 daxctl/device.c                                | 66 ++++++++++++++++++++++++++
 5 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/daxctl/daxctl-destroy-device.txt

diff --git a/Documentation/daxctl/Makefile.am b/Documentation/daxctl/Makefile.am
index 27e201dfc254..2b3f92ca5b96 100644
--- a/Documentation/daxctl/Makefile.am
+++ b/Documentation/daxctl/Makefile.am
@@ -34,7 +34,8 @@ man1_MANS = \
 	daxctl-offline-memory.1 \
 	daxctl-disable-device.1 \
 	daxctl-enable-device.1 \
-	daxctl-create-device.1
+	daxctl-create-device.1 \
+	daxctl-destroy-device.1
 
 EXTRA_DIST = $(man1_MANS)
 
diff --git a/Documentation/daxctl/daxctl-destroy-device.txt b/Documentation/daxctl/daxctl-destroy-device.txt
new file mode 100644
index 000000000000..1c91cb2fab75
--- /dev/null
+++ b/Documentation/daxctl/daxctl-destroy-device.txt
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+
+daxctl-destroy-device(1)
+========================
+
+NAME
+----
+daxctl-destroy-device - Destroy a devdax device
+
+SYNOPSIS
+--------
+[verse]
+'daxctl destroy-device' <dax0.0> [<dax1.0>...<daxY.Z>] [<options>]
+
+EXAMPLES
+--------
+
+* Destroys dax0.1
+----
+# daxctl disable-device dax0.1
+disabled 1 device
+# daxctl destroy-device dax0.1
+destroyed 1 device
+----
+
+* Destroys all devices in region id 0
+----
+# daxctl disable-device -r 0 all
+disabled 3 devices
+# daxctl destroy-device -r 0 all
+destroyed 2 devices
+----
+
+DESCRIPTION
+-----------
+
+Destroys a dax device in 'devdax' mode.
+
+OPTIONS
+-------
+-r::
+--region=::
+	Restrict the operation to devices belonging to the specified region(s).
+	A device-dax region is a contiguous range of memory that hosts one or
+	more /dev/daxX.Y devices, where X is the region id and Y is the device
+	instance id.
+
+-u::
+--human::
+	By default the command will output machine-friendly raw-integer
+	data. Instead, with this flag, numbers representing storage size
+	will be formatted as human readable strings with units, other
+	fields are converted to hexadecimal strings.
+
+-v::
+--verbose::
+	Emit more debug messages
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkdaxctl:daxctl-list[1],daxctl-reconfigure-device[1],daxctl-create-device[1]
diff --git a/daxctl/builtin.h b/daxctl/builtin.h
index 19b33933b91b..29ba63ca17aa 100644
--- a/daxctl/builtin.h
+++ b/daxctl/builtin.h
@@ -7,6 +7,7 @@ struct daxctl_ctx;
 int cmd_list(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_create_device(int argc, const char **argv, struct daxctl_ctx *ctx);
+int cmd_destroy_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_disable_device(int argc, const char **argv, struct daxctl_ctx *ctx);
 int cmd_enable_device(int argc, const char **argv, struct daxctl_ctx *ctx);
diff --git a/daxctl/daxctl.c b/daxctl/daxctl.c
index 1f315168c513..bd5539900391 100644
--- a/daxctl/daxctl.c
+++ b/daxctl/daxctl.c
@@ -72,6 +72,7 @@ static struct cmd_struct commands[] = {
 	{ "help", .d_fn = cmd_help },
 	{ "migrate-device-model", .d_fn = cmd_migrate },
 	{ "create-device", .d_fn = cmd_create_device },
+	{ "destroy-device", .d_fn = cmd_destroy_device },
 	{ "reconfigure-device", .d_fn = cmd_reconfig_device },
 	{ "online-memory", .d_fn = cmd_online_memory },
 	{ "offline-memory", .d_fn = cmd_offline_memory },
diff --git a/daxctl/device.c b/daxctl/device.c
index c038abba8063..05293d6c38ee 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -52,6 +52,7 @@ enum device_action {
 	ACTION_CREATE,
 	ACTION_DISABLE,
 	ACTION_ENABLE,
+	ACTION_DESTROY,
 };
 
 #define BASE_OPTIONS() \
@@ -69,6 +70,10 @@ OPT_BOOLEAN('f', "force", &param.force, \
 #define CREATE_OPTIONS() \
 OPT_STRING('s', "size", &param.size, "size", "size to switch the device to")
 
+#define DESTROY_OPTIONS() \
+OPT_BOOLEAN('f', "force", &param.force, \
+		"attempt to disable before destroying device")
+
 #define ZONE_OPTIONS() \
 OPT_BOOLEAN('\0', "no-movable", &param.no_movable, \
 		"online memory in ZONE_NORMAL")
@@ -110,6 +115,12 @@ static const struct option enable_options[] = {
 	OPT_END(),
 };
 
+static const struct option destroy_options[] = {
+	BASE_OPTIONS(),
+	DESTROY_OPTIONS(),
+	OPT_END(),
+};
+
 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)
@@ -144,6 +155,9 @@ static const char *parse_device_options(int argc, const char **argv,
 		case ACTION_ENABLE:
 			action_string = "enable";
 			break;
+		case ACTION_DESTROY:
+			action_string = "destroy";
+			break;
 		default:
 			action_string = "<>";
 			break;
@@ -199,6 +213,7 @@ static const char *parse_device_options(int argc, const char **argv,
 		if (param.no_movable)
 			mem_zone = MEM_ZONE_NORMAL;
 		/* fall through */
+	case ACTION_DESTROY:
 	case ACTION_OFFLINE:
 	case ACTION_DISABLE:
 	case ACTION_ENABLE:
@@ -364,6 +379,35 @@ static int dev_resize(struct daxctl_dev *dev, unsigned long long val)
 	return 0;
 }
 
+static int dev_destroy(struct daxctl_dev *dev)
+{
+	const char *devname = daxctl_dev_get_devname(dev);
+	int rc;
+
+	if (daxctl_dev_is_enabled(dev) && !param.force) {
+		fprintf(stderr, "%s is active, specify --force for deletion\n",
+			devname);
+		return -ENXIO;
+	} else {
+		rc = daxctl_dev_disable(dev);
+		if (rc) {
+			fprintf(stderr, "%s: disable failed: %s\n",
+				daxctl_dev_get_devname(dev), strerror(-rc));
+			return rc;
+		}
+	}
+
+	rc = daxctl_dev_set_size(dev, 0);
+	if (rc < 0)
+		return rc;
+
+	rc = daxctl_region_destroy_dev(daxctl_dev_get_region(dev), dev);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
 static int disable_devdax_device(struct daxctl_dev *dev)
 {
 	struct daxctl_memory *mem = daxctl_dev_get_memory(dev);
@@ -689,6 +733,11 @@ static int do_xaction_device(const char *device, enum device_action action,
 				if (rc == 0)
 					(*processed)++;
 				break;
+			case ACTION_DESTROY:
+				rc = dev_destroy(dev);
+				if (rc == 0)
+					(*processed)++;
+				break;
 			default:
 				rc = -EINVAL;
 				break;
@@ -725,6 +774,23 @@ int cmd_create_device(int argc, const char **argv, struct daxctl_ctx *ctx)
 	return rc;
 }
 
+int cmd_destroy_device(int argc, const char **argv, struct daxctl_ctx *ctx)
+{
+	char *usage = "daxctl destroy-device <device> [<options>]";
+	const char *device = parse_device_options(argc, argv, ACTION_DESTROY,
+			destroy_options, usage, ctx);
+	int processed, rc;
+
+	rc = do_xaction_device(device, ACTION_DESTROY, ctx, &processed);
+	if (rc < 0)
+		fprintf(stderr, "error destroying devices: %s\n",
+				strerror(-rc));
+
+	fprintf(stderr, "destroyed %d device%s\n", processed,
+			processed == 1 ? "" : "s");
+	return rc;
+}
+
 int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx)
 {
 	char *usage = "daxctl reconfigure-device <device> [<options>]";
-- 
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

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

* [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions
  2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
                   ` (8 preceding siblings ...)
  2020-07-13 16:08 ` [PATCH ndctl v2 09/10] daxctl: add command to destroy device Joao Martins
@ 2020-07-13 16:08 ` Joao Martins
  2020-07-21 16:49   ` Joao Martins
  9 siblings, 1 reply; 16+ messages in thread
From: Joao Martins @ 2020-07-13 16:08 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dan Williams, Vishal Verma

Add a couple tests which exercise the new sysfs based
interface for Soft-Reserved regions (by EFI/HMAT, or
efi_fake_mem).

The tests exercise the daxctl orchestration surrounding
for creating/disabling/destroying/reconfiguring devices.
Furthermore it exercises dax region space allocation
code paths particularly:

 1) zeroing out and reconfiguring a dax device from
 its current size to be max available and back to initial
 size

 2) creates devices from holes in the beginning,
 middle of the region.

 3) reconfigures devices in a interleaving fashion

 4) test adjust of the region towards beginning and end

The tests assume you pass a valid efi_fake_mem parameter
marked as EFI_MEMORY_SP e.g.

	efi_fake_mem=112G@16G:0x40000

Naturally it bails out from the test if hmem device driver
isn't loaded or builtin. If hmem regions are found, only
region 0 is used, and the others remain untouched.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 test/Makefile.am      |   1 +
 test/daxctl-create.sh | 294 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 295 insertions(+)
 create mode 100755 test/daxctl-create.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index 1d24a65ded8b..6b7c82f9a4e2 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -58,6 +58,7 @@ TESTS +=\
 	device-dax \
 	device-dax-fio.sh \
 	daxctl-devices.sh \
+	daxctl-create.sh \
 	dm.sh \
 	mmap.sh
 
diff --git a/test/daxctl-create.sh b/test/daxctl-create.sh
new file mode 100755
index 000000000000..0d35112b4119
--- /dev/null
+++ b/test/daxctl-create.sh
@@ -0,0 +1,294 @@
+#!/bin/bash -Ex
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2020 Oracle Corporation.
+
+rc=77
+. $(dirname $0)/common
+
+trap 'cleanup $LINENO' ERR
+
+cleanup()
+{
+	printf "Error at line %d\n" "$1"
+	[[ $testdev ]] && reset
+	exit $rc
+}
+
+find_testdev()
+{
+	local rc=77
+
+	# The hmem driver is needed to change the device mode, only
+	# kernels >= v5.6 might have it available. Skip if not.
+	if ! modinfo hmem; then
+		# check if hmem is builtin
+		if [ ! -d "/sys/module/device_hmem" ]; then
+			printf "Unable to find hmem module\n"
+			exit $rc
+		fi
+	fi
+
+	# find a victim region provided by dax_hmem
+	testpath=$("$DAXCTL" list -r 0 | jq -er '.[0].path | .//""')
+	if [[ ! "$testpath" == *"hmem"* ]]; then
+		printf "Unable to find a victim region\n"
+		exit "$rc"
+	fi
+
+	# find a victim device
+	testdev=$("$DAXCTL" list -D -r 0 | jq -er '.[0].chardev | .//""')
+	if [[ ! $testdev  ]]; then
+		printf "Unable to find a victim device\n"
+		exit "$rc"
+	fi
+	printf "Found victim dev: %s on region id 0\n" "$testdev"
+}
+
+setup_dev()
+{
+	test -n "$testdev"
+
+	"$DAXCTL" disable-device "$testdev"
+	"$DAXCTL" reconfigure-device -s 0 "$testdev"
+	available=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
+}
+
+reset_dev()
+{
+	test -n "$testdev"
+
+	"$DAXCTL" disable-device "$testdev"
+	"$DAXCTL" reconfigure-device -s $available "$testdev"
+	"$DAXCTL" enable-device "$testdev"
+}
+
+reset()
+{
+	test -n "$testdev"
+
+	"$DAXCTL" disable-device -r 0 all
+	"$DAXCTL" destroy-device -r 0 all
+	"$DAXCTL" reconfigure-device -s $available "$testdev"
+}
+
+clear_dev()
+{
+	"$DAXCTL" disable-device "$testdev"
+	"$DAXCTL" reconfigure-device -s 0 "$testdev"
+}
+
+test_pass()
+{
+	local rc=1
+
+	# Available size
+	_available_size=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
+	if [[ ! $_available_size == $available ]]; then
+		printf "Unexpected available size $_available_size != $available\n"
+		exit "$rc"
+	fi
+}
+
+fail_if_available()
+{
+	local rc=1
+
+	_size=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
+	if [[ $_size ]]; then
+		printf "Unexpected available size $_size\n"
+		exit "$rc"
+	fi
+}
+
+daxctl_get_dev()
+{
+	"$DAXCTL" list -d "$1" | jq -er '.[].chardev'
+}
+
+daxctl_get_mode()
+{
+	"$DAXCTL" list -d "$1" | jq -er '.[].mode'
+}
+
+daxctl_test_multi()
+{
+	local daxdev
+
+	size=$(expr $available / 4)
+
+	if [[ $2 ]]; then
+		"$DAXCTL" disable-device "$testdev"
+		"$DAXCTL" reconfigure-device -s $size "$testdev"
+	fi
+
+	daxdev_1=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
+	test -n $daxdev_1
+
+	daxdev_2=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
+	test -n $daxdev_2
+
+	if [[ ! $2 ]]; then
+		daxdev_3=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
+		test -n $daxdev_3
+	fi
+
+	# Hole
+	"$DAXCTL" disable-device  $1 &&	"$DAXCTL" destroy-device  $1
+
+	# Pick space in the created hole and at the end
+	new_size=$(expr $size \* 2)
+	daxdev_4=$("$DAXCTL" create-device -r 0 -s $new_size | jq -er '.[].chardev')
+	test -n $daxdev_4
+
+	fail_if_available
+
+	"$DAXCTL" disable-device -r 0 all
+	"$DAXCTL" destroy-device -r 0 all
+}
+
+daxctl_test_multi_reconfig()
+{
+	local ncfgs=$1
+	local daxdev
+
+	size=$(expr $available / $ncfgs)
+
+	test -n "$testdev"
+
+	"$DAXCTL" disable-device "$testdev"
+	"$DAXCTL" reconfigure-device -s $size "$testdev"
+	"$DAXCTL" disable-device "$testdev"
+
+	daxdev_1=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
+	"$DAXCTL" disable-device "$daxdev_1"
+
+	start=$(expr $size + $size)
+	max=$(expr $ncfgs / 2 \* $size)
+	for i in $(seq $start $size $max)
+	do
+		"$DAXCTL" disable-device "$testdev"
+		"$DAXCTL" reconfigure-device -s $i "$testdev"
+
+		"$DAXCTL" disable-device "$daxdev_1"
+		"$DAXCTL" reconfigure-device -s $i "$daxdev_1"
+	done
+
+	fail_if_available
+
+	"$DAXCTL" disable-device "$daxdev_1" && "$DAXCTL" destroy-device "$daxdev_1"
+}
+
+daxctl_test_adjust()
+{
+	local rc=1
+	local ncfgs=4
+	local daxdev
+
+	size=$(expr $available / $ncfgs)
+
+	test -n "$testdev"
+
+	start=$(expr $size + $size)
+	for i in $(seq 1 1 $ncfgs)
+	do
+		daxdev=$("$DAXCTL" create-device -r 0 -s $size)
+	done
+
+	daxdev=$(daxctl_get_dev "dax0.1")
+	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"
+	daxdev=$(daxctl_get_dev "dax0.4")
+	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"
+
+	daxdev=$(daxctl_get_dev "dax0.2")
+	"$DAXCTL" disable-device "$daxdev"
+	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
+
+	daxdev=$(daxctl_get_dev "dax0.3")
+	"$DAXCTL" disable-device "$daxdev"
+	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
+
+	fail_if_available
+
+	daxdev=$(daxctl_get_dev "dax0.3")
+	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"
+	daxdev=$(daxctl_get_dev "dax0.2")
+	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"
+}
+
+# Test 0:
+# Sucessfully zero out the region device and allocate the whole space again.
+daxctl_test0()
+{
+	clear_dev
+	test_pass
+}
+
+# Test 1:
+# Sucessfully creates and destroys a device with the whole available space
+daxctl_test1()
+{
+	local daxdev
+
+	daxdev=$("$DAXCTL" create-device -r 0 | jq -er '.[].chardev')
+
+	test -n "$daxdev"
+	fail_if_available
+
+	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"
+
+	clear_dev
+	test_pass
+}
+
+# Test 2: space at the middle and at the end
+# Successfully pick space in the middle and space at the end, by
+# having the region device reconfigured with some of the memory.
+daxctl_test2()
+{
+	daxctl_test_multi "dax0.1" 1
+	clear_dev
+	test_pass
+}
+
+# Test 3: space at the beginning and at the end
+# Successfully pick space in the beginning and space at the end, by
+# having the region device emptied (so region beginning starts with dax0.1).
+daxctl_test3()
+{
+	daxctl_test_multi "dax0.1"
+	clear_dev
+	test_pass
+}
+
+# Test 4: space at the end
+# Successfully reconfigure two devices in increasingly bigger allocations.
+# The difference is that it reuses an existing resource, and only needs to
+# pick at the end of the region
+daxctl_test4()
+{
+	daxctl_test_multi_reconfig 8
+	clear_dev
+	test_pass
+}
+
+# Test 5: space adjust
+# Successfully adjusts two resources to fill the whole region
+# First adjusts towards the beginning of region, the second towards the end.
+daxctl_test5()
+{
+	daxctl_test_adjust
+	clear_dev
+	test_pass
+}
+
+find_testdev
+rc=1
+setup_dev
+daxctl_test0
+daxctl_test1
+daxctl_test2
+daxctl_test3
+daxctl_test4
+daxctl_test5
+reset_dev
+exit 0
-- 
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

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

* Re: [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions
  2020-07-13 16:08 ` [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions Joao Martins
@ 2020-07-21 16:49   ` Joao Martins
  2020-12-10 15:01     ` Joao Martins
  0 siblings, 1 reply; 16+ messages in thread
From: Joao Martins @ 2020-07-21 16:49 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-nvdimm

On 7/13/20 5:08 PM, Joao Martins wrote:
> Add a couple tests which exercise the new sysfs based
> interface for Soft-Reserved regions (by EFI/HMAT, or
> efi_fake_mem).
> 
> The tests exercise the daxctl orchestration surrounding
> for creating/disabling/destroying/reconfiguring devices.
> Furthermore it exercises dax region space allocation
> code paths particularly:
> 
>  1) zeroing out and reconfiguring a dax device from
>  its current size to be max available and back to initial
>  size
> 
>  2) creates devices from holes in the beginning,
>  middle of the region.
> 
>  3) reconfigures devices in a interleaving fashion
> 
>  4) test adjust of the region towards beginning and end
> 
> The tests assume you pass a valid efi_fake_mem parameter
> marked as EFI_MEMORY_SP e.g.
> 
> 	efi_fake_mem=112G@16G:0x40000
> 
> Naturally it bails out from the test if hmem device driver
> isn't loaded or builtin. If hmem regions are found, only
> region 0 is used, and the others remain untouched.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>

Following your suggestion[0], I added a couple more validations
to this test suite, covering the mappings. So on top of this patch
I added the following snip below the scissors mark. Mainly, I check
that the size calculated by mappingNNNN is the same as advertised by
the sysfs size attribute, thus looping through all the mappings.

Perhaps it would be enough to have just such validation in setup_dev()
to catch the bug in [0]. But I went ahead and also validated the test
cases where a certain amount of mappings are meant to be created.

My only worry is the last piece in daxctl_test_adjust() where we might
be tying too much on how a kernel version picks space from the region;
should this logic change in an unforeseeable future (e.g. allowing space
at the beginning to be adjusted). Otherwise, if this no concern, let me
know and I can resend a v3 with the adjustment below.

----->8------
Subject: Validate @size versus mappingX sizes

[0]
https://lore.kernel.org/linux-nvdimm/CAPcyv4hFS7JS9s7cUY=2Ru2kUTRsesxwX1PGnnc_tudJjoDUGw@mail.gmail.com/

---

 test/daxctl-create.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/test/daxctl-create.sh b/test/daxctl-create.sh
index 0d35112b4119..8dbc00fc574f 100755
--- a/test/daxctl-create.sh
+++ b/test/daxctl-create.sh
@@ -46,8 +46,16 @@ find_testdev()

 setup_dev()
 {
+	local rc=1
+	local nmaps=0
 	test -n "$testdev"

+	nmaps=$(daxctl_get_nr_mappings "$testdev")
+	if [[ $nmaps == 0 ]]; then
+		printf "Device is idle"
+		exit "$rc"
+	fi
+
 	"$DAXCTL" disable-device "$testdev"
 	"$DAXCTL" reconfigure-device -s 0 "$testdev"
 	available=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
@@ -110,6 +118,47 @@ daxctl_get_mode()
 	"$DAXCTL" list -d "$1" | jq -er '.[].mode'
 }

+daxctl_get_size_by_mapping()
+{
+	local size=0
+	local _start=0
+	local _end=0
+
+	_start=$(cat $1/start)
+	_end=$(cat $1/end)
+	((size=size + _end - _start + 1))
+	echo $size
+}
+
+daxctl_get_nr_mappings()
+{
+	local i=0
+	local _size=0
+	local devsize=0
+	local path=""
+
+	path=$(readlink -f /sys/bus/dax/devices/"$1"/)
+	until ! [ -d $path/mapping$i ]
+	do
+		_size=$(daxctl_get_size_by_mapping "$path/mapping$i")
+		if [[ $msize == 0 ]]; then
+			i=0
+			break
+		fi
+		((devsize=devsize + _size))
+		((i=i + 1))
+	done
+
+	# Return number of mappings if the sizes between size field
+	# and the one computed by mappingNNN are the same
+	_size=$("$DAXCTL" list -d $1 | jq -er '.[0].size | .//""')
+	if [[ ! $_size == $devsize ]]; then
+		echo 0
+	else
+		echo $i
+	fi
+}
+
 daxctl_test_multi()
 {
 	local daxdev
@@ -139,6 +188,7 @@ daxctl_test_multi()
 	new_size=$(expr $size \* 2)
 	daxdev_4=$("$DAXCTL" create-device -r 0 -s $new_size | jq -er '.[].chardev')
 	test -n $daxdev_4
+	test $(daxctl_get_nr_mappings $daxdev_4) -eq 2

 	fail_if_available

@@ -173,6 +223,9 @@ daxctl_test_multi_reconfig()
 		"$DAXCTL" reconfigure-device -s $i "$daxdev_1"
 	done

+	test $(daxctl_get_nr_mappings $testdev) -eq $((ncfgs / 2))
+	test $(daxctl_get_nr_mappings $daxdev_1) -eq $((ncfgs / 2))
+
 	fail_if_available

 	"$DAXCTL" disable-device "$daxdev_1" && "$DAXCTL" destroy-device "$daxdev_1"
@@ -191,7 +244,8 @@ daxctl_test_adjust()
 	start=$(expr $size + $size)
 	for i in $(seq 1 1 $ncfgs)
 	do
-		daxdev=$("$DAXCTL" create-device -r 0 -s $size)
+		daxdev=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
+		test $(daxctl_get_nr_mappings $daxdev) -eq 1
 	done

 	daxdev=$(daxctl_get_dev "dax0.1")
@@ -202,10 +256,17 @@ daxctl_test_adjust()
 	daxdev=$(daxctl_get_dev "dax0.2")
 	"$DAXCTL" disable-device "$daxdev"
 	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
+	# Allocates space at the beginning: expect two mappings as
+	# as don't adjust the mappingX region. This is because we
+	# preserve the relative page_offset of existing allocations
+	test $(daxctl_get_nr_mappings $daxdev) -eq 2

 	daxdev=$(daxctl_get_dev "dax0.3")
 	"$DAXCTL" disable-device "$daxdev"
 	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
+	# Adjusts space at the end, expect one mapping because we are
+	# able to extend existing region range.
+	test $(daxctl_get_nr_mappings $daxdev) -eq 1

 	fail_if_available

@@ -232,6 +293,7 @@ daxctl_test1()
 	daxdev=$("$DAXCTL" create-device -r 0 | jq -er '.[].chardev')

 	test -n "$daxdev"
+	test $(daxctl_get_nr_mappings $daxdev) -eq 1
 	fail_if_available

 	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions
  2020-07-21 16:49   ` Joao Martins
@ 2020-12-10 15:01     ` Joao Martins
  2020-12-16 10:25       ` Verma, Vishal L
  2020-12-16 10:25       ` Verma, Vishal L
  0 siblings, 2 replies; 16+ messages in thread
From: Joao Martins @ 2020-12-10 15:01 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma; +Cc: linux-nvdimm

On 7/21/20 5:49 PM, Joao Martins wrote:
> On 7/13/20 5:08 PM, Joao Martins wrote:
>> Add a couple tests which exercise the new sysfs based
>> interface for Soft-Reserved regions (by EFI/HMAT, or
>> efi_fake_mem).
>>
>> The tests exercise the daxctl orchestration surrounding
>> for creating/disabling/destroying/reconfiguring devices.
>> Furthermore it exercises dax region space allocation
>> code paths particularly:
>>
>>  1) zeroing out and reconfiguring a dax device from
>>  its current size to be max available and back to initial
>>  size
>>
>>  2) creates devices from holes in the beginning,
>>  middle of the region.
>>
>>  3) reconfigures devices in a interleaving fashion
>>
>>  4) test adjust of the region towards beginning and end
>>
>> The tests assume you pass a valid efi_fake_mem parameter
>> marked as EFI_MEMORY_SP e.g.
>>
>> 	efi_fake_mem=112G@16G:0x40000
>>
>> Naturally it bails out from the test if hmem device driver
>> isn't loaded or builtin. If hmem regions are found, only
>> region 0 is used, and the others remain untouched.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> 
> Following your suggestion[0], I added a couple more validations
> to this test suite, covering the mappings. So on top of this patch
> I added the following snip below the scissors mark. Mainly, I check
> that the size calculated by mappingNNNN is the same as advertised by
> the sysfs size attribute, thus looping through all the mappings.
> 
> Perhaps it would be enough to have just such validation in setup_dev()
> to catch the bug in [0]. But I went ahead and also validated the test
> cases where a certain amount of mappings are meant to be created.
> 
> My only worry is the last piece in daxctl_test_adjust() where we might
> be tying too much on how a kernel version picks space from the region;
> should this logic change in an unforeseeable future (e.g. allowing space
> at the beginning to be adjusted). Otherwise, if this no concern, let me
> know and I can resend a v3 with the adjustment below.
> 

Ping?

> ----->8------
> Subject: Validate @size versus mappingX sizes
> 
> [0]
> https://lore.kernel.org/linux-nvdimm/CAPcyv4hFS7JS9s7cUY=2Ru2kUTRsesxwX1PGnnc_tudJjoDUGw@mail.gmail.com/
> 
> ---
> 
>  test/daxctl-create.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/test/daxctl-create.sh b/test/daxctl-create.sh
> index 0d35112b4119..8dbc00fc574f 100755
> --- a/test/daxctl-create.sh
> +++ b/test/daxctl-create.sh
> @@ -46,8 +46,16 @@ find_testdev()
> 
>  setup_dev()
>  {
> +	local rc=1
> +	local nmaps=0
>  	test -n "$testdev"
> 
> +	nmaps=$(daxctl_get_nr_mappings "$testdev")
> +	if [[ $nmaps == 0 ]]; then
> +		printf "Device is idle"
> +		exit "$rc"
> +	fi
> +
>  	"$DAXCTL" disable-device "$testdev"
>  	"$DAXCTL" reconfigure-device -s 0 "$testdev"
>  	available=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
> @@ -110,6 +118,47 @@ daxctl_get_mode()
>  	"$DAXCTL" list -d "$1" | jq -er '.[].mode'
>  }
> 
> +daxctl_get_size_by_mapping()
> +{
> +	local size=0
> +	local _start=0
> +	local _end=0
> +
> +	_start=$(cat $1/start)
> +	_end=$(cat $1/end)
> +	((size=size + _end - _start + 1))
> +	echo $size
> +}
> +
> +daxctl_get_nr_mappings()
> +{
> +	local i=0
> +	local _size=0
> +	local devsize=0
> +	local path=""
> +
> +	path=$(readlink -f /sys/bus/dax/devices/"$1"/)
> +	until ! [ -d $path/mapping$i ]
> +	do
> +		_size=$(daxctl_get_size_by_mapping "$path/mapping$i")
> +		if [[ $msize == 0 ]]; then
> +			i=0
> +			break
> +		fi
> +		((devsize=devsize + _size))
> +		((i=i + 1))
> +	done
> +
> +	# Return number of mappings if the sizes between size field
> +	# and the one computed by mappingNNN are the same
> +	_size=$("$DAXCTL" list -d $1 | jq -er '.[0].size | .//""')
> +	if [[ ! $_size == $devsize ]]; then
> +		echo 0
> +	else
> +		echo $i
> +	fi
> +}
> +
>  daxctl_test_multi()
>  {
>  	local daxdev
> @@ -139,6 +188,7 @@ daxctl_test_multi()
>  	new_size=$(expr $size \* 2)
>  	daxdev_4=$("$DAXCTL" create-device -r 0 -s $new_size | jq -er '.[].chardev')
>  	test -n $daxdev_4
> +	test $(daxctl_get_nr_mappings $daxdev_4) -eq 2
> 
>  	fail_if_available
> 
> @@ -173,6 +223,9 @@ daxctl_test_multi_reconfig()
>  		"$DAXCTL" reconfigure-device -s $i "$daxdev_1"
>  	done
> 
> +	test $(daxctl_get_nr_mappings $testdev) -eq $((ncfgs / 2))
> +	test $(daxctl_get_nr_mappings $daxdev_1) -eq $((ncfgs / 2))
> +
>  	fail_if_available
> 
>  	"$DAXCTL" disable-device "$daxdev_1" && "$DAXCTL" destroy-device "$daxdev_1"
> @@ -191,7 +244,8 @@ daxctl_test_adjust()
>  	start=$(expr $size + $size)
>  	for i in $(seq 1 1 $ncfgs)
>  	do
> -		daxdev=$("$DAXCTL" create-device -r 0 -s $size)
> +		daxdev=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
> +		test $(daxctl_get_nr_mappings $daxdev) -eq 1
>  	done
> 
>  	daxdev=$(daxctl_get_dev "dax0.1")
> @@ -202,10 +256,17 @@ daxctl_test_adjust()
>  	daxdev=$(daxctl_get_dev "dax0.2")
>  	"$DAXCTL" disable-device "$daxdev"
>  	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
> +	# Allocates space at the beginning: expect two mappings as
> +	# as don't adjust the mappingX region. This is because we
> +	# preserve the relative page_offset of existing allocations
> +	test $(daxctl_get_nr_mappings $daxdev) -eq 2
> 
>  	daxdev=$(daxctl_get_dev "dax0.3")
>  	"$DAXCTL" disable-device "$daxdev"
>  	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
> +	# Adjusts space at the end, expect one mapping because we are
> +	# able to extend existing region range.
> +	test $(daxctl_get_nr_mappings $daxdev) -eq 1
> 
>  	fail_if_available
> 
> @@ -232,6 +293,7 @@ daxctl_test1()
>  	daxdev=$("$DAXCTL" create-device -r 0 | jq -er '.[].chardev')
> 
>  	test -n "$daxdev"
> +	test $(daxctl_get_nr_mappings $daxdev) -eq 1
>  	fail_if_available
> 
>  	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
> 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions
  2020-12-10 15:01     ` Joao Martins
@ 2020-12-16 10:25       ` Verma, Vishal L
  2020-12-16 11:28         ` Joao Martins
  2020-12-16 10:25       ` Verma, Vishal L
  1 sibling, 1 reply; 16+ messages in thread
From: Verma, Vishal L @ 2020-12-16 10:25 UTC (permalink / raw)
  To: Williams, Dan J, joao.m.martins; +Cc: linux-nvdimm

On Thu, 2020-12-10 at 15:01 +0000, Joao Martins wrote:
> On 7/21/20 5:49 PM, Joao Martins wrote:
> > On 7/13/20 5:08 PM, Joao Martins wrote:
> > > Add a couple tests which exercise the new sysfs based
> > > interface for Soft-Reserved regions (by EFI/HMAT, or
> > > efi_fake_mem).
> > > 
> > > The tests exercise the daxctl orchestration surrounding
> > > for creating/disabling/destroying/reconfiguring devices.
> > > Furthermore it exercises dax region space allocation
> > > code paths particularly:
> > > 
> > >  1) zeroing out and reconfiguring a dax device from
> > >  its current size to be max available and back to initial
> > >  size
> > > 
> > >  2) creates devices from holes in the beginning,
> > >  middle of the region.
> > > 
> > >  3) reconfigures devices in a interleaving fashion
> > > 
> > >  4) test adjust of the region towards beginning and end
> > > 
> > > The tests assume you pass a valid efi_fake_mem parameter
> > > marked as EFI_MEMORY_SP e.g.
> > > 
> > > 	efi_fake_mem=112G@16G:0x40000
> > > 
> > > Naturally it bails out from the test if hmem device driver
> > > isn't loaded or builtin. If hmem regions are found, only
> > > region 0 is used, and the others remain untouched.
> > > 
> > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> > 
> > Following your suggestion[0], I added a couple more validations
> > to this test suite, covering the mappings. So on top of this patch
> > I added the following snip below the scissors mark. Mainly, I check
> > that the size calculated by mappingNNNN is the same as advertised by
> > the sysfs size attribute, thus looping through all the mappings.
> > 
> > Perhaps it would be enough to have just such validation in setup_dev()
> > to catch the bug in [0]. But I went ahead and also validated the test
> > cases where a certain amount of mappings are meant to be created.
> > 
> > My only worry is the last piece in daxctl_test_adjust() where we might
> > be tying too much on how a kernel version picks space from the region;
> > should this logic change in an unforeseeable future (e.g. allowing space
> > at the beginning to be adjusted). Otherwise, if this no concern, let me
> > know and I can resend a v3 with the adjustment below.
> > 
> 
> Ping?

Hi Joao,

Thanks for the patience on these, I've gone through the patches in
preparation for the next release, and they all look mostly fine. I had a
few minor fixups - to the documentation and the test (fixup module name,
and shellcheck complaints). I've appended a diff below of all the fixups
I added.

I've also included the patch below for the mapping size validation. I
think the concern for future kernel layout changes is valid, but if/when
that happens, we can always come back and relax or adjust the test as
needed. So for now, I think having a pickier test should be better than
not having one.

> 
> > ----->8------
> > Subject: Validate @size versus mappingX sizes
> > 
> > [0]
> > https://lore.kernel.org/linux-nvdimm/CAPcyv4hFS7JS9s7cUY=2Ru2kUTRsesxwX1PGnnc_tudJjoDUGw@mail.gmail.com/
> > 
> > ---
> > 
> >  test/daxctl-create.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 63 insertions(+), 1 deletion(-)
> > 

My fixups:

---

 Documentation/daxctl/daxctl-create-device.txt      | 18 +++-----------
 Documentation/daxctl/daxctl-destroy-device.txt     | 22 ++++--------------
 Documentation/daxctl/daxctl-disable-device.txt     | 22 ++++--------------
 Documentation/daxctl/daxctl-enable-device.txt      | 22 ++++--------------
 Documentation/daxctl/daxctl-reconfigure-device.txt | 19 ++++-----------
 Documentation/daxctl/human-option.txt              |  8 +++++++
 Documentation/daxctl/region-option.txt             |  8 +++++++
 Documentation/daxctl/verbose-option.txt            |  5 ++++
 util/filter.c                                      |  2 +-
 test/daxctl-create.sh                              | 76 ++++++++++++++++++++++++++++++------------------------------
 10 files changed, 82 insertions(+), 120 deletions(-)

---

diff --git a/Documentation/daxctl/daxctl-create-device.txt b/Documentation/daxctl/daxctl-create-device.txt
index 648d254..70029ab 100644
--- a/Documentation/daxctl/daxctl-create-device.txt
+++ b/Documentation/daxctl/daxctl-create-device.txt
@@ -71,12 +71,7 @@ EFI memory map with EFI_MEMORY_SP. The resultant ranges mean that it's
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
+include::region-option.txt[]
 
 -s::
 --size=::
@@ -87,16 +82,9 @@ OPTIONS
 
 	The size must be a multiple of the region alignment.
 
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
+include::human-option.txt[]
 
--v::
---verbose::
-	Emit more debug messages
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/daxctl-destroy-device.txt b/Documentation/daxctl/daxctl-destroy-device.txt
index 1c91cb2..a63ab0c 100644
--- a/Documentation/daxctl/daxctl-destroy-device.txt
+++ b/Documentation/daxctl/daxctl-destroy-device.txt
@@ -38,23 +38,11 @@ Destroys a dax device in 'devdax' mode.
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
-
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
-
--v::
---verbose::
-	Emit more debug messages
+include::region-option.txt[]
+
+include::human-option.txt[]
+
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/daxctl-disable-device.txt b/Documentation/daxctl/daxctl-disable-device.txt
index 383aeeb..ee9f6e8 100644
--- a/Documentation/daxctl/daxctl-disable-device.txt
+++ b/Documentation/daxctl/daxctl-disable-device.txt
@@ -33,23 +33,11 @@ Disables a dax device in 'devdax' mode.
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
-
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
-
--v::
---verbose::
-	Emit more debug messages
+include::region-option.txt[]
+
+include::human-option.txt[]
+
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/daxctl-enable-device.txt b/Documentation/daxctl/daxctl-enable-device.txt
index 6410d92..24cdcf3 100644
--- a/Documentation/daxctl/daxctl-enable-device.txt
+++ b/Documentation/daxctl/daxctl-enable-device.txt
@@ -34,23 +34,11 @@ Enables a dax device in 'devdax' mode.
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
-
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
-
--v::
---verbose::
-	Emit more debug messages
+include::region-option.txt[]
+
+include::human-option.txt[]
+
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/daxctl-reconfigure-device.txt b/Documentation/daxctl/daxctl-reconfigure-device.txt
index 8caae43..9a11ff5 100644
--- a/Documentation/daxctl/daxctl-reconfigure-device.txt
+++ b/Documentation/daxctl/daxctl-reconfigure-device.txt
@@ -121,12 +121,7 @@ refrain from then onlining it.
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
+include::region-option.txt[]
 
 -s::
 --size=::
@@ -161,16 +156,10 @@ include::movable-options.txt[]
 	to offline the memory on the NUMA node associated with the dax device
 	before converting it back to "devdax" mode.
 
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
 
--v::
---verbose::
-	Emit more debug messages
+include::human-option.txt[]
+
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/human-option.txt b/Documentation/daxctl/human-option.txt
new file mode 100644
index 0000000..2f4de7a
--- /dev/null
+++ b/Documentation/daxctl/human-option.txt
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+-u::
+--human::
+	By default the command will output machine-friendly raw-integer
+	data. Instead, with this flag, numbers representing storage size
+	will be formatted as human readable strings with units, other
+	fields are converted to hexadecimal strings.
diff --git a/Documentation/daxctl/region-option.txt b/Documentation/daxctl/region-option.txt
new file mode 100644
index 0000000..a824e22
--- /dev/null
+++ b/Documentation/daxctl/region-option.txt
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+-r::
+--region=::
+	Restrict the operation to devices belonging to the specified region(s).
+	A device-dax region is a contiguous range of memory that hosts one or
+	more /dev/daxX.Y devices, where X is the region id and Y is the device
+	instance id.
diff --git a/Documentation/daxctl/verbose-option.txt b/Documentation/daxctl/verbose-option.txt
new file mode 100644
index 0000000..cb62c8e
--- /dev/null
+++ b/Documentation/daxctl/verbose-option.txt
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+
+-v::
+--verbose::
+	Emit more debug messages
diff --git a/util/filter.c b/util/filter.c
index 7c8debb..8c78f32 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -342,7 +342,7 @@ struct daxctl_region *util_daxctl_region_filter(struct daxctl_region *region,
 		return region;
 
 	if ((sscanf(ident, "%d", &region_id) == 1
-				|| sscanf(ident, "region%d", &region_id) == 1)
+			|| sscanf(ident, "region%d", &region_id) == 1)
 			&& daxctl_region_get_id(region) == region_id)
 		return region;
 
diff --git a/test/daxctl-create.sh b/test/daxctl-create.sh
index 8dbc00f..a4fbe06 100755
--- a/test/daxctl-create.sh
+++ b/test/daxctl-create.sh
@@ -20,8 +20,8 @@ find_testdev()
 
 	# The hmem driver is needed to change the device mode, only
 	# kernels >= v5.6 might have it available. Skip if not.
-	if ! modinfo hmem; then
-		# check if hmem is builtin
+	if ! modinfo dax_hmem; then
+		# check if dax_hmem is builtin
 		if [ ! -d "/sys/module/device_hmem" ]; then
 			printf "Unable to find hmem module\n"
 			exit $rc
@@ -66,7 +66,7 @@ reset_dev()
 	test -n "$testdev"
 
 	"$DAXCTL" disable-device "$testdev"
-	"$DAXCTL" reconfigure-device -s $available "$testdev"
+	"$DAXCTL" reconfigure-device -s "$available" "$testdev"
 	"$DAXCTL" enable-device "$testdev"
 }
 
@@ -76,7 +76,7 @@ reset()
 
 	"$DAXCTL" disable-device -r 0 all
 	"$DAXCTL" destroy-device -r 0 all
-	"$DAXCTL" reconfigure-device -s $available "$testdev"
+	"$DAXCTL" reconfigure-device -s "$available" "$testdev"
 }
 
 clear_dev()
@@ -91,8 +91,8 @@ test_pass()
 
 	# Available size
 	_available_size=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
-	if [[ ! $_available_size == $available ]]; then
-		printf "Unexpected available size $_available_size != $available\n"
+	if [[ ! $_available_size == "$available" ]]; then
+		echo "Unexpected available size $_available_size != $available"
 		exit "$rc"
 	fi
 }
@@ -103,7 +103,7 @@ fail_if_available()
 
 	_size=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
 	if [[ $_size ]]; then
-		printf "Unexpected available size $_size\n"
+		echo "Unexpected available size $_size"
 		exit "$rc"
 	fi
 }
@@ -124,8 +124,8 @@ daxctl_get_size_by_mapping()
 	local _start=0
 	local _end=0
 
-	_start=$(cat $1/start)
-	_end=$(cat $1/end)
+	_start=$(cat "$1"/start)
+	_end=$(cat "$1"/end)
 	((size=size + _end - _start + 1))
 	echo $size
 }
@@ -138,10 +138,10 @@ daxctl_get_nr_mappings()
 	local path=""
 
 	path=$(readlink -f /sys/bus/dax/devices/"$1"/)
-	until ! [ -d $path/mapping$i ]
+	until ! [ -d "$path/mapping$i" ]
 	do
 		_size=$(daxctl_get_size_by_mapping "$path/mapping$i")
-		if [[ $msize == 0 ]]; then
+		if [[ $_size == 0 ]]; then
 			i=0
 			break
 		fi
@@ -151,8 +151,8 @@ daxctl_get_nr_mappings()
 
 	# Return number of mappings if the sizes between size field
 	# and the one computed by mappingNNN are the same
-	_size=$("$DAXCTL" list -d $1 | jq -er '.[0].size | .//""')
-	if [[ ! $_size == $devsize ]]; then
+	_size=$("$DAXCTL" list -d "$1" | jq -er '.[0].size | .//""')
+	if [[ ! $_size == "$devsize" ]]; then
 		echo 0
 	else
 		echo $i
@@ -163,7 +163,7 @@ daxctl_test_multi()
 {
 	local daxdev
 
-	size=$(expr $available / 4)
+	size=$((available / 4))
 
 	if [[ $2 ]]; then
 		"$DAXCTL" disable-device "$testdev"
@@ -171,24 +171,24 @@ daxctl_test_multi()
 	fi
 
 	daxdev_1=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
-	test -n $daxdev_1
+	test -n "$daxdev_1"
 
 	daxdev_2=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
-	test -n $daxdev_2
+	test -n "$daxdev_2"
 
 	if [[ ! $2 ]]; then
 		daxdev_3=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
-		test -n $daxdev_3
+		test -n "$daxdev_3"
 	fi
 
 	# Hole
-	"$DAXCTL" disable-device  $1 &&	"$DAXCTL" destroy-device  $1
+	"$DAXCTL" disable-device  "$1" && "$DAXCTL" destroy-device "$1"
 
 	# Pick space in the created hole and at the end
-	new_size=$(expr $size \* 2)
-	daxdev_4=$("$DAXCTL" create-device -r 0 -s $new_size | jq -er '.[].chardev')
-	test -n $daxdev_4
-	test $(daxctl_get_nr_mappings $daxdev_4) -eq 2
+	new_size=$((size * 2))
+	daxdev_4=$("$DAXCTL" create-device -r 0 -s "$new_size" | jq -er '.[].chardev')
+	test -n "$daxdev_4"
+	test "$(daxctl_get_nr_mappings "$daxdev_4")" -eq 2
 
 	fail_if_available
 
@@ -201,7 +201,7 @@ daxctl_test_multi_reconfig()
 	local ncfgs=$1
 	local daxdev
 
-	size=$(expr $available / $ncfgs)
+	size=$((available / ncfgs))
 
 	test -n "$testdev"
 
@@ -212,19 +212,19 @@ daxctl_test_multi_reconfig()
 	daxdev_1=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
 	"$DAXCTL" disable-device "$daxdev_1"
 
-	start=$(expr $size + $size)
-	max=$(expr $ncfgs / 2 \* $size)
+	start=$((size + size))
+	max=$((size * ncfgs / 2))
 	for i in $(seq $start $size $max)
 	do
 		"$DAXCTL" disable-device "$testdev"
-		"$DAXCTL" reconfigure-device -s $i "$testdev"
+		"$DAXCTL" reconfigure-device -s "$i" "$testdev"
 
 		"$DAXCTL" disable-device "$daxdev_1"
-		"$DAXCTL" reconfigure-device -s $i "$daxdev_1"
+		"$DAXCTL" reconfigure-device -s "$i" "$daxdev_1"
 	done
 
-	test $(daxctl_get_nr_mappings $testdev) -eq $((ncfgs / 2))
-	test $(daxctl_get_nr_mappings $daxdev_1) -eq $((ncfgs / 2))
+	test "$(daxctl_get_nr_mappings "$testdev")" -eq $((ncfgs / 2))
+	test "$(daxctl_get_nr_mappings "$daxdev_1")" -eq $((ncfgs / 2))
 
 	fail_if_available
 
@@ -237,15 +237,15 @@ daxctl_test_adjust()
 	local ncfgs=4
 	local daxdev
 
-	size=$(expr $available / $ncfgs)
+	size=$((available / ncfgs))
 
 	test -n "$testdev"
 
-	start=$(expr $size + $size)
+	start=$((size + size))
 	for i in $(seq 1 1 $ncfgs)
 	do
-		daxdev=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
-		test $(daxctl_get_nr_mappings $daxdev) -eq 1
+		daxdev=$("$DAXCTL" create-device -r 0 -s "$size" | jq -er '.[].chardev')
+		test "$(daxctl_get_nr_mappings "$daxdev")" -eq 1
 	done
 
 	daxdev=$(daxctl_get_dev "dax0.1")
@@ -255,18 +255,18 @@ daxctl_test_adjust()
 
 	daxdev=$(daxctl_get_dev "dax0.2")
 	"$DAXCTL" disable-device "$daxdev"
-	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
+	"$DAXCTL" reconfigure-device -s $((size * 2)) "$daxdev"
 	# Allocates space at the beginning: expect two mappings as
 	# as don't adjust the mappingX region. This is because we
 	# preserve the relative page_offset of existing allocations
-	test $(daxctl_get_nr_mappings $daxdev) -eq 2
+	test "$(daxctl_get_nr_mappings "$daxdev")" -eq 2
 
 	daxdev=$(daxctl_get_dev "dax0.3")
 	"$DAXCTL" disable-device "$daxdev"
-	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
+	"$DAXCTL" reconfigure-device -s $((size * 2)) "$daxdev"
 	# Adjusts space at the end, expect one mapping because we are
 	# able to extend existing region range.
-	test $(daxctl_get_nr_mappings $daxdev) -eq 1
+	test "$(daxctl_get_nr_mappings "$daxdev")" -eq 1
 
 	fail_if_available
 
@@ -293,7 +293,7 @@ daxctl_test1()
 	daxdev=$("$DAXCTL" create-device -r 0 | jq -er '.[].chardev')
 
 	test -n "$daxdev"
-	test $(daxctl_get_nr_mappings $daxdev) -eq 1
+	test "$(daxctl_get_nr_mappings "$daxdev")" -eq 1
 	fail_if_available
 
 	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"

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

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

* Re: [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions
  2020-12-16 10:25       ` Verma, Vishal L
@ 2020-12-16 11:28         ` Joao Martins
  0 siblings, 0 replies; 16+ messages in thread
From: Joao Martins @ 2020-12-16 11:28 UTC (permalink / raw)
  To: Verma, Vishal L, Williams, Dan J; +Cc: linux-nvdimm

On 12/16/20 10:25 AM, Verma, Vishal L wrote:
> On Thu, 2020-12-10 at 15:01 +0000, Joao Martins wrote:
>> On 7/21/20 5:49 PM, Joao Martins wrote:
>>> On 7/13/20 5:08 PM, Joao Martins wrote:
>>>> Add a couple tests which exercise the new sysfs based
>>>> interface for Soft-Reserved regions (by EFI/HMAT, or
>>>> efi_fake_mem).
>>>>
>>>> The tests exercise the daxctl orchestration surrounding
>>>> for creating/disabling/destroying/reconfiguring devices.
>>>> Furthermore it exercises dax region space allocation
>>>> code paths particularly:
>>>>
>>>>  1) zeroing out and reconfiguring a dax device from
>>>>  its current size to be max available and back to initial
>>>>  size
>>>>
>>>>  2) creates devices from holes in the beginning,
>>>>  middle of the region.
>>>>
>>>>  3) reconfigures devices in a interleaving fashion
>>>>
>>>>  4) test adjust of the region towards beginning and end
>>>>
>>>> The tests assume you pass a valid efi_fake_mem parameter
>>>> marked as EFI_MEMORY_SP e.g.
>>>>
>>>> 	efi_fake_mem=112G@16G:0x40000
>>>>
>>>> Naturally it bails out from the test if hmem device driver
>>>> isn't loaded or builtin. If hmem regions are found, only
>>>> region 0 is used, and the others remain untouched.
>>>>
>>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>>
>>> Following your suggestion[0], I added a couple more validations
>>> to this test suite, covering the mappings. So on top of this patch
>>> I added the following snip below the scissors mark. Mainly, I check
>>> that the size calculated by mappingNNNN is the same as advertised by
>>> the sysfs size attribute, thus looping through all the mappings.
>>>
>>> Perhaps it would be enough to have just such validation in setup_dev()
>>> to catch the bug in [0]. But I went ahead and also validated the test
>>> cases where a certain amount of mappings are meant to be created.
>>>
>>> My only worry is the last piece in daxctl_test_adjust() where we might
>>> be tying too much on how a kernel version picks space from the region;
>>> should this logic change in an unforeseeable future (e.g. allowing space
>>> at the beginning to be adjusted). Otherwise, if this no concern, let me
>>> know and I can resend a v3 with the adjustment below.
>>>
>>
>> Ping?
> 
> Hi Joao,
> 
> Thanks for the patience on these, I've gone through the patches in
> preparation for the next release, and they all look mostly fine. I had a
> few minor fixups - to the documentation and the test (fixup module name,
> and shellcheck complaints). I've appended a diff below of all the fixups
> I added.
> 
The adjustments are looking good AFAICT.

> I've also included the patch below for the mapping size validation. I
> think the concern for future kernel layout changes is valid, but if/when
> that happens, we can always come back and relax or adjust the test as
> needed. So for now, I think having a pickier test should be better than
> not having one.
> 
Yeah, makes sense.

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

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

* Re: [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions
  2020-12-10 15:01     ` Joao Martins
  2020-12-16 10:25       ` Verma, Vishal L
@ 2020-12-16 10:25       ` Verma, Vishal L
  1 sibling, 0 replies; 16+ messages in thread
From: Verma, Vishal L @ 2020-12-16 10:25 UTC (permalink / raw)
  To: Williams, Dan J, joao.m.martins; +Cc: linux-nvdimm

On Thu, 2020-12-10 at 15:01 +0000, Joao Martins wrote:
> On 7/21/20 5:49 PM, Joao Martins wrote:
> > On 7/13/20 5:08 PM, Joao Martins wrote:
> > > Add a couple tests which exercise the new sysfs based
> > > interface for Soft-Reserved regions (by EFI/HMAT, or
> > > efi_fake_mem).
> > > 
> > > The tests exercise the daxctl orchestration surrounding
> > > for creating/disabling/destroying/reconfiguring devices.
> > > Furthermore it exercises dax region space allocation
> > > code paths particularly:
> > > 
> > >  1) zeroing out and reconfiguring a dax device from
> > >  its current size to be max available and back to initial
> > >  size
> > > 
> > >  2) creates devices from holes in the beginning,
> > >  middle of the region.
> > > 
> > >  3) reconfigures devices in a interleaving fashion
> > > 
> > >  4) test adjust of the region towards beginning and end
> > > 
> > > The tests assume you pass a valid efi_fake_mem parameter
> > > marked as EFI_MEMORY_SP e.g.
> > > 
> > > 	efi_fake_mem=112G@16G:0x40000
> > > 
> > > Naturally it bails out from the test if hmem device driver
> > > isn't loaded or builtin. If hmem regions are found, only
> > > region 0 is used, and the others remain untouched.
> > > 
> > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> > 
> > Following your suggestion[0], I added a couple more validations
> > to this test suite, covering the mappings. So on top of this patch
> > I added the following snip below the scissors mark. Mainly, I check
> > that the size calculated by mappingNNNN is the same as advertised by
> > the sysfs size attribute, thus looping through all the mappings.
> > 
> > Perhaps it would be enough to have just such validation in setup_dev()
> > to catch the bug in [0]. But I went ahead and also validated the test
> > cases where a certain amount of mappings are meant to be created.
> > 
> > My only worry is the last piece in daxctl_test_adjust() where we might
> > be tying too much on how a kernel version picks space from the region;
> > should this logic change in an unforeseeable future (e.g. allowing space
> > at the beginning to be adjusted). Otherwise, if this no concern, let me
> > know and I can resend a v3 with the adjustment below.
> > 
> 
> Ping?

Hi Joao,

Thanks for the patience on these, I've gone through the patches in
preparation for the next release, and they all look mostly fine. I had a
few minor fixups - to the documentation and the test (fixup module name,
and shellcheck complaints). I've appended a diff below of all the fixups
I added.

I've also included the patch below for the mapping size validation. I
think the concern for future kernel layout changes is valid, but if/when
that happens, we can always come back and relax or adjust the test as
needed. So for now, I think having a pickier test should be better than
not having one.

> 
> > ----->8------
> > Subject: Validate @size versus mappingX sizes
> > 
> > [0]
> > https://lore.kernel.org/linux-nvdimm/CAPcyv4hFS7JS9s7cUY=2Ru2kUTRsesxwX1PGnnc_tudJjoDUGw@mail.gmail.com/
> > 
> > ---
> > 
> >  test/daxctl-create.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 63 insertions(+), 1 deletion(-)
> > 

My fixups:

---

Documentation/daxctl/daxctl-create-device.txt      | 18 +++-----------
 Documentation/daxctl/daxctl-destroy-device.txt     | 22 ++++--------------
 Documentation/daxctl/daxctl-disable-device.txt     | 22 ++++--------------
 Documentation/daxctl/daxctl-enable-device.txt      | 22 ++++--------------
 Documentation/daxctl/daxctl-reconfigure-device.txt | 19 ++++-----------
 Documentation/daxctl/human-option.txt              |  8 +++++++
 Documentation/daxctl/region-option.txt             |  8 +++++++
 Documentation/daxctl/verbose-option.txt            |  5 ++++
 util/filter.c                                      |  2 +-
 test/daxctl-create.sh                              | 76 ++++++++++++++++++++++++++++++------------------------------
 10 files changed, 82 insertions(+), 120 deletions(-)

---

diff --git a/Documentation/daxctl/daxctl-create-device.txt b/Documentation/daxctl/daxctl-create-device.txt
index 648d254..70029ab 100644
--- a/Documentation/daxctl/daxctl-create-device.txt
+++ b/Documentation/daxctl/daxctl-create-device.txt
@@ -71,12 +71,7 @@ EFI memory map with EFI_MEMORY_SP. The resultant ranges mean that it's
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
+include::region-option.txt[]
 
 -s::
 --size=::
@@ -87,16 +82,9 @@ OPTIONS
 
 	The size must be a multiple of the region alignment.
 
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
+include::human-option.txt[]
 
--v::
---verbose::
-	Emit more debug messages
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/daxctl-destroy-device.txt b/Documentation/daxctl/daxctl-destroy-device.txt
index 1c91cb2..a63ab0c 100644
--- a/Documentation/daxctl/daxctl-destroy-device.txt
+++ b/Documentation/daxctl/daxctl-destroy-device.txt
@@ -38,23 +38,11 @@ Destroys a dax device in 'devdax' mode.
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
-
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
-
--v::
---verbose::
-	Emit more debug messages
+include::region-option.txt[]
+
+include::human-option.txt[]
+
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/daxctl-disable-device.txt b/Documentation/daxctl/daxctl-disable-device.txt
index 383aeeb..ee9f6e8 100644
--- a/Documentation/daxctl/daxctl-disable-device.txt
+++ b/Documentation/daxctl/daxctl-disable-device.txt
@@ -33,23 +33,11 @@ Disables a dax device in 'devdax' mode.
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
-
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
-
--v::
---verbose::
-	Emit more debug messages
+include::region-option.txt[]
+
+include::human-option.txt[]
+
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/daxctl-enable-device.txt b/Documentation/daxctl/daxctl-enable-device.txt
index 6410d92..24cdcf3 100644
--- a/Documentation/daxctl/daxctl-enable-device.txt
+++ b/Documentation/daxctl/daxctl-enable-device.txt
@@ -34,23 +34,11 @@ Enables a dax device in 'devdax' mode.
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
-
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
-
--v::
---verbose::
-	Emit more debug messages
+include::region-option.txt[]
+
+include::human-option.txt[]
+
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/daxctl-reconfigure-device.txt b/Documentation/daxctl/daxctl-reconfigure-device.txt
index 8caae43..9a11ff5 100644
--- a/Documentation/daxctl/daxctl-reconfigure-device.txt
+++ b/Documentation/daxctl/daxctl-reconfigure-device.txt
@@ -121,12 +121,7 @@ refrain from then onlining it.
 
 OPTIONS
 -------
--r::
---region=::
-	Restrict the operation to devices belonging to the specified region(s).
-	A device-dax region is a contiguous range of memory that hosts one or
-	more /dev/daxX.Y devices, where X is the region id and Y is the device
-	instance id.
+include::region-option.txt[]
 
 -s::
 --size=::
@@ -161,16 +156,10 @@ include::movable-options.txt[]
 	to offline the memory on the NUMA node associated with the dax device
 	before converting it back to "devdax" mode.
 
--u::
---human::
-	By default the command will output machine-friendly raw-integer
-	data. Instead, with this flag, numbers representing storage size
-	will be formatted as human readable strings with units, other
-	fields are converted to hexadecimal strings.
 
--v::
---verbose::
-	Emit more debug messages
+include::human-option.txt[]
+
+include::verbose-option.txt[]
 
 include::../copyright.txt[]
 
diff --git a/Documentation/daxctl/human-option.txt b/Documentation/daxctl/human-option.txt
new file mode 100644
index 0000000..2f4de7a
--- /dev/null
+++ b/Documentation/daxctl/human-option.txt
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+-u::
+--human::
+	By default the command will output machine-friendly raw-integer
+	data. Instead, with this flag, numbers representing storage size
+	will be formatted as human readable strings with units, other
+	fields are converted to hexadecimal strings.
diff --git a/Documentation/daxctl/region-option.txt b/Documentation/daxctl/region-option.txt
new file mode 100644
index 0000000..a824e22
--- /dev/null
+++ b/Documentation/daxctl/region-option.txt
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+-r::
+--region=::
+	Restrict the operation to devices belonging to the specified region(s).
+	A device-dax region is a contiguous range of memory that hosts one or
+	more /dev/daxX.Y devices, where X is the region id and Y is the device
+	instance id.
diff --git a/Documentation/daxctl/verbose-option.txt b/Documentation/daxctl/verbose-option.txt
new file mode 100644
index 0000000..cb62c8e
--- /dev/null
+++ b/Documentation/daxctl/verbose-option.txt
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+
+-v::
+--verbose::
+	Emit more debug messages
diff --git a/util/filter.c b/util/filter.c
index 7c8debb..8c78f32 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -342,7 +342,7 @@ struct daxctl_region *util_daxctl_region_filter(struct daxctl_region *region,
 		return region;
 
 	if ((sscanf(ident, "%d", &region_id) == 1
-				|| sscanf(ident, "region%d", &region_id) == 1)
+			|| sscanf(ident, "region%d", &region_id) == 1)
 			&& daxctl_region_get_id(region) == region_id)
 		return region;
 
diff --git a/test/daxctl-create.sh b/test/daxctl-create.sh
index 8dbc00f..a4fbe06 100755
--- a/test/daxctl-create.sh
+++ b/test/daxctl-create.sh
@@ -20,8 +20,8 @@ find_testdev()
 
 	# The hmem driver is needed to change the device mode, only
 	# kernels >= v5.6 might have it available. Skip if not.
-	if ! modinfo hmem; then
-		# check if hmem is builtin
+	if ! modinfo dax_hmem; then
+		# check if dax_hmem is builtin
 		if [ ! -d "/sys/module/device_hmem" ]; then
 			printf "Unable to find hmem module\n"
 			exit $rc
@@ -66,7 +66,7 @@ reset_dev()
 	test -n "$testdev"
 
 	"$DAXCTL" disable-device "$testdev"
-	"$DAXCTL" reconfigure-device -s $available "$testdev"
+	"$DAXCTL" reconfigure-device -s "$available" "$testdev"
 	"$DAXCTL" enable-device "$testdev"
 }
 
@@ -76,7 +76,7 @@ reset()
 
 	"$DAXCTL" disable-device -r 0 all
 	"$DAXCTL" destroy-device -r 0 all
-	"$DAXCTL" reconfigure-device -s $available "$testdev"
+	"$DAXCTL" reconfigure-device -s "$available" "$testdev"
 }
 
 clear_dev()
@@ -91,8 +91,8 @@ test_pass()
 
 	# Available size
 	_available_size=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
-	if [[ ! $_available_size == $available ]]; then
-		printf "Unexpected available size $_available_size != $available\n"
+	if [[ ! $_available_size == "$available" ]]; then
+		echo "Unexpected available size $_available_size != $available"
 		exit "$rc"
 	fi
 }
@@ -103,7 +103,7 @@ fail_if_available()
 
 	_size=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
 	if [[ $_size ]]; then
-		printf "Unexpected available size $_size\n"
+		echo "Unexpected available size $_size"
 		exit "$rc"
 	fi
 }
@@ -124,8 +124,8 @@ daxctl_get_size_by_mapping()
 	local _start=0
 	local _end=0
 
-	_start=$(cat $1/start)
-	_end=$(cat $1/end)
+	_start=$(cat "$1"/start)
+	_end=$(cat "$1"/end)
 	((size=size + _end - _start + 1))
 	echo $size
 }
@@ -138,10 +138,10 @@ daxctl_get_nr_mappings()
 	local path=""
 
 	path=$(readlink -f /sys/bus/dax/devices/"$1"/)
-	until ! [ -d $path/mapping$i ]
+	until ! [ -d "$path/mapping$i" ]
 	do
 		_size=$(daxctl_get_size_by_mapping "$path/mapping$i")
-		if [[ $msize == 0 ]]; then
+		if [[ $_size == 0 ]]; then
 			i=0
 			break
 		fi
@@ -151,8 +151,8 @@ daxctl_get_nr_mappings()
 
 	# Return number of mappings if the sizes between size field
 	# and the one computed by mappingNNN are the same
-	_size=$("$DAXCTL" list -d $1 | jq -er '.[0].size | .//""')
-	if [[ ! $_size == $devsize ]]; then
+	_size=$("$DAXCTL" list -d "$1" | jq -er '.[0].size | .//""')
+	if [[ ! $_size == "$devsize" ]]; then
 		echo 0
 	else
 		echo $i
@@ -163,7 +163,7 @@ daxctl_test_multi()
 {
 	local daxdev
 
-	size=$(expr $available / 4)
+	size=$((available / 4))
 
 	if [[ $2 ]]; then
 		"$DAXCTL" disable-device "$testdev"
@@ -171,24 +171,24 @@ daxctl_test_multi()
 	fi
 
 	daxdev_1=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
-	test -n $daxdev_1
+	test -n "$daxdev_1"
 
 	daxdev_2=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
-	test -n $daxdev_2
+	test -n "$daxdev_2"
 
 	if [[ ! $2 ]]; then
 		daxdev_3=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
-		test -n $daxdev_3
+		test -n "$daxdev_3"
 	fi
 
 	# Hole
-	"$DAXCTL" disable-device  $1 &&	"$DAXCTL" destroy-device  $1
+	"$DAXCTL" disable-device  "$1" && "$DAXCTL" destroy-device "$1"
 
 	# Pick space in the created hole and at the end
-	new_size=$(expr $size \* 2)
-	daxdev_4=$("$DAXCTL" create-device -r 0 -s $new_size | jq -er '.[].chardev')
-	test -n $daxdev_4
-	test $(daxctl_get_nr_mappings $daxdev_4) -eq 2
+	new_size=$((size * 2))
+	daxdev_4=$("$DAXCTL" create-device -r 0 -s "$new_size" | jq -er '.[].chardev')
+	test -n "$daxdev_4"
+	test "$(daxctl_get_nr_mappings "$daxdev_4")" -eq 2
 
 	fail_if_available
 
@@ -201,7 +201,7 @@ daxctl_test_multi_reconfig()
 	local ncfgs=$1
 	local daxdev
 
-	size=$(expr $available / $ncfgs)
+	size=$((available / ncfgs))
 
 	test -n "$testdev"
 
@@ -212,19 +212,19 @@ daxctl_test_multi_reconfig()
 	daxdev_1=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
 	"$DAXCTL" disable-device "$daxdev_1"
 
-	start=$(expr $size + $size)
-	max=$(expr $ncfgs / 2 \* $size)
+	start=$((size + size))
+	max=$((size * ncfgs / 2))
 	for i in $(seq $start $size $max)
 	do
 		"$DAXCTL" disable-device "$testdev"
-		"$DAXCTL" reconfigure-device -s $i "$testdev"
+		"$DAXCTL" reconfigure-device -s "$i" "$testdev"
 
 		"$DAXCTL" disable-device "$daxdev_1"
-		"$DAXCTL" reconfigure-device -s $i "$daxdev_1"
+		"$DAXCTL" reconfigure-device -s "$i" "$daxdev_1"
 	done
 
-	test $(daxctl_get_nr_mappings $testdev) -eq $((ncfgs / 2))
-	test $(daxctl_get_nr_mappings $daxdev_1) -eq $((ncfgs / 2))
+	test "$(daxctl_get_nr_mappings "$testdev")" -eq $((ncfgs / 2))
+	test "$(daxctl_get_nr_mappings "$daxdev_1")" -eq $((ncfgs / 2))
 
 	fail_if_available
 
@@ -237,15 +237,15 @@ daxctl_test_adjust()
 	local ncfgs=4
 	local daxdev
 
-	size=$(expr $available / $ncfgs)
+	size=$((available / ncfgs))
 
 	test -n "$testdev"
 
-	start=$(expr $size + $size)
+	start=$((size + size))
 	for i in $(seq 1 1 $ncfgs)
 	do
-		daxdev=$("$DAXCTL" create-device -r 0 -s $size | jq -er '.[].chardev')
-		test $(daxctl_get_nr_mappings $daxdev) -eq 1
+		daxdev=$("$DAXCTL" create-device -r 0 -s "$size" | jq -er '.[].chardev')
+		test "$(daxctl_get_nr_mappings "$daxdev")" -eq 1
 	done
 
 	daxdev=$(daxctl_get_dev "dax0.1")
@@ -255,18 +255,18 @@ daxctl_test_adjust()
 
 	daxdev=$(daxctl_get_dev "dax0.2")
 	"$DAXCTL" disable-device "$daxdev"
-	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
+	"$DAXCTL" reconfigure-device -s $((size * 2)) "$daxdev"
 	# Allocates space at the beginning: expect two mappings as
 	# as don't adjust the mappingX region. This is because we
 	# preserve the relative page_offset of existing allocations
-	test $(daxctl_get_nr_mappings $daxdev) -eq 2
+	test "$(daxctl_get_nr_mappings "$daxdev")" -eq 2
 
 	daxdev=$(daxctl_get_dev "dax0.3")
 	"$DAXCTL" disable-device "$daxdev"
-	"$DAXCTL" reconfigure-device -s $(expr $size \* 2) "$daxdev"
+	"$DAXCTL" reconfigure-device -s $((size * 2)) "$daxdev"
 	# Adjusts space at the end, expect one mapping because we are
 	# able to extend existing region range.
-	test $(daxctl_get_nr_mappings $daxdev) -eq 1
+	test "$(daxctl_get_nr_mappings "$daxdev")" -eq 1
 
 	fail_if_available
 
@@ -293,7 +293,7 @@ daxctl_test1()
 	daxdev=$("$DAXCTL" create-device -r 0 | jq -er '.[].chardev')
 
 	test -n "$daxdev"
-	test $(daxctl_get_nr_mappings $daxdev) -eq 1
+	test "$(daxctl_get_nr_mappings "$daxdev")" -eq 1
 	fail_if_available
 
 	"$DAXCTL" disable-device "$daxdev" && "$DAXCTL" destroy-device "$daxdev"

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

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

end of thread, other threads:[~2020-12-16 11:29 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 01/10] daxctl: Cleanup whitespace Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 02/10] libdaxctl: add daxctl_dev_set_size() Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 03/10] daxctl: add resize support in reconfigure-device Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 04/10] daxctl: add command to disable devdax device Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 05/10] daxctl: add command to enable " Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 06/10] libdaxctl: add daxctl_region_create_dev() Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 07/10] daxctl: add command to create device Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 08/10] libdaxctl: add daxctl_region_destroy_dev() Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 09/10] daxctl: add command to destroy device Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions Joao Martins
2020-07-21 16:49   ` Joao Martins
2020-12-10 15:01     ` Joao Martins
2020-12-16 10:25       ` Verma, Vishal L
2020-12-16 11:28         ` Joao Martins
2020-12-16 10:25       ` Verma, Vishal L

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).