[ndctl PATCH 0/3] greedy namespace creation

[ndctl PATCH 0/3] greedy namespace creation

[Vishal Verma]

Patch 1 and 2 refactor and clarify the documentation for the way
filtering works using --bus=, --region=, and --dimm= options for all
commands.

Patch 3 adds a new --continue option to continue creating as many
namespaces as possible for the given filter spec.

Vishal Verma (3):
  Documentation: refactor 'bus options' into its own include
  Documentation: clarify bus/dimm/region filtering
  ndctl/namespace: add a --continue option to create namespaces greedily

 Documentation/ndctl/ndctl-check-namespace.txt |  4 +++
 Documentation/ndctl/ndctl-clear-errors.txt    |  4 +++
 .../ndctl/ndctl-create-namespace.txt          | 11 ++++++++
 Documentation/ndctl/ndctl-disable-dimm.txt    |  4 +++
 Documentation/ndctl/ndctl-disable-region.txt  |  4 +++
 Documentation/ndctl/ndctl-enable-dimm.txt     |  4 +++
 Documentation/ndctl/ndctl-enable-region.txt   |  4 +++
 Documentation/ndctl/ndctl-freeze-security.txt |  4 +++
 Documentation/ndctl/ndctl-inject-error.txt    |  4 +++
 Documentation/ndctl/ndctl-list.txt            |  4 +++
 .../ndctl/ndctl-remove-passphrase.txt         |  4 +++
 Documentation/ndctl/ndctl-sanitize-dimm.txt   |  4 +++
 .../ndctl/ndctl-setup-passphrase.txt          |  4 +++
 Documentation/ndctl/ndctl-update-firmware.txt |  4 +++
 .../ndctl/ndctl-update-passphrase.txt         |  4 +++
 Documentation/ndctl/ndctl-wait-overwrite.txt  |  4 +++
 Documentation/ndctl/xable-bus-options.txt     |  5 ++++
 Documentation/ndctl/xable-dimm-options.txt    | 14 +++--------
 .../ndctl/xable-namespace-options.txt         |  4 +++
 Documentation/ndctl/xable-region-options.txt  | 14 +++--------
 ndctl/namespace.c                             | 25 +++++++++++++++----
 21 files changed, 108 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/ndctl/xable-bus-options.txt

-- 
2.20.1

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

[ndctl PATCH 1/3] Documentation: refactor 'bus options' into its own include

[Vishal Verma]

Create a new 'xable-bus-options.txt' file that is included everywhere
that previously relied upon xable-region-options.txt for bus options
inclusion.

While at it, fix the formatting for the first description text block
from an include - the empty line at the start caused the first block to
be incorrectly indented. This fixes both the dimm and region variants.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 Documentation/ndctl/ndctl-check-namespace.txt   |  4 ++++
 Documentation/ndctl/ndctl-clear-errors.txt      |  4 ++++
 Documentation/ndctl/ndctl-create-namespace.txt  |  4 ++++
 Documentation/ndctl/ndctl-disable-dimm.txt      |  4 ++++
 Documentation/ndctl/ndctl-disable-region.txt    |  4 ++++
 Documentation/ndctl/ndctl-enable-dimm.txt       |  4 ++++
 Documentation/ndctl/ndctl-enable-region.txt     |  4 ++++
 Documentation/ndctl/ndctl-freeze-security.txt   |  4 ++++
 Documentation/ndctl/ndctl-inject-error.txt      |  4 ++++
 Documentation/ndctl/ndctl-list.txt              |  4 ++++
 Documentation/ndctl/ndctl-remove-passphrase.txt |  4 ++++
 Documentation/ndctl/ndctl-sanitize-dimm.txt     |  4 ++++
 Documentation/ndctl/ndctl-setup-passphrase.txt  |  4 ++++
 Documentation/ndctl/ndctl-update-firmware.txt   |  4 ++++
 Documentation/ndctl/ndctl-update-passphrase.txt |  4 ++++
 Documentation/ndctl/ndctl-wait-overwrite.txt    |  4 ++++
 Documentation/ndctl/xable-bus-options.txt       |  4 ++++
 Documentation/ndctl/xable-dimm-options.txt      | 13 +++----------
 Documentation/ndctl/xable-namespace-options.txt |  4 ++++
 Documentation/ndctl/xable-region-options.txt    | 13 +++----------
 20 files changed, 78 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/ndctl/xable-bus-options.txt

diff --git a/Documentation/ndctl/ndctl-check-namespace.txt b/Documentation/ndctl/ndctl-check-namespace.txt
index 2ece4e7..222f729 100644
--- a/Documentation/ndctl/ndctl-check-namespace.txt
+++ b/Documentation/ndctl/ndctl-check-namespace.txt
@@ -69,6 +69,10 @@ OPTIONS
 --region=::
 include::xable-region-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/Documentation/ndctl/ndctl-clear-errors.txt b/Documentation/ndctl/ndctl-clear-errors.txt
index 206ab58..8eff57e 100644
--- a/Documentation/ndctl/ndctl-clear-errors.txt
+++ b/Documentation/ndctl/ndctl-clear-errors.txt
@@ -98,6 +98,10 @@ can potentially be a very long-running operation.
 --region=::
 include::xable-region-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/Documentation/ndctl/ndctl-create-namespace.txt b/Documentation/ndctl/ndctl-create-namespace.txt
index 343733d..c9ae27c 100644
--- a/Documentation/ndctl/ndctl-create-namespace.txt
+++ b/Documentation/ndctl/ndctl-create-namespace.txt
@@ -211,6 +211,10 @@ OPTIONS
 --region=::
 include::xable-region-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/Documentation/ndctl/ndctl-disable-dimm.txt b/Documentation/ndctl/ndctl-disable-dimm.txt
index 7706ac3..caa6714 100644
--- a/Documentation/ndctl/ndctl-disable-dimm.txt
+++ b/Documentation/ndctl/ndctl-disable-dimm.txt
@@ -19,6 +19,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/Documentation/ndctl/ndctl-disable-region.txt b/Documentation/ndctl/ndctl-disable-region.txt
index ea1fd3a..3ca65a6 100644
--- a/Documentation/ndctl/ndctl-disable-region.txt
+++ b/Documentation/ndctl/ndctl-disable-region.txt
@@ -19,6 +19,10 @@ OPTIONS
 <region>::
 include::xable-region-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/Documentation/ndctl/ndctl-enable-dimm.txt b/Documentation/ndctl/ndctl-enable-dimm.txt
index cd9908d..3e7106c 100644
--- a/Documentation/ndctl/ndctl-enable-dimm.txt
+++ b/Documentation/ndctl/ndctl-enable-dimm.txt
@@ -19,6 +19,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/Documentation/ndctl/ndctl-enable-region.txt b/Documentation/ndctl/ndctl-enable-region.txt
index e5cbddb..96c2d1e 100644
--- a/Documentation/ndctl/ndctl-enable-region.txt
+++ b/Documentation/ndctl/ndctl-enable-region.txt
@@ -19,6 +19,10 @@ OPTIONS
 <region>::
 include::xable-region-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/Documentation/ndctl/ndctl-freeze-security.txt b/Documentation/ndctl/ndctl-freeze-security.txt
index dbb94e7..55aaab7 100644
--- a/Documentation/ndctl/ndctl-freeze-security.txt
+++ b/Documentation/ndctl/ndctl-freeze-security.txt
@@ -56,6 +56,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -v::
 --verbose::
         Emit debug messages.
diff --git a/Documentation/ndctl/ndctl-inject-error.txt b/Documentation/ndctl/ndctl-inject-error.txt
index 744ea50..da376ec 100644
--- a/Documentation/ndctl/ndctl-inject-error.txt
+++ b/Documentation/ndctl/ndctl-inject-error.txt
@@ -112,6 +112,10 @@ include::human-option.txt[]
 --region=::
 include::xable-region-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index 80ad610..f9c7434 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -62,6 +62,10 @@ OPTIONS
 --region=::
 include::xable-region-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -d::
 --dimm=::
 	An 'nmemX' device name, or dimm id number. The dimm id number
diff --git a/Documentation/ndctl/ndctl-remove-passphrase.txt b/Documentation/ndctl/ndctl-remove-passphrase.txt
index 3e838bb..f14e649 100644
--- a/Documentation/ndctl/ndctl-remove-passphrase.txt
+++ b/Documentation/ndctl/ndctl-remove-passphrase.txt
@@ -25,6 +25,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -v::
 --verbose::
         Emit debug messages.
diff --git a/Documentation/ndctl/ndctl-sanitize-dimm.txt b/Documentation/ndctl/ndctl-sanitize-dimm.txt
index 91abf47..b2e5fde 100644
--- a/Documentation/ndctl/ndctl-sanitize-dimm.txt
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -38,6 +38,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -c::
 --crypto-erase::
 	Replace the media encryption key on the NVDIMM causing all existing
diff --git a/Documentation/ndctl/ndctl-setup-passphrase.txt b/Documentation/ndctl/ndctl-setup-passphrase.txt
index b22d352..1219279 100644
--- a/Documentation/ndctl/ndctl-setup-passphrase.txt
+++ b/Documentation/ndctl/ndctl-setup-passphrase.txt
@@ -34,6 +34,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -k::
 --key_handle=::
 	Handle for the master 'kek' (key-encryption-key) that will be used for
diff --git a/Documentation/ndctl/ndctl-update-firmware.txt b/Documentation/ndctl/ndctl-update-firmware.txt
index 449d858..1aa7fee 100644
--- a/Documentation/ndctl/ndctl-update-firmware.txt
+++ b/Documentation/ndctl/ndctl-update-firmware.txt
@@ -24,6 +24,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -f::
 --firmware::
 	firmware file used to perform the update
diff --git a/Documentation/ndctl/ndctl-update-passphrase.txt b/Documentation/ndctl/ndctl-update-passphrase.txt
index 995efec..c7c1bfc 100644
--- a/Documentation/ndctl/ndctl-update-passphrase.txt
+++ b/Documentation/ndctl/ndctl-update-passphrase.txt
@@ -31,6 +31,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -k::
 --key_handle=::
 	Handle for the master 'kek' (key-encryption-key) that will be used for
diff --git a/Documentation/ndctl/ndctl-wait-overwrite.txt b/Documentation/ndctl/ndctl-wait-overwrite.txt
index 0fada36..eb24f39 100644
--- a/Documentation/ndctl/ndctl-wait-overwrite.txt
+++ b/Documentation/ndctl/ndctl-wait-overwrite.txt
@@ -23,6 +23,10 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -v::
 --verbose::
 	Emit debug messages.
diff --git a/Documentation/ndctl/xable-bus-options.txt b/Documentation/ndctl/xable-bus-options.txt
new file mode 100644
index 0000000..8813113
--- /dev/null
+++ b/Documentation/ndctl/xable-bus-options.txt
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
+Enforce that the operation only be carried on devices that are
+attached to the given bus. Where 'bus' can be a provider name or a bus
+id number.
diff --git a/Documentation/ndctl/xable-dimm-options.txt b/Documentation/ndctl/xable-dimm-options.txt
index 709e2e4..8826c2b 100644
--- a/Documentation/ndctl/xable-dimm-options.txt
+++ b/Documentation/ndctl/xable-dimm-options.txt
@@ -1,11 +1,4 @@
 // SPDX-License-Identifier: GPL-2.0
-
-	A 'nmemX' device name, or a dimm id number. The keyword 'all' can
-	be specified to carry out the operation on every dimm in the system,
-	optionally filtered by bus id (see --bus= option).
-
--b::
---bus=::
-	Enforce that the operation only be carried on devices that are
-	attached to the given bus. Where 'bus' can be a provider name or a bus
-	id number.
+A 'nmemX' device name, or a dimm id number. The keyword 'all' can
+be specified to carry out the operation on every dimm in the system,
+optionally filtered by bus id (see --bus= option).
diff --git a/Documentation/ndctl/xable-namespace-options.txt b/Documentation/ndctl/xable-namespace-options.txt
index babc248..1a25fee 100644
--- a/Documentation/ndctl/xable-namespace-options.txt
+++ b/Documentation/ndctl/xable-namespace-options.txt
@@ -9,6 +9,10 @@ the operation on every namespace in the system, optionally filtered by region
 --region=::
 include::xable-region-options.txt[]
 
+-b::
+--bus=::
+include::xable-bus-options.txt[]
+
 -v::
 --verbose::
 	Emit debug messages for the namespace operation
diff --git a/Documentation/ndctl/xable-region-options.txt b/Documentation/ndctl/xable-region-options.txt
index 69d3926..d5198f5 100644
--- a/Documentation/ndctl/xable-region-options.txt
+++ b/Documentation/ndctl/xable-region-options.txt
@@ -1,11 +1,4 @@
 // SPDX-License-Identifier: GPL-2.0
-
-	A 'regionX' device name, or a region id number. The keyword 'all' can
-	be specified to carry out the operation on every region in the system,
-	optionally filtered by bus id (see --bus= option).
-
--b::
---bus=::
-	Enforce that the operation only be carried on devices that are
-	attached to the given bus. Where 'bus' can be a provider name or a bus
-	id number.
+A 'regionX' device name, or a region id number. The keyword 'all' can
+be specified to carry out the operation on every region in the system,
+optionally filtered by bus id (see --bus= option).
-- 
2.20.1

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

[ndctl PATCH 2/3] Documentation: clarify bus/dimm/region filtering

[Vishal Verma]

Reword the option descriptions in xable-{bus,dimm,region}-options.txt to
clarify that the options are a filtering restriction rather than a
directive to perform an action on the supplied objects, especially in
case of the 'all' keyword.

Link: https://github.com/pmem/ndctl/issues/106
Cc: Jeff Moyer <jmoyer@redhat.com>
Reported-by: Steve Scargal <steve.scargall@intel.com>
Reported-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 Documentation/ndctl/xable-bus-options.txt    | 7 ++++---
 Documentation/ndctl/xable-dimm-options.txt   | 7 ++++---
 Documentation/ndctl/xable-region-options.txt | 7 ++++---
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/ndctl/xable-bus-options.txt b/Documentation/ndctl/xable-bus-options.txt
index 8813113..6e0435a 100644
--- a/Documentation/ndctl/xable-bus-options.txt
+++ b/Documentation/ndctl/xable-bus-options.txt
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
-Enforce that the operation only be carried on devices that are
-attached to the given bus. Where 'bus' can be a provider name or a bus
-id number.
+A bus id number, or a provider string (e.g. "ACPI.NFIT"). Restrict the
+operation to the specified bus(es). The keyword 'all' can be specified
+to indicate the lack of any restriction, however this is the same as
+not supplying a --bus option at all.
diff --git a/Documentation/ndctl/xable-dimm-options.txt b/Documentation/ndctl/xable-dimm-options.txt
index 8826c2b..c5b9c8c 100644
--- a/Documentation/ndctl/xable-dimm-options.txt
+++ b/Documentation/ndctl/xable-dimm-options.txt
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
-A 'nmemX' device name, or a dimm id number. The keyword 'all' can
-be specified to carry out the operation on every dimm in the system,
-optionally filtered by bus id (see --bus= option).
+A 'nmemX' device name, or a dimm id number. Restrict the operation to
+the specified dimm(s). The keyword 'all' can be specified to indicate
+the lack of any restriction, however this is the same as not supplying
+a --dimm option at all.
diff --git a/Documentation/ndctl/xable-region-options.txt b/Documentation/ndctl/xable-region-options.txt
index d5198f5..e098684 100644
--- a/Documentation/ndctl/xable-region-options.txt
+++ b/Documentation/ndctl/xable-region-options.txt
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
-A 'regionX' device name, or a region id number. The keyword 'all' can
-be specified to carry out the operation on every region in the system,
-optionally filtered by bus id (see --bus= option).
+A 'regionX' device name, or a region id number. Restrict the operation to
+the specified region(s). The keyword 'all' can be specified to indicate
+the lack of any restriction, however this is the same as not supplying a
+--region option at all.
-- 
2.20.1

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

[ndctl PATCH 3/3] ndctl/namespace: add a --continue option to create namespaces greedily

[Vishal Verma]

Add a --continue option to ndctl-create-namespaces to allow the creation
of as many namespaces as possible, that meet the given filter
restrictions.

The creation loop will be aborted if a failure is encountered at any
point.

Link: https://github.com/pmem/ndctl/issues/106
Reported-by: Steve Scargal <steve.scargall@intel.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 .../ndctl/ndctl-create-namespace.txt          |  7 ++++++
 ndctl/namespace.c                             | 25 +++++++++++++++----
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Documentation/ndctl/ndctl-create-namespace.txt b/Documentation/ndctl/ndctl-create-namespace.txt
index c9ae27c..55a8581 100644
--- a/Documentation/ndctl/ndctl-create-namespace.txt
+++ b/Documentation/ndctl/ndctl-create-namespace.txt
@@ -215,6 +215,13 @@ include::xable-region-options.txt[]
 --bus=::
 include::xable-bus-options.txt[]
 
+-c::
+--continue::
+	Do not stop after creating one namespace. Instead, greedily create as
+	many namespaces as possible within the given --bus and --region filter
+	restrictions. This will abort if any creation attempt results in an
+	error.
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index af20a42..8d6b249 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -41,6 +41,7 @@ static struct parameters {
 	bool do_scan;
 	bool mode_default;
 	bool autolabel;
+	bool greedy;
 	const char *bus;
 	const char *map;
 	const char *type;
@@ -114,7 +115,9 @@ OPT_STRING('t', "type", &param.type, "type", \
 OPT_STRING('a', "align", &param.align, "align", \
 	"specify the namespace alignment in bytes (default: 2M)"), \
 OPT_BOOLEAN('f', "force", &force, "reconfigure namespace even if currently active"), \
-OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels")
+OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels"), \
+OPT_BOOLEAN('c', "continue", &param.greedy, \
+	"continue creating namespaces as long as the filter criteria are met")
 
 #define CHECK_OPTIONS() \
 OPT_BOOLEAN('R', "repair", &repair, "perform metadata repairs"), \
@@ -1365,8 +1368,11 @@ static int do_xaction_namespace(const char *namespace,
 				rc = namespace_create(region);
 				if (rc == -EAGAIN)
 					continue;
-				if (rc == 0)
-					*processed = 1;
+				if (rc == 0) {
+					(*processed)++;
+					if (param.greedy)
+						continue;
+				}
 				return rc;
 			}
 			ndctl_namespace_foreach_safe(region, ndns, _n) {
@@ -1427,9 +1433,15 @@ static int do_xaction_namespace(const char *namespace,
 		/*
 		 * Namespace creation searched through all candidate
 		 * regions and all of them said "nope, I don't have
-		 * enough capacity", so report -ENOSPC
+		 * enough capacity", so report -ENOSPC. Except during
+		 * greedy namespace creation using --continue as we
+		 * may have created some namespaces already, and the
+		 * last one in the region search may preexist.
 		 */
-		rc = -ENOSPC;
+		if (param.greedy && (*processed) > 0)
+			rc = 0;
+		else
+			rc = -ENOSPC;
 	}
 
 	return rc;
@@ -1487,6 +1499,9 @@ int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 		rc = do_xaction_namespace(NULL, ACTION_CREATE, ctx, &created);
 	}
 
+	if (param.greedy)
+		fprintf(stderr, "created %d namespace%s\n", created,
+			created == 1 ? "" : "s");
 	if (rc < 0 || (!namespace && created < 1)) {
 		fprintf(stderr, "failed to %s namespace: %s\n", namespace
 				? "reconfigure" : "create", strerror(-rc));
-- 
2.20.1

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

Re: [ndctl PATCH 3/3] ndctl/namespace: add a --continue option to create namespaces greedily

[Dan Williams]

On Wed, Aug 28, 2019 at 5:17 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Add a --continue option to ndctl-create-namespaces to allow the creation
> of as many namespaces as possible, that meet the given filter
> restrictions.
>
> The creation loop will be aborted if a failure is encountered at any
> point.
>
> Link: https://github.com/pmem/ndctl/issues/106
> Reported-by: Steve Scargal <steve.scargall@intel.com>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  .../ndctl/ndctl-create-namespace.txt          |  7 ++++++
>  ndctl/namespace.c                             | 25 +++++++++++++++----
>  2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ndctl/ndctl-create-namespace.txt b/Documentation/ndctl/ndctl-create-namespace.txt
> index c9ae27c..55a8581 100644
> --- a/Documentation/ndctl/ndctl-create-namespace.txt
> +++ b/Documentation/ndctl/ndctl-create-namespace.txt
> @@ -215,6 +215,13 @@ include::xable-region-options.txt[]
>  --bus=::
>  include::xable-bus-options.txt[]
>
> +-c::
> +--continue::
> +       Do not stop after creating one namespace. Instead, greedily create as

I like the "greedy" terminology here because it makes the option seem
a bit off-putting. "Do you really want to be greedy?"

> +       many namespaces as possible within the given --bus and --region filter
> +       restrictions. This will abort if any creation attempt results in an
> +       error.

Hmm, should "--continue --force" override that policy?

Otherwise this looks good to me.

> +
>  include::../copyright.txt[]
>
>  SEE ALSO
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index af20a42..8d6b249 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -41,6 +41,7 @@ static struct parameters {
>         bool do_scan;
>         bool mode_default;
>         bool autolabel;
> +       bool greedy;
>         const char *bus;
>         const char *map;
>         const char *type;
> @@ -114,7 +115,9 @@ OPT_STRING('t', "type", &param.type, "type", \
>  OPT_STRING('a', "align", &param.align, "align", \
>         "specify the namespace alignment in bytes (default: 2M)"), \
>  OPT_BOOLEAN('f', "force", &force, "reconfigure namespace even if currently active"), \
> -OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels")
> +OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels"), \
> +OPT_BOOLEAN('c', "continue", &param.greedy, \
> +       "continue creating namespaces as long as the filter criteria are met")
>
>  #define CHECK_OPTIONS() \
>  OPT_BOOLEAN('R', "repair", &repair, "perform metadata repairs"), \
> @@ -1365,8 +1368,11 @@ static int do_xaction_namespace(const char *namespace,
>                                 rc = namespace_create(region);
>                                 if (rc == -EAGAIN)
>                                         continue;
> -                               if (rc == 0)
> -                                       *processed = 1;
> +                               if (rc == 0) {
> +                                       (*processed)++;
> +                                       if (param.greedy)
> +                                               continue;
> +                               }
>                                 return rc;
>                         }
>                         ndctl_namespace_foreach_safe(region, ndns, _n) {
> @@ -1427,9 +1433,15 @@ static int do_xaction_namespace(const char *namespace,
>                 /*
>                  * Namespace creation searched through all candidate
>                  * regions and all of them said "nope, I don't have
> -                * enough capacity", so report -ENOSPC
> +                * enough capacity", so report -ENOSPC. Except during
> +                * greedy namespace creation using --continue as we
> +                * may have created some namespaces already, and the
> +                * last one in the region search may preexist.
>                  */
> -               rc = -ENOSPC;
> +               if (param.greedy && (*processed) > 0)
> +                       rc = 0;
> +               else
> +                       rc = -ENOSPC;
>         }
>
>         return rc;
> @@ -1487,6 +1499,9 @@ int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
>                 rc = do_xaction_namespace(NULL, ACTION_CREATE, ctx, &created);
>         }
>
> +       if (param.greedy)
> +               fprintf(stderr, "created %d namespace%s\n", created,
> +                       created == 1 ? "" : "s");
>         if (rc < 0 || (!namespace && created < 1)) {
>                 fprintf(stderr, "failed to %s namespace: %s\n", namespace
>                                 ? "reconfigure" : "create", strerror(-rc));
> --
> 2.20.1
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/3] ndctl/namespace: add a --continue option to create namespaces greedily

[jane.chu]

Hi, Vishal,


On 8/28/19 5:17 PM, Vishal Verma wrote:
> Add a --continue option to ndctl-create-namespaces to allow the creation
> of as many namespaces as possible, that meet the given filter
> restrictions.
> 
> The creation loop will be aborted if a failure is encountered at any
> point.

Just wondering what is the motivation behind providing this option?

thanks!
-jane

> 
> Link: https://github.com/pmem/ndctl/issues/106
> Reported-by: Steve Scargal <steve.scargall@intel.com>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>   .../ndctl/ndctl-create-namespace.txt          |  7 ++++++
>   ndctl/namespace.c                             | 25 +++++++++++++++----
>   2 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/ndctl/ndctl-create-namespace.txt b/Documentation/ndctl/ndctl-create-namespace.txt
> index c9ae27c..55a8581 100644
> --- a/Documentation/ndctl/ndctl-create-namespace.txt
> +++ b/Documentation/ndctl/ndctl-create-namespace.txt
> @@ -215,6 +215,13 @@ include::xable-region-options.txt[]
>   --bus=::
>   include::xable-bus-options.txt[]
>   
> +-c::
> +--continue::
> +	Do not stop after creating one namespace. Instead, greedily create as
> +	many namespaces as possible within the given --bus and --region filter
> +	restrictions. This will abort if any creation attempt results in an
> +	error.
> +
>   include::../copyright.txt[]
>   
>   SEE ALSO
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index af20a42..8d6b249 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -41,6 +41,7 @@ static struct parameters {
>   	bool do_scan;
>   	bool mode_default;
>   	bool autolabel;
> +	bool greedy;
>   	const char *bus;
>   	const char *map;
>   	const char *type;
> @@ -114,7 +115,9 @@ OPT_STRING('t', "type", &param.type, "type", \
>   OPT_STRING('a', "align", &param.align, "align", \
>   	"specify the namespace alignment in bytes (default: 2M)"), \
>   OPT_BOOLEAN('f', "force", &force, "reconfigure namespace even if currently active"), \
> -OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels")
> +OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels"), \
> +OPT_BOOLEAN('c', "continue", &param.greedy, \
> +	"continue creating namespaces as long as the filter criteria are met")
>   
>   #define CHECK_OPTIONS() \
>   OPT_BOOLEAN('R', "repair", &repair, "perform metadata repairs"), \
> @@ -1365,8 +1368,11 @@ static int do_xaction_namespace(const char *namespace,
>   				rc = namespace_create(region);
>   				if (rc == -EAGAIN)
>   					continue;
> -				if (rc == 0)
> -					*processed = 1;
> +				if (rc == 0) {
> +					(*processed)++;
> +					if (param.greedy)
> +						continue;
> +				}
>   				return rc;
>   			}
>   			ndctl_namespace_foreach_safe(region, ndns, _n) {
> @@ -1427,9 +1433,15 @@ static int do_xaction_namespace(const char *namespace,
>   		/*
>   		 * Namespace creation searched through all candidate
>   		 * regions and all of them said "nope, I don't have
> -		 * enough capacity", so report -ENOSPC
> +		 * enough capacity", so report -ENOSPC. Except during
> +		 * greedy namespace creation using --continue as we
> +		 * may have created some namespaces already, and the
> +		 * last one in the region search may preexist.
>   		 */
> -		rc = -ENOSPC;
> +		if (param.greedy && (*processed) > 0)
> +			rc = 0;
> +		else
> +			rc = -ENOSPC;
>   	}
>   
>   	return rc;
> @@ -1487,6 +1499,9 @@ int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
>   		rc = do_xaction_namespace(NULL, ACTION_CREATE, ctx, &created);
>   	}
>   
> +	if (param.greedy)
> +		fprintf(stderr, "created %d namespace%s\n", created,
> +			created == 1 ? "" : "s");
>   	if (rc < 0 || (!namespace && created < 1)) {
>   		fprintf(stderr, "failed to %s namespace: %s\n", namespace
>   				? "reconfigure" : "create", strerror(-rc));
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/3] ndctl/namespace: add a --continue option to create namespaces greedily

[Verma, Vishal L]

On Wed, 2019-08-28 at 19:34 -0700, Dan Williams wrote:
> On Wed, Aug 28, 2019 at 5:17 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> > Add a --continue option to ndctl-create-namespaces to allow the creation
> > of as many namespaces as possible, that meet the given filter
> > restrictions.
> > 
> > The creation loop will be aborted if a failure is encountered at any
> > point.
> > 
> > Link: https://github.com/pmem/ndctl/issues/106
> > Reported-by: Steve Scargal <steve.scargall@intel.com>
> > Cc: Jeff Moyer <jmoyer@redhat.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> >  .../ndctl/ndctl-create-namespace.txt          |  7 ++++++
> >  ndctl/namespace.c                             | 25 +++++++++++++++----
> >  2 files changed, 27 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/ndctl/ndctl-create-namespace.txt b/Documentation/ndctl/ndctl-create-namespace.txt
> > index c9ae27c..55a8581 100644
> > --- a/Documentation/ndctl/ndctl-create-namespace.txt
> > +++ b/Documentation/ndctl/ndctl-create-namespace.txt
> > @@ -215,6 +215,13 @@ include::xable-region-options.txt[]
> >  --bus=::
> >  include::xable-bus-options.txt[]
> > 
> > +-c::
> > +--continue::
> > +       Do not stop after creating one namespace. Instead, greedily create as
> 
> I like the "greedy" terminology here because it makes the option seem
> a bit off-putting. "Do you really want to be greedy?"

Ha, I just borrowed it from regular expressions :)

> 
> > +       many namespaces as possible within the given --bus and --region filter
> > +       restrictions. This will abort if any creation attempt results in an
> > +       error.
> 
> Hmm, should "--continue --force" override that policy?

Yep that's a good idea, with a big note in the man page that errors
could be lost/meaningless in that case.

> 
> Otherwise this looks good to me.

Thanks, I'll send a new version with the above.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/3] ndctl/namespace: add a --continue option to create namespaces greedily

[Verma, Vishal L]

On Thu, 2019-08-29 at 10:38 -0700, jane.chu@oracle.com wrote:
> Hi, Vishal,
> 
> 
> On 8/28/19 5:17 PM, Vishal Verma wrote:
> > Add a --continue option to ndctl-create-namespaces to allow the creation
> > of as many namespaces as possible, that meet the given filter
> > restrictions.
> > 
> > The creation loop will be aborted if a failure is encountered at any
> > point.
> 
> Just wondering what is the motivation behind providing this option?

Hi Jane,

See Steve's email here:
https://lists.01.org/pipermail/linux-nvdimm/2019-August/023390.html

Essentially it lets sysadmins create a simple, maximal configuration
without everyone having to script it.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/3] ndctl/namespace: add a --continue option to create namespaces greedily

[Dan Williams]

On Thu, Aug 29, 2019 at 11:08 AM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Thu, 2019-08-29 at 10:38 -0700, jane.chu@oracle.com wrote:
> > Hi, Vishal,
> >
> >
> > On 8/28/19 5:17 PM, Vishal Verma wrote:
> > > Add a --continue option to ndctl-create-namespaces to allow the creation
> > > of as many namespaces as possible, that meet the given filter
> > > restrictions.
> > >
> > > The creation loop will be aborted if a failure is encountered at any
> > > point.
> >
> > Just wondering what is the motivation behind providing this option?
>
> Hi Jane,
>
> See Steve's email here:
> https://lists.01.org/pipermail/linux-nvdimm/2019-August/023390.html
>
> Essentially it lets sysadmins create a simple, maximal configuration
> without everyone having to script it.

It also gives a touch point to start thinking about parallel namespace
creation. The large advancements in boot time that Alex achieved were
mainly from parallelizing namespace init. With --continue we could
batch the namespace enable calls and kick off a bind thread per
namespace.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/3] ndctl/namespace: add a --continue option to create namespaces greedily

[Vishal Verma]

On 08/29, Verma, Vishal L wrote:
> On Wed, 2019-08-28 at 19:34 -0700, Dan Williams wrote:
> > 
> > Hmm, should "--continue --force" override that policy?
> 
> Yep that's a good idea, with a big note in the man page that errors
> could be lost/meaningless in that case.
> 
> > 
> > Otherwise this looks good to me.
> 
> Thanks, I'll send a new version with the above.
> _______________________________________________

Here is v2 with the --force change:

3<----


>From a91425beabae750227931594f77fe3db72b4cfff Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.verma@intel.com>
Date: Wed, 28 Aug 2019 18:01:38 -0600
Subject: [ndctl PATCH v2] ndctl/namespace: add a --continue option to create
 namespaces greedily

Add a --continue option to ndctl-create-namespaces to allow the creation
of as many namespaces as possible, that meet the given filter
restrictions.

The creation loop will be aborted if a failure is encountered at any
point, unless --force is also specified.

Link: https://github.com/pmem/ndctl/issues/106
Reported-by: Steve Scargal <steve.scargall@intel.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---

v2: Allow --force to continue in spite of errors (Dan)

 .../ndctl/ndctl-create-namespace.txt          | 11 +++++-
 ndctl/namespace.c                             | 34 +++++++++++++++----
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/Documentation/ndctl/ndctl-create-namespace.txt b/Documentation/ndctl/ndctl-create-namespace.txt
index c9ae27c..e29a5e7 100644
--- a/Documentation/ndctl/ndctl-create-namespace.txt
+++ b/Documentation/ndctl/ndctl-create-namespace.txt
@@ -152,6 +152,13 @@ OPTIONS
 	namespace directly ("--map=dev"). The overhead is 64-bytes per
 	4K (16GB per 1TB) on x86.
 
+-c::
+--continue::
+	Do not stop after creating one namespace. Instead, greedily create as
+	many namespaces as possible within the given --bus and --region filter
+	restrictions. This will abort if any creation attempt results in an
+	error unless --force is also supplied.
+
 -f::
 --force::
 	Unless this option is specified the 'reconfigure namespace'
@@ -160,7 +167,9 @@ OPTIONS
 	the operation is attempted. However, if the namespace is
 	mounted then the 'disable namespace' and 'reconfigure
 	namespace' operations will be aborted.  The namespace must be
-	unmounted before being reconfigured.
+	unmounted before being reconfigured. When used in conjunction
+	with --continue, continue the namespace creation loop even
+	if an error is encountered for intermediate namespaces.
 
 -L::
 --autolabel::
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index af20a42..67768f3 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -41,6 +41,7 @@ static struct parameters {
 	bool do_scan;
 	bool mode_default;
 	bool autolabel;
+	bool greedy;
 	const char *bus;
 	const char *map;
 	const char *type;
@@ -114,7 +115,9 @@ OPT_STRING('t', "type", &param.type, "type", \
 OPT_STRING('a', "align", &param.align, "align", \
 	"specify the namespace alignment in bytes (default: 2M)"), \
 OPT_BOOLEAN('f', "force", &force, "reconfigure namespace even if currently active"), \
-OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels")
+OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels"), \
+OPT_BOOLEAN('c', "continue", &param.greedy, \
+	"continue creating namespaces as long as the filter criteria are met")
 
 #define CHECK_OPTIONS() \
 OPT_BOOLEAN('R', "repair", &repair, "perform metadata repairs"), \
@@ -1322,10 +1325,10 @@ static int do_xaction_namespace(const char *namespace,
 		int *processed)
 {
 	struct ndctl_namespace *ndns, *_n;
+	int rc = -ENXIO, saved_rc = 0;
 	struct ndctl_region *region;
 	const char *ndns_name;
 	struct ndctl_bus *bus;
-	int rc = -ENXIO;
 
 	*processed = 0;
 
@@ -1365,8 +1368,16 @@ static int do_xaction_namespace(const char *namespace,
 				rc = namespace_create(region);
 				if (rc == -EAGAIN)
 					continue;
-				if (rc == 0)
-					*processed = 1;
+				if (rc == 0) {
+					(*processed)++;
+					if (param.greedy)
+						continue;
+				}
+				if (force) {
+					if (rc)
+						saved_rc = rc;
+					continue;
+				}
 				return rc;
 			}
 			ndctl_namespace_foreach_safe(region, ndns, _n) {
@@ -1427,10 +1438,18 @@ static int do_xaction_namespace(const char *namespace,
 		/*
 		 * Namespace creation searched through all candidate
 		 * regions and all of them said "nope, I don't have
-		 * enough capacity", so report -ENOSPC
+		 * enough capacity", so report -ENOSPC. Except during
+		 * greedy namespace creation using --continue as we
+		 * may have created some namespaces already, and the
+		 * last one in the region search may preexist.
 		 */
-		rc = -ENOSPC;
+		if (param.greedy && (*processed) > 0)
+			rc = 0;
+		else
+			rc = -ENOSPC;
 	}
+	if (saved_rc)
+		rc = saved_rc;
 
 	return rc;
 }
@@ -1487,6 +1506,9 @@ int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 		rc = do_xaction_namespace(NULL, ACTION_CREATE, ctx, &created);
 	}
 
+	if (param.greedy)
+		fprintf(stderr, "created %d namespace%s\n", created,
+			created == 1 ? "" : "s");
 	if (rc < 0 || (!namespace && created < 1)) {
 		fprintf(stderr, "failed to %s namespace: %s\n", namespace
 				? "reconfigure" : "create", strerror(-rc));
-- 
2.20.1

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

Re: [ndctl PATCH 3/3] ndctl/namespace: add a --continue option to create namespaces greedily

[jane.chu]

On 8/29/19 11:28 AM, Dan Williams wrote:
> On Thu, Aug 29, 2019 at 11:08 AM Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
>>
>> On Thu, 2019-08-29 at 10:38 -0700, jane.chu@oracle.com wrote:
>>> Hi, Vishal,
>>>
>>>
>>> On 8/28/19 5:17 PM, Vishal Verma wrote:
>>>> Add a --continue option to ndctl-create-namespaces to allow the creation
>>>> of as many namespaces as possible, that meet the given filter
>>>> restrictions.
>>>>
>>>> The creation loop will be aborted if a failure is encountered at any
>>>> point.
>>>
>>> Just wondering what is the motivation behind providing this option?
>>
>> Hi Jane,
>>
>> See Steve's email here:
>> https://lists.01.org/pipermail/linux-nvdimm/2019-August/023390.html
>>
>> Essentially it lets sysadmins create a simple, maximal configuration
>> without everyone having to script it.
> 
> It also gives a touch point to start thinking about parallel namespace
> creation. The large advancements in boot time that Alex achieved were
> mainly from parallelizing namespace init. With --continue we could
> batch the namespace enable calls and kick off a bind thread per
> namespace.
> 

Thanks Dan!  Sorry I was reading email backwards, just caught up with 
the earlier discussions.

With the --continue option, assuming the --size option will be taken if 
specified, it would be possible to end up with a large number of small 
namespaces
with a simple command that runs for a long while, right? can it be 
killed by ctrl-c once the innocent user regrets? :)

thanks,
-jane

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