* [PATCH v5 1/4] ndctl: convert namespace actions to use util_filter_params
2018-04-27 22:08 [PATCH v5 0/4] ndctl: convert actions to use util_filter_walk Dave Jiang
@ 2018-04-27 22:08 ` Dave Jiang
2018-04-27 22:08 ` [PATCH v5 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Dave Jiang @ 2018-04-27 22:08 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
In preparation of moving to using util_filter_walk, moving parts of
namespace params to util_filter_params.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
ndctl/namespace.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index fe86d826..e61f500c 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -40,14 +40,11 @@ static struct parameters {
bool mode_default;
bool align_default;
bool autolabel;
- const char *bus;
const char *map;
- const char *type;
const char *uuid;
const char *name;
const char *size;
const char *mode;
- const char *region;
const char *reconfig;
const char *sector_size;
const char *align;
@@ -55,6 +52,8 @@ static struct parameters {
.autolabel = true,
};
+struct util_filter_params uf_param;
+
void builtin_xaction_namespace_reset(void)
{
/*
@@ -87,9 +86,9 @@ struct parsed_parameters {
}})
#define BASE_OPTIONS() \
-OPT_STRING('b', "bus", ¶m.bus, "bus-id", \
+OPT_STRING('b', "bus", &uf_param.bus, "bus-id", \
"limit namespace to a bus with an id or provider of <bus-id>"), \
-OPT_STRING('r', "region", ¶m.region, "region-id", \
+OPT_STRING('r', "region", &uf_param.region, "region-id", \
"limit namespace to a region with an id or name of <region-id>"), \
OPT_BOOLEAN('v', "verbose", &verbose, "emit extra debug messages to stderr")
@@ -108,7 +107,7 @@ OPT_STRING('M', "map", ¶m.map, "memmap-location", \
"specify 'mem' or 'dev' for the location of the memmap"), \
OPT_STRING('l', "sector-size", ¶m.sector_size, "lba-size", \
"specify the logical sector size in bytes"), \
-OPT_STRING('t', "type", ¶m.type, "type", \
+OPT_STRING('t', "type", &uf_param.type, "type", \
"specify the type of namespace to create 'pmem' or 'blk'"), \
OPT_STRING('a', "align", ¶m.align, "align", \
"specify the namespace alignment in bytes (default: 2M)"), \
@@ -148,18 +147,18 @@ static int set_defaults(enum device_action mode)
{
int rc = 0;
- if (param.type) {
- if (strcmp(param.type, "pmem") == 0)
+ if (uf_param.type) {
+ if (strcmp(uf_param.type, "pmem") == 0)
/* pass */;
- else if (strcmp(param.type, "blk") == 0)
+ else if (strcmp(uf_param.type, "blk") == 0)
/* pass */;
else {
error("invalid type '%s', must be either 'pmem' or 'blk'\n",
- param.type);
+ uf_param.type);
rc = -EINVAL;
}
} else if (!param.reconfig && mode == ACTION_CREATE)
- param.type = "pmem";
+ uf_param.type = "pmem";
if (param.mode) {
if (strcmp(param.mode, "safe") == 0)
@@ -180,8 +179,8 @@ static int set_defaults(enum device_action mode)
error("invalid mode '%s'\n", param.mode);
rc = -EINVAL;
}
- } else if (!param.reconfig && param.type) {
- if (strcmp(param.type, "pmem") == 0)
+ } else if (!param.reconfig && uf_param.type) {
+ if (strcmp(uf_param.type, "pmem") == 0)
param.mode = "memory";
else
param.mode = "safe";
@@ -208,7 +207,7 @@ static int set_defaults(enum device_action mode)
param.map = "dev";
/* check for incompatible mode and type combinations */
- if (param.type && param.mode && strcmp(param.type, "blk") == 0
+ if (uf_param.type && param.mode && strcmp(uf_param.type, "blk") == 0
&& (strcmp(param.mode, "memory") == 0
|| strcmp(param.mode, "dax") == 0)) {
error("only 'pmem' namespaces support dax operation\n");
@@ -244,7 +243,7 @@ static int set_defaults(enum device_action mode)
error("invalid sector size: %s\n", param.sector_size);
rc = -EINVAL;
}
- } else if (((param.type && strcmp(param.type, "blk") == 0)
+ } else if (((uf_param.type && strcmp(uf_param.type, "blk") == 0)
|| (param.mode && strcmp(param.mode, "safe") == 0))) {
/* default sector size for blk-type or safe-mode */
param.sector_size = "4096";
@@ -1000,19 +999,19 @@ static int do_xaction_namespace(const char *namespace,
ndctl_set_log_priority(ctx, LOG_DEBUG);
ndctl_bus_foreach(ctx, bus) {
- if (!util_bus_filter(bus, param.bus))
+ if (!util_bus_filter(bus, uf_param.bus))
continue;
ndctl_region_foreach(bus, region) {
- if (!util_region_filter(region, param.region))
+ if (!util_region_filter(region, uf_param.region))
continue;
- if (param.type) {
- if (strcmp(param.type, "pmem") == 0
+ if (uf_param.type) {
+ if (strcmp(uf_param.type, "pmem") == 0
&& ndctl_region_get_type(region)
== ND_DEVICE_REGION_PMEM)
/* pass */;
- else if (strcmp(param.type, "blk") == 0
+ else if (strcmp(uf_param.type, "blk") == 0
&& ndctl_region_get_type(region)
== ND_DEVICE_REGION_BLK)
/* pass */;
@@ -1125,7 +1124,7 @@ int cmd_create_namespace(int argc, const char **argv, void *ctx)
* fallback to blk before giving up.
*/
memset(¶m, 0, sizeof(param));
- param.type = "blk";
+ uf_param.type = "blk";
set_defaults(ACTION_CREATE);
created = do_xaction_namespace(NULL, ACTION_CREATE, ctx);
}
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 2/4] ndctl: convert namespace actions to use util_filter_walk()
2018-04-27 22:08 [PATCH v5 0/4] ndctl: convert actions to use util_filter_walk Dave Jiang
2018-04-27 22:08 ` [PATCH v5 1/4] ndctl: convert namespace actions to use util_filter_params Dave Jiang
@ 2018-04-27 22:08 ` Dave Jiang
2018-05-09 22:47 ` Verma, Vishal L
2018-04-27 22:08 ` [PATCH v5 3/4] ndctl: convert region " Dave Jiang
2018-04-27 22:08 ` [PATCH v5 4/4] ndctl: convert dimm " Dave Jiang
3 siblings, 1 reply; 14+ messages in thread
From: Dave Jiang @ 2018-04-27 22:08 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
util_filter_walk() does the looping through of busses and regions. Removing
duplicate code in namespace ops and provide filter functions so we can
utilize util_filter_walk() and share common code.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
ndctl/namespace.c | 163 ++++++++++++++++++++++++++++++-----------------------
test/btt-check.sh | 2 -
util/filter.c | 5 +-
util/filter.h | 8 +++
4 files changed, 104 insertions(+), 74 deletions(-)
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index e61f500c..b0505ce8 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -983,14 +983,91 @@ static int namespace_reconfig(struct ndctl_region *region,
int namespace_check(struct ndctl_namespace *ndns, bool verbose, bool force,
bool repair, bool logfix);
+static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
+{
+ return true;
+}
+
+static bool filter_region(struct ndctl_region *region,
+ struct util_filter_ctx *ctx)
+{
+ struct nsaction_filter_arg *nfa = ctx->nsaction;
+ int rc = 0;
+ bool out = true;
+
+ if (nfa->action == ACTION_CREATE && !nfa->namespace) {
+ if (nfa->rc == 1)
+ return false;
+
+ rc = namespace_create(region);
+ if (rc != -EAGAIN) {
+ if (rc == 0)
+ rc = 1;
+ /* don't proceed in the filter loop */
+ out = false;
+ }
+ nfa->rc = rc;
+ }
+
+ return out;
+}
+
+static void filter_namespace(struct ndctl_namespace *ndns,
+ struct util_filter_ctx *ctx)
+{
+ struct ndctl_region *region = ndctl_namespace_get_region(ndns);
+ struct nsaction_filter_arg *nfa = ctx->nsaction;
+ const char *ndns_name;
+ int rc;
+
+ if (!nfa->namespace)
+ return;
+
+ ndns_name = ndctl_namespace_get_devname(ndns);
+ if (strcmp(nfa->namespace, "all") != 0
+ && strcmp(nfa->namespace, ndns_name) != 0) {
+ return;
+ }
+
+ switch (nfa->action) {
+ case ACTION_DISABLE:
+ rc = ndctl_namespace_disable_safe(ndns);
+ break;
+ case ACTION_ENABLE:
+ rc = ndctl_namespace_enable(ndns);
+ break;
+ case ACTION_DESTROY:
+ rc = namespace_destroy(region, ndns);
+ break;
+ case ACTION_CHECK:
+ rc = namespace_check(ndns, verbose, force, repair, logfix);
+ if (rc < 0) {
+ nfa->rc = rc;
+ return;
+ }
+ break;
+ case ACTION_CREATE:
+ rc = namespace_reconfig(region, ndns);
+ if (rc < 0) {
+ nfa->rc = rc;
+ return;
+ }
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ if (rc >= 0)
+ nfa->rc++;
+}
+
static int do_xaction_namespace(const char *namespace,
enum device_action action, struct ndctl_ctx *ctx)
{
- struct ndctl_namespace *ndns, *_n;
- int rc = -ENXIO, success = 0;
- struct ndctl_region *region;
- const char *ndns_name;
- struct ndctl_bus *bus;
+ int rc = -ENXIO;
+ struct util_filter_ctx fctx = { 0 };
+ struct nsaction_filter_arg nfa = { 0 };
if (!namespace && action != ACTION_CREATE)
return rc;
@@ -998,74 +1075,18 @@ static int do_xaction_namespace(const char *namespace,
if (verbose)
ndctl_set_log_priority(ctx, LOG_DEBUG);
- ndctl_bus_foreach(ctx, bus) {
- if (!util_bus_filter(bus, uf_param.bus))
- continue;
-
- ndctl_region_foreach(bus, region) {
- if (!util_region_filter(region, uf_param.region))
- continue;
-
- if (uf_param.type) {
- if (strcmp(uf_param.type, "pmem") == 0
- && ndctl_region_get_type(region)
- == ND_DEVICE_REGION_PMEM)
- /* pass */;
- else if (strcmp(uf_param.type, "blk") == 0
- && ndctl_region_get_type(region)
- == ND_DEVICE_REGION_BLK)
- /* pass */;
- else
- continue;
- }
+ fctx.filter_bus = filter_bus;
+ fctx.filter_region = filter_region;
+ fctx.filter_namespace = filter_namespace;
+ fctx.nsaction = &nfa;
+ fctx.nsaction->action = action;
+ fctx.nsaction->namespace = namespace;
- if (action == ACTION_CREATE && !namespace) {
- rc = namespace_create(region);
- if (rc == -EAGAIN)
- continue;
- if (rc == 0)
- rc = 1;
- return rc;
- }
- ndctl_namespace_foreach_safe(region, ndns, _n) {
- ndns_name = ndctl_namespace_get_devname(ndns);
-
- if (strcmp(namespace, "all") != 0
- && strcmp(namespace, ndns_name) != 0)
- continue;
- switch (action) {
- case ACTION_DISABLE:
- rc = ndctl_namespace_disable_safe(ndns);
- break;
- case ACTION_ENABLE:
- rc = ndctl_namespace_enable(ndns);
- break;
- case ACTION_DESTROY:
- rc = namespace_destroy(region, ndns);
- break;
- case ACTION_CHECK:
- rc = namespace_check(ndns, verbose,
- force, repair, logfix);
- if (rc < 0)
- return rc;
- break;
- case ACTION_CREATE:
- rc = namespace_reconfig(region, ndns);
- if (rc < 0)
- return rc;
- return 1;
- default:
- rc = -EINVAL;
- break;
- }
- if (rc >= 0)
- success++;
- }
- }
- }
+ rc = util_filter_walk(ctx, &fctx, &uf_param);
+ if (rc < 0)
+ return rc;
- if (success)
- return success;
+ rc = nfa.rc;
return rc;
}
diff --git a/test/btt-check.sh b/test/btt-check.sh
index 353d4375..50c1b592 100755
--- a/test/btt-check.sh
+++ b/test/btt-check.sh
@@ -1,4 +1,4 @@
-#!/bin/bash -E
+#!/bin/bash -Ex
# Copyright(c) 2015-2017 Intel Corporation. All rights reserved.
#
diff --git a/util/filter.c b/util/filter.c
index 0d3cc02f..b7a4fbd6 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -18,6 +18,7 @@
#include <sys/stat.h>
#include <util/util.h>
#include <sys/types.h>
+#include <ndctl/action.h>
#include <ndctl/ndctl.h>
#include <util/filter.h>
#include <ndctl/libndctl.h>
@@ -418,7 +419,7 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
}
ndctl_region_foreach(bus, region) {
- struct ndctl_namespace *ndns;
+ struct ndctl_namespace *ndns, *_n;
if (!util_region_filter(region, param->region)
|| !util_region_filter_by_dimm(region,
@@ -437,7 +438,7 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
if (!fctx->filter_region(region, fctx))
continue;
- ndctl_namespace_foreach(region, ndns) {
+ ndctl_namespace_foreach_safe(region, ndns, _n) {
enum ndctl_namespace_mode mode;
if (!fctx->filter_namespace)
diff --git a/util/filter.h b/util/filter.h
index effda24b..9f3786a9 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -13,6 +13,7 @@
#ifndef _UTIL_FILTER_H_
#define _UTIL_FILTER_H_
#include <stdbool.h>
+#include <ndctl/action.h>
struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char *ident);
struct ndctl_region *util_region_filter(struct ndctl_region *region,
@@ -50,6 +51,12 @@ struct list_filter_arg {
unsigned long flags;
};
+struct nsaction_filter_arg {
+ enum device_action action;
+ const char *namespace;
+ int rc;
+};
+
/*
* struct util_filter_ctx - control and callbacks for util_filter_walk()
* ->filter_bus() and ->filter_region() return bool because the
@@ -67,6 +74,7 @@ struct util_filter_ctx {
union {
void *arg;
struct list_filter_arg *list;
+ struct nsaction_filter_arg *nsaction;
};
};
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v5 2/4] ndctl: convert namespace actions to use util_filter_walk()
2018-04-27 22:08 ` [PATCH v5 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
@ 2018-05-09 22:47 ` Verma, Vishal L
2018-05-09 22:49 ` Dave Jiang
0 siblings, 1 reply; 14+ messages in thread
From: Verma, Vishal L @ 2018-05-09 22:47 UTC (permalink / raw)
To: Williams, Dan J, Jiang, Dave; +Cc: linux-nvdimm
On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
> util_filter_walk() does the looping through of busses and regions.
> Removing
> duplicate code in namespace ops and provide filter functions so we can
> utilize util_filter_walk() and share common code.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> ndctl/namespace.c | 163 ++++++++++++++++++++++++++++++-----------------
> ------
> test/btt-check.sh | 2 -
> util/filter.c | 5 +-
> util/filter.h | 8 +++
> 4 files changed, 104 insertions(+), 74 deletions(-)
>
[...]
>
> diff --git a/test/btt-check.sh b/test/btt-check.sh
> index 353d4375..50c1b592 100755
> --- a/test/btt-check.sh
> +++ b/test/btt-check.sh
> @@ -1,4 +1,4 @@
> -#!/bin/bash -E
> +#!/bin/bash -Ex
Did you mean to add this hunk?
>
> # Copyright(c) 2015-2017 Intel Corporation. All rights reserved.
> #
> diff --git a/util/filter.c b/util/filter.c
> index 0d3cc02f..b7a4fbd6 100644
> --- a/util/filter.c
> +++ b/util/filter.c
>
[...]
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 2/4] ndctl: convert namespace actions to use util_filter_walk()
2018-05-09 22:47 ` Verma, Vishal L
@ 2018-05-09 22:49 ` Dave Jiang
0 siblings, 0 replies; 14+ messages in thread
From: Dave Jiang @ 2018-05-09 22:49 UTC (permalink / raw)
To: Verma, Vishal L, Williams, Dan J; +Cc: linux-nvdimm
On 05/09/2018 03:47 PM, Verma, Vishal L wrote:
> On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
>> util_filter_walk() does the looping through of busses and regions.
>> Removing
>> duplicate code in namespace ops and provide filter functions so we can
>> utilize util_filter_walk() and share common code.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> ndctl/namespace.c | 163 ++++++++++++++++++++++++++++++-----------------
>> ------
>> test/btt-check.sh | 2 -
>> util/filter.c | 5 +-
>> util/filter.h | 8 +++
>> 4 files changed, 104 insertions(+), 74 deletions(-)
>>
>
> [...]
>
>>
>> diff --git a/test/btt-check.sh b/test/btt-check.sh
>> index 353d4375..50c1b592 100755
>> --- a/test/btt-check.sh
>> +++ b/test/btt-check.sh
>> @@ -1,4 +1,4 @@
>> -#!/bin/bash -E
>> +#!/bin/bash -Ex
>
> Did you mean to add this hunk?
No, must be accident from debug.
>
>>
>> # Copyright(c) 2015-2017 Intel Corporation. All rights reserved.
>> #
>> diff --git a/util/filter.c b/util/filter.c
>> index 0d3cc02f..b7a4fbd6 100644
>> --- a/util/filter.c
>> +++ b/util/filter.c
>>
> [...]
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 3/4] ndctl: convert region actions to use util_filter_walk()
2018-04-27 22:08 [PATCH v5 0/4] ndctl: convert actions to use util_filter_walk Dave Jiang
2018-04-27 22:08 ` [PATCH v5 1/4] ndctl: convert namespace actions to use util_filter_params Dave Jiang
2018-04-27 22:08 ` [PATCH v5 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
@ 2018-04-27 22:08 ` Dave Jiang
2018-05-09 23:17 ` Verma, Vishal L
2018-04-27 22:08 ` [PATCH v5 4/4] ndctl: convert dimm " Dave Jiang
3 siblings, 1 reply; 14+ messages in thread
From: Dave Jiang @ 2018-04-27 22:08 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
util_filter_walk() does the looping through of busses and regions. Removing
duplicate code in region ops and provide filter functions so we can
utilize util_filter_walk() and share common code.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++----------------------
util/filter.h | 6 ++++++
2 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/ndctl/region.c b/ndctl/region.c
index 9fc90808..9fd07af6 100644
--- a/ndctl/region.c
+++ b/ndctl/region.c
@@ -19,10 +19,7 @@
#include <util/parse-options.h>
#include <ndctl/libndctl.h>
-static struct {
- const char *bus;
- const char *type;
-} param;
+struct util_filter_params param;
static const struct option region_options[] = {
OPT_STRING('b', "bus", ¶m.bus, "bus-id",
@@ -92,33 +89,49 @@ static int region_action(struct ndctl_region *region, enum device_action mode)
return 0;
}
+static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
+{
+ return true;
+}
+
+static bool filter_region(struct ndctl_region *region,
+ struct util_filter_ctx *ctx)
+{
+ struct rgaction_filter_arg *rfa = ctx->rgaction;
+ int rc;
+
+ if (rfa->rc < 0)
+ return false;
+
+ rc = region_action(region, rfa->action);
+
+ if (rc == 0)
+ rfa->rc++;
+ else
+ rfa->rc = 0;
+
+ /* we don't need to fall through, can continue the loop */
+ return false;
+}
+
static int do_xable_region(const char *region_arg, enum device_action mode,
struct ndctl_ctx *ctx)
{
- int rc = -ENXIO, success = 0;
- struct ndctl_region *region;
- struct ndctl_bus *bus;
+ int rc = -ENXIO;
+ struct util_filter_ctx fctx = { 0 };
+ struct rgaction_filter_arg rfa = { 0 };
if (!region_arg)
goto out;
- ndctl_bus_foreach(ctx, bus) {
- if (!util_bus_filter(bus, param.bus))
- continue;
-
- ndctl_region_foreach(bus, region) {
- const char *type = ndctl_region_get_type_name(region);
-
- if (param.type && strcmp(param.type, type) != 0)
- continue;
- if (!util_region_filter(region, region_arg))
- continue;
- if (region_action(region, mode) == 0)
- success++;
- }
- }
+ fctx.filter_bus = filter_bus;
+ fctx.filter_region = filter_region;
+ fctx.rgaction = &rfa;
+ fctx.rgaction->action = mode;
+ rc = util_filter_walk(ctx, &fctx, ¶m);
+ if (rc == 0)
+ rc = fctx.rgaction->rc;
- rc = success;
out:
param.bus = NULL;
return rc;
diff --git a/util/filter.h b/util/filter.h
index 9f3786a9..0e0e3bec 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -57,6 +57,11 @@ struct nsaction_filter_arg {
int rc;
};
+struct rgaction_filter_arg {
+ enum device_action action;
+ int rc;
+};
+
/*
* struct util_filter_ctx - control and callbacks for util_filter_walk()
* ->filter_bus() and ->filter_region() return bool because the
@@ -75,6 +80,7 @@ struct util_filter_ctx {
void *arg;
struct list_filter_arg *list;
struct nsaction_filter_arg *nsaction;
+ struct rgaction_filter_arg *rgaction;
};
};
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] ndctl: convert region actions to use util_filter_walk()
2018-04-27 22:08 ` [PATCH v5 3/4] ndctl: convert region " Dave Jiang
@ 2018-05-09 23:17 ` Verma, Vishal L
2018-05-09 23:23 ` Dan Williams
0 siblings, 1 reply; 14+ messages in thread
From: Verma, Vishal L @ 2018-05-09 23:17 UTC (permalink / raw)
To: Williams, Dan J, Jiang, Dave; +Cc: linux-nvdimm
On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
> util_filter_walk() does the looping through of busses and regions.
> Removing
> duplicate code in region ops and provide filter functions so we can
> utilize util_filter_walk() and share common code.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++--------------
> --------
> util/filter.h | 6 ++++++
> 2 files changed, 42 insertions(+), 23 deletions(-)
>
> diff --git a/ndctl/region.c b/ndctl/region.c
> index 9fc90808..9fd07af6 100644
> --- a/ndctl/region.c
> +++ b/ndctl/region.c
> @@ -19,10 +19,7 @@
> #include <util/parse-options.h>
> #include <ndctl/libndctl.h>
>
> -static struct {
> - const char *bus;
> - const char *type;
> -} param;
> +struct util_filter_params param;
>
> static const struct option region_options[] = {
> OPT_STRING('b', "bus", ¶m.bus, "bus-id",
> @@ -92,33 +89,49 @@ static int region_action(struct ndctl_region *region,
> enum device_action mode)
> return 0;
> }
>
> +static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx
> *ctx)
> +{
> + return true;
> +}
> +
Instead of creating these trivial functions everywhere (also applies to
namespaces.c, dimm.c), should we just leave fctx.bus_filter NULL. And fix
util_filter_walk to check for fctx->ptr != NULL any time it calls one of
the functions..
> +static bool filter_region(struct ndctl_region *region,
> + struct util_filter_ctx *ctx)
> +{
> + struct rgaction_filter_arg *rfa = ctx->rgaction;
> + int rc;
> +
> + if (rfa->rc < 0)
> + return false;
> +
> + rc = region_action(region, rfa->action);
> +
> + if (rc == 0)
> + rfa->rc++;
> + else
> + rfa->rc = 0;
> +
> + /* we don't need to fall through, can continue the loop */
> + return false;
> +}
> +
> static int do_xable_region(const char *region_arg, enum device_action
> mode,
> struct ndctl_ctx *ctx)
> {
> - int rc = -ENXIO, success = 0;
> - struct ndctl_region *region;
> - struct ndctl_bus *bus;
> + int rc = -ENXIO;
> + struct util_filter_ctx fctx = { 0 };
> + struct rgaction_filter_arg rfa = { 0 };
>
> if (!region_arg)
> goto out;
>
> - ndctl_bus_foreach(ctx, bus) {
> - if (!util_bus_filter(bus, param.bus))
> - continue;
> -
> - ndctl_region_foreach(bus, region) {
> - const char *type =
> ndctl_region_get_type_name(region);
> -
> - if (param.type && strcmp(param.type, type) != 0)
> - continue;
> - if (!util_region_filter(region, region_arg))
> - continue;
> - if (region_action(region, mode) == 0)
> - success++;
> - }
> - }
> + fctx.filter_bus = filter_bus;
> + fctx.filter_region = filter_region;
> + fctx.rgaction = &rfa;
> + fctx.rgaction->action = mode;
> + rc = util_filter_walk(ctx, &fctx, ¶m);
> + if (rc == 0)
> + rc = fctx.rgaction->rc;
>
> - rc = success;
> out:
> param.bus = NULL;
> return rc;
> diff --git a/util/filter.h b/util/filter.h
> index 9f3786a9..0e0e3bec 100644
> --- a/util/filter.h
> +++ b/util/filter.h
> @@ -57,6 +57,11 @@ struct nsaction_filter_arg {
> int rc;
> };
>
> +struct rgaction_filter_arg {
> + enum device_action action;
> + int rc;
> +};
> +
> /*
> * struct util_filter_ctx - control and callbacks for util_filter_walk()
> * ->filter_bus() and ->filter_region() return bool because the
> @@ -75,6 +80,7 @@ struct util_filter_ctx {
> void *arg;
> struct list_filter_arg *list;
> struct nsaction_filter_arg *nsaction;
> + struct rgaction_filter_arg *rgaction;
> };
> };
>
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] ndctl: convert region actions to use util_filter_walk()
2018-05-09 23:17 ` Verma, Vishal L
@ 2018-05-09 23:23 ` Dan Williams
2018-05-09 23:24 ` Dave Jiang
0 siblings, 1 reply; 14+ messages in thread
From: Dan Williams @ 2018-05-09 23:23 UTC (permalink / raw)
To: Verma, Vishal L; +Cc: linux-nvdimm
On Wed, May 9, 2018 at 4:17 PM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
> On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
>> util_filter_walk() does the looping through of busses and regions.
>> Removing
>> duplicate code in region ops and provide filter functions so we can
>> utilize util_filter_walk() and share common code.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++--------------
>> --------
>> util/filter.h | 6 ++++++
>> 2 files changed, 42 insertions(+), 23 deletions(-)
>>
>> diff --git a/ndctl/region.c b/ndctl/region.c
>> index 9fc90808..9fd07af6 100644
>> --- a/ndctl/region.c
>> +++ b/ndctl/region.c
>> @@ -19,10 +19,7 @@
>> #include <util/parse-options.h>
>> #include <ndctl/libndctl.h>
>>
>> -static struct {
>> - const char *bus;
>> - const char *type;
>> -} param;
>> +struct util_filter_params param;
>>
>> static const struct option region_options[] = {
>> OPT_STRING('b', "bus", ¶m.bus, "bus-id",
>> @@ -92,33 +89,49 @@ static int region_action(struct ndctl_region *region,
>> enum device_action mode)
>> return 0;
>> }
>>
>> +static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx
>> *ctx)
>> +{
>> + return true;
>> +}
>> +
>
> Instead of creating these trivial functions everywhere (also applies to
> namespaces.c, dimm.c), should we just leave fctx.bus_filter NULL. And fix
> util_filter_walk to check for fctx->ptr != NULL any time it calls one of
> the functions..
I think I'd prefer a common nop routine. That way individual are
forced to decide to use the nop or implement something. Leaving it
NULL may just be a programming mistake. I.e. it's harder to get wrong
if it's required.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] ndctl: convert region actions to use util_filter_walk()
2018-05-09 23:23 ` Dan Williams
@ 2018-05-09 23:24 ` Dave Jiang
2018-05-09 23:26 ` Dan Williams
0 siblings, 1 reply; 14+ messages in thread
From: Dave Jiang @ 2018-05-09 23:24 UTC (permalink / raw)
To: Dan Williams, Verma, Vishal L; +Cc: linux-nvdimm
On 05/09/2018 04:23 PM, Dan Williams wrote:
> On Wed, May 9, 2018 at 4:17 PM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
>> On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
>>> util_filter_walk() does the looping through of busses and regions.
>>> Removing
>>> duplicate code in region ops and provide filter functions so we can
>>> utilize util_filter_walk() and share common code.
>>>
>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>> ---
>>> ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++--------------
>>> --------
>>> util/filter.h | 6 ++++++
>>> 2 files changed, 42 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/ndctl/region.c b/ndctl/region.c
>>> index 9fc90808..9fd07af6 100644
>>> --- a/ndctl/region.c
>>> +++ b/ndctl/region.c
>>> @@ -19,10 +19,7 @@
>>> #include <util/parse-options.h>
>>> #include <ndctl/libndctl.h>
>>>
>>> -static struct {
>>> - const char *bus;
>>> - const char *type;
>>> -} param;
>>> +struct util_filter_params param;
>>>
>>> static const struct option region_options[] = {
>>> OPT_STRING('b', "bus", ¶m.bus, "bus-id",
>>> @@ -92,33 +89,49 @@ static int region_action(struct ndctl_region *region,
>>> enum device_action mode)
>>> return 0;
>>> }
>>>
>>> +static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx
>>> *ctx)
>>> +{
>>> + return true;
>>> +}
>>> +
>>
>> Instead of creating these trivial functions everywhere (also applies to
>> namespaces.c, dimm.c), should we just leave fctx.bus_filter NULL. And fix
>> util_filter_walk to check for fctx->ptr != NULL any time it calls one of
>> the functions..
>
> I think I'd prefer a common nop routine. That way individual are
> forced to decide to use the nop or implement something. Leaving it
> NULL may just be a programming mistake. I.e. it's harder to get wrong
> if it's required.
>
I'll add a common nop routine.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] ndctl: convert region actions to use util_filter_walk()
2018-05-09 23:24 ` Dave Jiang
@ 2018-05-09 23:26 ` Dan Williams
2018-05-09 23:41 ` Verma, Vishal L
2018-05-11 19:52 ` Dave Jiang
0 siblings, 2 replies; 14+ messages in thread
From: Dan Williams @ 2018-05-09 23:26 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Wed, May 9, 2018 at 4:24 PM, Dave Jiang <dave.jiang@intel.com> wrote:
>
>
> On 05/09/2018 04:23 PM, Dan Williams wrote:
>> On Wed, May 9, 2018 at 4:17 PM, Verma, Vishal L
>> <vishal.l.verma@intel.com> wrote:
>>> On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
>>>> util_filter_walk() does the looping through of busses and regions.
>>>> Removing
>>>> duplicate code in region ops and provide filter functions so we can
>>>> utilize util_filter_walk() and share common code.
>>>>
>>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>>> ---
>>>> ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++--------------
>>>> --------
>>>> util/filter.h | 6 ++++++
>>>> 2 files changed, 42 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/ndctl/region.c b/ndctl/region.c
>>>> index 9fc90808..9fd07af6 100644
>>>> --- a/ndctl/region.c
>>>> +++ b/ndctl/region.c
>>>> @@ -19,10 +19,7 @@
>>>> #include <util/parse-options.h>
>>>> #include <ndctl/libndctl.h>
>>>>
>>>> -static struct {
>>>> - const char *bus;
>>>> - const char *type;
>>>> -} param;
>>>> +struct util_filter_params param;
>>>>
>>>> static const struct option region_options[] = {
>>>> OPT_STRING('b', "bus", ¶m.bus, "bus-id",
>>>> @@ -92,33 +89,49 @@ static int region_action(struct ndctl_region *region,
>>>> enum device_action mode)
>>>> return 0;
>>>> }
>>>>
>>>> +static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx
>>>> *ctx)
>>>> +{
>>>> + return true;
>>>> +}
>>>> +
>>>
>>> Instead of creating these trivial functions everywhere (also applies to
>>> namespaces.c, dimm.c), should we just leave fctx.bus_filter NULL. And fix
>>> util_filter_walk to check for fctx->ptr != NULL any time it calls one of
>>> the functions..
>>
>> I think I'd prefer a common nop routine. That way individual are
>> forced to decide to use the nop or implement something. Leaving it
>> NULL may just be a programming mistake. I.e. it's harder to get wrong
>> if it's required.
>>
>
> I'll add a common nop routine.
...but wait why would it be a nop in this case. Region action commands
take a --bus= option?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] ndctl: convert region actions to use util_filter_walk()
2018-05-09 23:26 ` Dan Williams
@ 2018-05-09 23:41 ` Verma, Vishal L
2018-05-11 19:52 ` Dave Jiang
1 sibling, 0 replies; 14+ messages in thread
From: Verma, Vishal L @ 2018-05-09 23:41 UTC (permalink / raw)
To: Williams, Dan J, Jiang, Dave; +Cc: linux-nvdimm
On Wed, 2018-05-09 at 16:26 -0700, Dan Williams wrote:
> On Wed, May 9, 2018 at 4:24 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> >
> >
> > On 05/09/2018 04:23 PM, Dan Williams wrote:
> > > On Wed, May 9, 2018 at 4:17 PM, Verma, Vishal L
> > > <vishal.l.verma@intel.com> wrote:
> > > > On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
> > > > > util_filter_walk() does the looping through of busses and
> > > > > regions.
> > > > > Removing
> > > > > duplicate code in region ops and provide filter functions so we
> > > > > can
> > > > > utilize util_filter_walk() and share common code.
> > > > >
> > > > > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > > > > ---
> > > > > ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++------
> > > > > --------
> > > > > --------
> > > > > util/filter.h | 6 ++++++
> > > > > 2 files changed, 42 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/ndctl/region.c b/ndctl/region.c
> > > > > index 9fc90808..9fd07af6 100644
> > > > > --- a/ndctl/region.c
> > > > > +++ b/ndctl/region.c
> > > > > @@ -19,10 +19,7 @@
> > > > > #include <util/parse-options.h>
> > > > > #include <ndctl/libndctl.h>
> > > > >
> > > > > -static struct {
> > > > > - const char *bus;
> > > > > - const char *type;
> > > > > -} param;
> > > > > +struct util_filter_params param;
> > > > >
> > > > > static const struct option region_options[] = {
> > > > > OPT_STRING('b', "bus", ¶m.bus, "bus-id",
> > > > > @@ -92,33 +89,49 @@ static int region_action(struct ndctl_region
> > > > > *region,
> > > > > enum device_action mode)
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static bool filter_bus(struct ndctl_bus *bus, struct
> > > > > util_filter_ctx
> > > > > *ctx)
> > > > > +{
> > > > > + return true;
> > > > > +}
> > > > > +
> > > >
> > > > Instead of creating these trivial functions everywhere (also
> > > > applies to
> > > > namespaces.c, dimm.c), should we just leave fctx.bus_filter NULL.
> > > > And fix
> > > > util_filter_walk to check for fctx->ptr != NULL any time it calls
> > > > one of
> > > > the functions..
> > >
> > > I think I'd prefer a common nop routine. That way individual are
> > > forced to decide to use the nop or implement something. Leaving it
> > > NULL may just be a programming mistake. I.e. it's harder to get wrong
> > > if it's required.
> > >
> >
> > I'll add a common nop routine.
>
> ...but wait why would it be a nop in this case. Region action commands
> take a --bus= option?
Ah yeah, and so do namespace and dimm actions right?
Also, should we then error out in the util function in case a NULL is
encountered? Should be better than just dereferencing it..
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] ndctl: convert region actions to use util_filter_walk()
2018-05-09 23:26 ` Dan Williams
2018-05-09 23:41 ` Verma, Vishal L
@ 2018-05-11 19:52 ` Dave Jiang
2018-05-11 20:17 ` Dan Williams
1 sibling, 1 reply; 14+ messages in thread
From: Dave Jiang @ 2018-05-11 19:52 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm
On 05/09/2018 04:26 PM, Dan Williams wrote:
> On Wed, May 9, 2018 at 4:24 PM, Dave Jiang <dave.jiang@intel.com> wrote:
>>
>>
>> On 05/09/2018 04:23 PM, Dan Williams wrote:
>>> On Wed, May 9, 2018 at 4:17 PM, Verma, Vishal L
>>> <vishal.l.verma@intel.com> wrote:
>>>> On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
>>>>> util_filter_walk() does the looping through of busses and regions.
>>>>> Removing
>>>>> duplicate code in region ops and provide filter functions so we can
>>>>> utilize util_filter_walk() and share common code.
>>>>>
>>>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>>>> ---
>>>>> ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++--------------
>>>>> --------
>>>>> util/filter.h | 6 ++++++
>>>>> 2 files changed, 42 insertions(+), 23 deletions(-)
>>>>>
>>>>> diff --git a/ndctl/region.c b/ndctl/region.c
>>>>> index 9fc90808..9fd07af6 100644
>>>>> --- a/ndctl/region.c
>>>>> +++ b/ndctl/region.c
>>>>> @@ -19,10 +19,7 @@
>>>>> #include <util/parse-options.h>
>>>>> #include <ndctl/libndctl.h>
>>>>>
>>>>> -static struct {
>>>>> - const char *bus;
>>>>> - const char *type;
>>>>> -} param;
>>>>> +struct util_filter_params param;
>>>>>
>>>>> static const struct option region_options[] = {
>>>>> OPT_STRING('b', "bus", ¶m.bus, "bus-id",
>>>>> @@ -92,33 +89,49 @@ static int region_action(struct ndctl_region *region,
>>>>> enum device_action mode)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx
>>>>> *ctx)
>>>>> +{
>>>>> + return true;
>>>>> +}
>>>>> +
>>>>
>>>> Instead of creating these trivial functions everywhere (also applies to
>>>> namespaces.c, dimm.c), should we just leave fctx.bus_filter NULL. And fix
>>>> util_filter_walk to check for fctx->ptr != NULL any time it calls one of
>>>> the functions..
>>>
>>> I think I'd prefer a common nop routine. That way individual are
>>> forced to decide to use the nop or implement something. Leaving it
>>> NULL may just be a programming mistake. I.e. it's harder to get wrong
>>> if it's required.
>>>
>>
>> I'll add a common nop routine.
>
> ...but wait why would it be a nop in this case. Region action commands
> take a --bus= option?
>
It just passes through by return true because it doesn't do anything to
do the bus? The util_bus_filter() call before it filters appropriate bus
right?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] ndctl: convert region actions to use util_filter_walk()
2018-05-11 19:52 ` Dave Jiang
@ 2018-05-11 20:17 ` Dan Williams
0 siblings, 0 replies; 14+ messages in thread
From: Dan Williams @ 2018-05-11 20:17 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Fri, May 11, 2018 at 12:52 PM, Dave Jiang <dave.jiang@intel.com> wrote:
>
>
> On 05/09/2018 04:26 PM, Dan Williams wrote:
>> On Wed, May 9, 2018 at 4:24 PM, Dave Jiang <dave.jiang@intel.com> wrote:
>>>
>>>
>>> On 05/09/2018 04:23 PM, Dan Williams wrote:
>>>> On Wed, May 9, 2018 at 4:17 PM, Verma, Vishal L
>>>> <vishal.l.verma@intel.com> wrote:
>>>>> On Fri, 2018-04-27 at 15:08 -0700, Dave Jiang wrote:
>>>>>> util_filter_walk() does the looping through of busses and regions.
>>>>>> Removing
>>>>>> duplicate code in region ops and provide filter functions so we can
>>>>>> utilize util_filter_walk() and share common code.
>>>>>>
>>>>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>>>>> ---
>>>>>> ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++--------------
>>>>>> --------
>>>>>> util/filter.h | 6 ++++++
>>>>>> 2 files changed, 42 insertions(+), 23 deletions(-)
>>>>>>
>>>>>> diff --git a/ndctl/region.c b/ndctl/region.c
>>>>>> index 9fc90808..9fd07af6 100644
>>>>>> --- a/ndctl/region.c
>>>>>> +++ b/ndctl/region.c
>>>>>> @@ -19,10 +19,7 @@
>>>>>> #include <util/parse-options.h>
>>>>>> #include <ndctl/libndctl.h>
>>>>>>
>>>>>> -static struct {
>>>>>> - const char *bus;
>>>>>> - const char *type;
>>>>>> -} param;
>>>>>> +struct util_filter_params param;
>>>>>>
>>>>>> static const struct option region_options[] = {
>>>>>> OPT_STRING('b', "bus", ¶m.bus, "bus-id",
>>>>>> @@ -92,33 +89,49 @@ static int region_action(struct ndctl_region *region,
>>>>>> enum device_action mode)
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> +static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx
>>>>>> *ctx)
>>>>>> +{
>>>>>> + return true;
>>>>>> +}
>>>>>> +
>>>>>
>>>>> Instead of creating these trivial functions everywhere (also applies to
>>>>> namespaces.c, dimm.c), should we just leave fctx.bus_filter NULL. And fix
>>>>> util_filter_walk to check for fctx->ptr != NULL any time it calls one of
>>>>> the functions..
>>>>
>>>> I think I'd prefer a common nop routine. That way individual are
>>>> forced to decide to use the nop or implement something. Leaving it
>>>> NULL may just be a programming mistake. I.e. it's harder to get wrong
>>>> if it's required.
>>>>
>>>
>>> I'll add a common nop routine.
>>
>> ...but wait why would it be a nop in this case. Region action commands
>> take a --bus= option?
>>
>
> It just passes through by return true because it doesn't do anything to
> do the bus? The util_bus_filter() call before it filters appropriate bus
> right?
Ah true, sorry, I confused myself on when the filter should return false.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 4/4] ndctl: convert dimm actions to use util_filter_walk()
2018-04-27 22:08 [PATCH v5 0/4] ndctl: convert actions to use util_filter_walk Dave Jiang
` (2 preceding siblings ...)
2018-04-27 22:08 ` [PATCH v5 3/4] ndctl: convert region " Dave Jiang
@ 2018-04-27 22:08 ` Dave Jiang
3 siblings, 0 replies; 14+ messages in thread
From: Dave Jiang @ 2018-04-27 22:08 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
util_filter_walk() does the looping through of busses and dimms. Removing
duplicate code for dimm actions and provide filter functions so we can
utilize util_filter_walk() and share common code.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
ndctl/dimm.c | 83 ++++++++++++++++++++++++++++++++++-----------------------
util/filter.h | 9 ++++++
2 files changed, 59 insertions(+), 33 deletions(-)
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 1c168997..d652be69 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -822,7 +822,6 @@ static int action_update(struct ndctl_dimm *dimm, struct action_context *actx)
}
static struct parameters {
- const char *bus;
const char *outfile;
const char *infile;
const char *labelversion;
@@ -833,6 +832,8 @@ static struct parameters {
.labelversion = "1.1",
};
+struct util_filter_params uf_param;
+
static int __action_init(struct ndctl_dimm *dimm,
enum ndctl_namespace_version version, int chk_only)
{
@@ -899,7 +900,7 @@ static int action_check(struct ndctl_dimm *dimm, struct action_context *actx)
#define BASE_OPTIONS() \
-OPT_STRING('b', "bus", ¶m.bus, "bus-id", \
+OPT_STRING('b', "bus", &uf_param.bus, "bus-id", \
"<nmem> must be on a bus with an id/provider of <bus-id>"), \
OPT_BOOLEAN('v',"verbose", ¶m.verbose, "turn on debug")
@@ -951,18 +952,48 @@ static const struct option init_options[] = {
OPT_END(),
};
+static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
+{
+ return true;
+}
+
+static void filter_dimm(struct ndctl_dimm *dimm, struct util_filter_ctx *ctx)
+{
+ struct dmaction_filter_arg *dfa = ctx->dmaction;
+ int rc;
+
+ if (dfa->action == action_write) {
+ dfa->dimm = dimm;
+ rc = 0;
+ } else
+ rc = dfa->action(dimm, dfa->actx);
+
+ if (rc == 0)
+ dfa->count++;
+ else if (rc && !dfa->rc)
+ dfa->rc = rc;
+}
+
+/* dummy filter function to skip the loop */
+static bool filter_region(struct ndctl_region *region,
+ struct util_filter_ctx *ctx)
+{
+ return false;
+}
+
static int dimm_action(int argc, const char **argv, void *ctx,
int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
const struct option *options, const char *usage)
{
struct action_context actx = { 0 };
- int i, rc = 0, count = 0, err = 0;
- struct ndctl_dimm *single = NULL;
+ int i, rc = 0, err = 0;
const char * const u[] = {
usage,
NULL
};
unsigned long id;
+ struct util_filter_ctx fctx = { 0 };
+ struct dmaction_filter_arg dfa = { 0 };
argc = parse_options(argc, argv, options, u, 0);
@@ -1039,45 +1070,31 @@ static int dimm_action(int argc, const char **argv, void *ctx,
goto out;
}
+ fctx.filter_bus = filter_bus;
+ fctx.filter_dimm = filter_dimm;
+ fctx.filter_region = filter_region;
+ fctx.dmaction = &dfa;
+ dfa.action = action;
+ dfa.actx = &actx;
+
rc = 0;
- err = 0;
- count = 0;
for (i = 0; i < argc; i++) {
- struct ndctl_dimm *dimm;
- struct ndctl_bus *bus;
-
if (sscanf(argv[i], "nmem%lu", &id) != 1
&& strcmp(argv[i], "all") != 0)
continue;
- ndctl_bus_foreach(ctx, bus) {
- if (!util_bus_filter(bus, param.bus))
- continue;
- ndctl_dimm_foreach(bus, dimm) {
- if (!util_dimm_filter(dimm, argv[i]))
- continue;
- if (action == action_write) {
- single = dimm;
- rc = 0;
- } else
- rc = action(dimm, &actx);
-
- if (rc == 0)
- count++;
- else if (rc && !err)
- err = rc;
- }
- }
+ uf_param.dimm = argv[i];
+ rc = util_filter_walk(ctx, &fctx, &uf_param);
}
- rc = err;
+ rc = dfa.rc;
if (action == action_write) {
- if (count > 1) {
+ if (dfa.count > 1) {
error("write-labels only supports writing a single dimm\n");
usage_with_options(u, options);
return -EINVAL;
- } else if (single)
- rc = action(single, &actx);
+ } else if (dfa.dimm)
+ rc = action(dfa.dimm, &actx);
}
if (actx.jdimms)
@@ -1095,8 +1112,8 @@ static int dimm_action(int argc, const char **argv, void *ctx,
* count if some actions succeeded, 0 if none were attempted,
* negative error code otherwise.
*/
- if (count > 0)
- return count;
+ if (dfa.count > 0)
+ return dfa.count;
return rc;
}
diff --git a/util/filter.h b/util/filter.h
index 0e0e3bec..2bef9310 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -62,6 +62,14 @@ struct rgaction_filter_arg {
int rc;
};
+struct dmaction_filter_arg {
+ struct action_context *actx;
+ struct ndctl_dimm *dimm;
+ int (*action)(struct ndctl_dimm *dimm, struct action_context *actx);
+ int count;
+ int rc;
+};
+
/*
* struct util_filter_ctx - control and callbacks for util_filter_walk()
* ->filter_bus() and ->filter_region() return bool because the
@@ -81,6 +89,7 @@ struct util_filter_ctx {
struct list_filter_arg *list;
struct nsaction_filter_arg *nsaction;
struct rgaction_filter_arg *rgaction;
+ struct dmaction_filter_arg *dmaction;
};
};
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 14+ messages in thread