| Message ID | 20190829001735.30289-4-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | greedy namespace creation | expand |
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", ¶m.type, "type", \ > OPT_STRING('a', "align", ¶m.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", ¶m.autolabel, "automatically initialize labels") > +OPT_BOOLEAN('L', "autolabel", ¶m.autolabel, "automatically initialize labels"), \ > +OPT_BOOLEAN('c', "continue", ¶m.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 >
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", ¶m.type, "type", \ > OPT_STRING('a', "align", ¶m.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", ¶m.autolabel, "automatically initialize labels") > +OPT_BOOLEAN('L', "autolabel", ¶m.autolabel, "automatically initialize labels"), \ > +OPT_BOOLEAN('c', "continue", ¶m.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)); >
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.
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.
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.
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", ¶m.type, "type", \ OPT_STRING('a', "align", ¶m.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", ¶m.autolabel, "automatically initialize labels") +OPT_BOOLEAN('L', "autolabel", ¶m.autolabel, "automatically initialize labels"), \ +OPT_BOOLEAN('c', "continue", ¶m.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));
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
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", ¶m.type, "type", \ OPT_STRING('a', "align", ¶m.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", ¶m.autolabel, "automatically initialize labels") +OPT_BOOLEAN('L', "autolabel", ¶m.autolabel, "automatically initialize labels"), \ +OPT_BOOLEAN('c', "continue", ¶m.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));
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(-)