nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] convert actions to use util_filter_walk
@ 2018-04-26 18:17 Dave Jiang
  2018-04-26 18:17 ` [PATCH v3 1/4] ndctl: convert namespace actions to use util_filter_params Dave Jiang
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Dave Jiang @ 2018-04-26 18:17 UTC (permalink / raw)
  To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm

util_filter_walk() does the looping through bus/dimm/region/namespace
that a lot of the operations in ndctl uses. Converting them to common
code and reduce maintenance on individual versions of the same code.
In this series we are convering namespace, region, and dimm actions.

---
v3:
- fixed some corner cases in namespace patch.
- changed param renaming to reduce change for util_filter_params. (Dan)
- Adding conversion to region
- Adding conversion to dimm

v2:
- split out the conversion of util_filter_params to make things more
  readable (Dan).
- Not pass in mode as util_filter_params and put back the mode check in
  util_filter_walk() (Dan).

Dave Jiang (4):
      ndctl: convert namespace actions to use util_filter_params
      ndctl: convert namespace actions to use util_filter_walk()
      ndctl: convert region actions to use util_filter_walk()
      ndctl: convert dimm actions to use util_filter_walk()


 ndctl/dimm.c      |   83 ++++++++++++++---------
 ndctl/namespace.c |  195 +++++++++++++++++++++++++++++------------------------
 ndctl/region.c    |   59 ++++++++++------
 test/btt-check.sh |    2 -
 util/filter.c     |    5 +
 util/filter.h     |   23 ++++++
 6 files changed, 221 insertions(+), 146 deletions(-)

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/4] ndctl: convert namespace actions to use util_filter_params
  2018-04-26 18:17 [PATCH v3 0/4] convert actions to use util_filter_walk Dave Jiang
@ 2018-04-26 18:17 ` Dave Jiang
  2018-04-26 18:17 ` [PATCH v3 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2018-04-26 18:17 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", &param.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", &param.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", &param.map, "memmap-location", \
 	"specify 'mem' or 'dev' for the location of the memmap"), \
 OPT_STRING('l', "sector-size", &param.sector_size, "lba-size", \
 	"specify the logical sector size in bytes"), \
-OPT_STRING('t', "type", &param.type, "type", \
+OPT_STRING('t', "type", &uf_param.type, "type", \
 	"specify the type of namespace to create 'pmem' or 'blk'"), \
 OPT_STRING('a', "align", &param.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(&param, 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] 6+ messages in thread
* [PATCH v3 2/4] ndctl: convert namespace actions to use util_filter_walk()
  2018-04-26 18:17 [PATCH v3 0/4] convert actions to use util_filter_walk Dave Jiang
  2018-04-26 18:17 ` [PATCH v3 1/4] ndctl: convert namespace actions to use util_filter_params Dave Jiang
@ 2018-04-26 18:17 ` Dave Jiang
  2018-04-27  0:44   ` Dan Williams
  2018-04-26 18:17 ` [PATCH v3 3/4] ndctl: convert region " Dave Jiang
  2018-04-26 18:17 ` [PATCH v3 4/4] ndctl: convert dimm " Dave Jiang
  3 siblings, 1 reply; 6+ messages in thread
From: Dave Jiang @ 2018-04-26 18:17 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 |  164 ++++++++++++++++++++++++++++++-----------------------
 test/btt-check.sh |    2 -
 util/filter.c     |    5 +-
 util/filter.h     |    8 +++
 4 files changed, 105 insertions(+), 74 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index e61f500c..d2a4fbaf 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -983,14 +983,92 @@ 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 ndns_filter_arg *nfa = ctx->ndns;
+	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 ndns_filter_arg *nfa = ctx->ndns;
+	const char *ndns_name;
+	int rc;
+
+	/* we have an error, don't do anything else */
+	if (nfa->rc < 0)
+		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 ndns_filter_arg nfa = { 0 };
 
 	if (!namespace && action != ACTION_CREATE)
 		return rc;
@@ -998,74 +1076,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.ndns = &nfa;
+	fctx.ndns->action = action;
+	fctx.ndns->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..ce3336a5 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 ndns_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 ndns_filter_arg *ndns;
 	};
 };
 

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3 3/4] ndctl: convert region actions to use util_filter_walk()
  2018-04-26 18:17 [PATCH v3 0/4] convert actions to use util_filter_walk Dave Jiang
  2018-04-26 18:17 ` [PATCH v3 1/4] ndctl: convert namespace actions to use util_filter_params Dave Jiang
  2018-04-26 18:17 ` [PATCH v3 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
@ 2018-04-26 18:17 ` Dave Jiang
  2018-04-26 18:17 ` [PATCH v3 4/4] ndctl: convert dimm " Dave Jiang
  3 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2018-04-26 18:17 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..be12a546 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", &param.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 region_filter_arg *rfa = ctx->region;
+	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 region_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.region = &rfa;
+	fctx.region->action = mode;
+	rc = util_filter_walk(ctx, &fctx, &param);
+	if (rc == 0)
+		rc = fctx.region->rc;
 
-	rc = success;
  out:
 	param.bus = NULL;
 	return rc;
diff --git a/util/filter.h b/util/filter.h
index ce3336a5..6c14a303 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -57,6 +57,11 @@ struct ndns_filter_arg {
 	int rc;
 };
 
+struct region_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 ndns_filter_arg *ndns;
+		struct region_filter_arg *region;
 	};
 };
 

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3 4/4] ndctl: convert dimm actions to use util_filter_walk()
  2018-04-26 18:17 [PATCH v3 0/4] convert actions to use util_filter_walk Dave Jiang
                   ` (2 preceding siblings ...)
  2018-04-26 18:17 ` [PATCH v3 3/4] ndctl: convert region " Dave Jiang
@ 2018-04-26 18:17 ` Dave Jiang
  3 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2018-04-26 18:17 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..f55b5ff0 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", &param.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", &param.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 dimm_filter_arg *dfa = ctx->dimm;
+	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 dimm_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.dimm = &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 6c14a303..3224ef2e 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -62,6 +62,14 @@ struct region_filter_arg {
 	int rc;
 };
 
+struct dimm_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 ndns_filter_arg *ndns;
 		struct region_filter_arg *region;
+		struct dimm_filter_arg *dimm;
 	};
 };
 

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/4] ndctl: convert namespace actions to use util_filter_walk()
  2018-04-26 18:17 ` [PATCH v3 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
@ 2018-04-27  0:44   ` Dan Williams
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Williams @ 2018-04-27  0:44 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-nvdimm

> diff --git a/util/filter.h b/util/filter.h
> index effda24b..ce3336a5 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 ndns_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 ndns_filter_arg *ndns;

This naming is throwing my brain for a loop because 'ndns' is the
abbreviation for 'struct namespace *' instances used everywhere else
in the code.  Let's call this type 'nsaction_filter_arg' and the
member instance as 'nsaction', or anything other than 'ndns'.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-27  0:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-26 18:17 [PATCH v3 0/4] convert actions to use util_filter_walk Dave Jiang
2018-04-26 18:17 ` [PATCH v3 1/4] ndctl: convert namespace actions to use util_filter_params Dave Jiang
2018-04-26 18:17 ` [PATCH v3 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
2018-04-27  0:44   ` Dan Williams
2018-04-26 18:17 ` [PATCH v3 3/4] ndctl: convert region " Dave Jiang
2018-04-26 18:17 ` [PATCH v3 4/4] ndctl: convert dimm " Dave Jiang

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