diff mbox series

[ndctl,10/10] daxctl: add --no-movable option for onlining memory

Message ID 20191002234925.9190-11-vishal.l.verma@intel.com (mailing list archive)
State Superseded
Commit 3fdaf7ed512ac564a78d6d2c7152be218e39c3c1
Headers show
Series fixes and movability for system-ram mode | expand

Commit Message

Verma, Vishal L Oct. 2, 2019, 11:49 p.m. UTC 
Add a new '--no-movable' option to daxctl commands that may online
memory - i.e. daxctl-reconfigure-device and daxctl-online-memory.

Users may wish additional control over the state of the newly added
memory. Retain the daxctl default for onlining memory as 'movable', but
allow it to be overridden using the above option.

Link: https://github.com/pmem/ndctl/issues/110
Cc: Ben Olson <ben.olson@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 Documentation/daxctl/daxctl-online-memory.txt |  2 ++
 .../daxctl/daxctl-reconfigure-device.txt      |  2 ++
 Documentation/daxctl/movable-options.txt      | 10 ++++++
 daxctl/device.c                               | 34 ++++++++++++++++---
 4 files changed, 44 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/daxctl/movable-options.txt

Comments

Dan Williams Oct. 18, 2019, 8:58 p.m. UTC | #1 
On Wed, Oct 2, 2019 at 4:49 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Add a new '--no-movable' option to daxctl commands that may online
> memory - i.e. daxctl-reconfigure-device and daxctl-online-memory.
>
> Users may wish additional control over the state of the newly added
> memory. Retain the daxctl default for onlining memory as 'movable', but
> allow it to be overridden using the above option.
>
> Link: https://github.com/pmem/ndctl/issues/110
> Cc: Ben Olson <ben.olson@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  Documentation/daxctl/daxctl-online-memory.txt |  2 ++
>  .../daxctl/daxctl-reconfigure-device.txt      |  2 ++
>  Documentation/daxctl/movable-options.txt      | 10 ++++++
>  daxctl/device.c                               | 34 ++++++++++++++++---
>  4 files changed, 44 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/daxctl/movable-options.txt
>
> diff --git a/Documentation/daxctl/daxctl-online-memory.txt b/Documentation/daxctl/daxctl-online-memory.txt
> index 5ac1cbf..08b45cc 100644
> --- a/Documentation/daxctl/daxctl-online-memory.txt
> +++ b/Documentation/daxctl/daxctl-online-memory.txt
> @@ -62,6 +62,8 @@ OPTIONS
>         more /dev/daxX.Y devices, where X is the region id and Y is the device
>         instance id.
>
> +include::movable-options.txt[]
> +
>  -u::
>  --human::
>         By default the command will output machine-friendly raw-integer
> diff --git a/Documentation/daxctl/daxctl-reconfigure-device.txt b/Documentation/daxctl/daxctl-reconfigure-device.txt
> index 4663529..cb28fed 100644
> --- a/Documentation/daxctl/daxctl-reconfigure-device.txt
> +++ b/Documentation/daxctl/daxctl-reconfigure-device.txt
> @@ -135,6 +135,8 @@ OPTIONS
>         brought online automatically and immediately with the 'online_movable'
>         policy. Use this option to disable the automatic onlining behavior.
>
> +include::movable-options.txt[]
> +
>  -f::
>  --force::
>         When converting from "system-ram" mode to "devdax", it is expected
> diff --git a/Documentation/daxctl/movable-options.txt b/Documentation/daxctl/movable-options.txt
> new file mode 100644
> index 0000000..0ddd7b6
> --- /dev/null
> +++ b/Documentation/daxctl/movable-options.txt
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +-M::
> +--no-movable::

If --movable is the default I would expect -M to be associated with
--movable. Don't we otherwise get the --no-<option> handling for free
with boolean options? I otherwise don't think we need a short option
for the "no" case.
Verma, Vishal L Oct. 18, 2019, 9:04 p.m. UTC | #2 
On Fri, 2019-10-18 at 13:58 -0700, Dan Williams wrote:
> 
> > +++ b/Documentation/daxctl/movable-options.txt
> > @@ -0,0 +1,10 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +-M::
> > +--no-movable::
> 
> If --movable is the default I would expect -M to be associated with
> --movable. Don't we otherwise get the --no-<option> handling for free
> with boolean options? I otherwise don't think we need a short option
> for the "no" case.

Yep we get the inverse options for booleans for free, but we can define
them either way - i.e. define it as --no-movable, and --movable is
implied, or the other way round.

But I agree it makes sense to remove the short option here.
Dan Williams Oct. 18, 2019, 9:25 p.m. UTC | #3 
On Fri, Oct 18, 2019 at 2:04 PM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Fri, 2019-10-18 at 13:58 -0700, Dan Williams wrote:
> >
> > > +++ b/Documentation/daxctl/movable-options.txt
> > > @@ -0,0 +1,10 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +-M::
> > > +--no-movable::
> >
> > If --movable is the default I would expect -M to be associated with
> > --movable. Don't we otherwise get the --no-<option> handling for free
> > with boolean options? I otherwise don't think we need a short option
> > for the "no" case.
>
> Yep we get the inverse options for booleans for free, but we can define
> them either way - i.e. define it as --no-movable, and --movable is
> implied, or the other way round.

Ah, I missed that we get the inverse flag option either way its defined, cool!
diff mbox series

Patch

diff --git a/Documentation/daxctl/daxctl-online-memory.txt b/Documentation/daxctl/daxctl-online-memory.txt
index 5ac1cbf..08b45cc 100644
--- a/Documentation/daxctl/daxctl-online-memory.txt
+++ b/Documentation/daxctl/daxctl-online-memory.txt
@@ -62,6 +62,8 @@  OPTIONS
 	more /dev/daxX.Y devices, where X is the region id and Y is the device
 	instance id.
 
+include::movable-options.txt[]
+
 -u::
 --human::
 	By default the command will output machine-friendly raw-integer
diff --git a/Documentation/daxctl/daxctl-reconfigure-device.txt b/Documentation/daxctl/daxctl-reconfigure-device.txt
index 4663529..cb28fed 100644
--- a/Documentation/daxctl/daxctl-reconfigure-device.txt
+++ b/Documentation/daxctl/daxctl-reconfigure-device.txt
@@ -135,6 +135,8 @@  OPTIONS
 	brought online automatically and immediately with the 'online_movable'
 	policy. Use this option to disable the automatic onlining behavior.
 
+include::movable-options.txt[]
+
 -f::
 --force::
 	When converting from "system-ram" mode to "devdax", it is expected
diff --git a/Documentation/daxctl/movable-options.txt b/Documentation/daxctl/movable-options.txt
new file mode 100644
index 0000000..0ddd7b6
--- /dev/null
+++ b/Documentation/daxctl/movable-options.txt
@@ -0,0 +1,10 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+-M::
+--no-movable::
+	'--movable' is the default. This can be overridden to online new
+	memory such that is is not 'movable'. This allows any allocation
+	to potentially be served from this memory. This may preclude subsequent
+	removal. With the '--movable' behavior (which is default), kernel
+	allocations will not consider this memory, and it will be reserved
+	for application use.
diff --git a/daxctl/device.c b/daxctl/device.c
index 28698bf..0695a08 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -21,6 +21,7 @@  static struct {
 	const char *mode;
 	int region_id;
 	bool no_online;
+	bool no_movable;
 	bool force;
 	bool human;
 	bool verbose;
@@ -37,6 +38,12 @@  enum dev_mode {
 static enum dev_mode reconfig_mode = DAXCTL_DEV_MODE_UNKNOWN;
 static unsigned long flags;
 
+enum memory_zone {
+	MEM_ZONE_MOVABLE,
+	MEM_ZONE_NORMAL,
+};
+static enum memory_zone mem_zone = MEM_ZONE_MOVABLE;
+
 enum device_action {
 	ACTION_RECONFIG,
 	ACTION_ONLINE,
@@ -55,13 +62,24 @@  OPT_BOOLEAN('N', "no-online", &param.no_online, \
 OPT_BOOLEAN('f', "force", &param.force, \
 		"attempt to offline memory sections before reconfiguration")
 
+#define ZONE_OPTIONS() \
+OPT_BOOLEAN('M', "no-movable", &param.no_movable, \
+		"online memory in ZONE_NORMAL")
+
 static const struct option reconfig_options[] = {
 	BASE_OPTIONS(),
 	RECONFIG_OPTIONS(),
+	ZONE_OPTIONS(),
+	OPT_END(),
+};
+
+static const struct option online_options[] = {
+	BASE_OPTIONS(),
+	ZONE_OPTIONS(),
 	OPT_END(),
 };
 
-static const struct option memory_options[] = {
+static const struct option offline_options[] = {
 	BASE_OPTIONS(),
 	OPT_END(),
 };
@@ -126,6 +144,8 @@  static const char *parse_device_options(int argc, const char **argv,
 		}
 		if (strcmp(param.mode, "system-ram") == 0) {
 			reconfig_mode = DAXCTL_DEV_MODE_RAM;
+			if (param.no_movable)
+				mem_zone = MEM_ZONE_NORMAL;
 		} else if (strcmp(param.mode, "devdax") == 0) {
 			reconfig_mode = DAXCTL_DEV_MODE_DEVDAX;
 			if (param.no_online) {
@@ -136,6 +156,9 @@  static const char *parse_device_options(int argc, const char **argv,
 		}
 		break;
 	case ACTION_ONLINE:
+		if (param.no_movable)
+			mem_zone = MEM_ZONE_NORMAL;
+		/* fall through */
 	case ACTION_OFFLINE:
 		/* nothing special */
 		break;
@@ -194,7 +217,10 @@  static int dev_online_memory(struct daxctl_dev *dev)
 			num_on == 1 ? "" : "s");
 
 	/* online the remaining sections */
-	rc = daxctl_memory_online(mem);
+	if (param.no_movable)
+		rc = daxctl_memory_online_no_movable(mem);
+	else
+		rc = daxctl_memory_online(mem);
 	if (rc < 0) {
 		fprintf(stderr, "%s: failed to online memory: %s\n",
 			devname, strerror(-rc));
@@ -521,7 +547,7 @@  int cmd_online_memory(int argc, const char **argv, struct daxctl_ctx *ctx)
 {
 	char *usage = "daxctl online-memory <device> [<options>]";
 	const char *device = parse_device_options(argc, argv, ACTION_ONLINE,
-			memory_options, usage, ctx);
+			online_options, usage, ctx);
 	int processed, rc;
 
 	rc = do_xaction_device(device, ACTION_ONLINE, ctx, &processed);
@@ -538,7 +564,7 @@  int cmd_offline_memory(int argc, const char **argv, struct daxctl_ctx *ctx)
 {
 	char *usage = "daxctl offline-memory <device> [<options>]";
 	const char *device = parse_device_options(argc, argv, ACTION_OFFLINE,
-			memory_options, usage, ctx);
+			offline_options, usage, ctx);
 	int processed, rc;
 
 	rc = do_xaction_device(device, ACTION_OFFLINE, ctx, &processed);