[PATCH 1/3] ndctl: don't print erroneous namespace numa_nodes

[PATCH 1/3] ndctl: don't print erroneous namespace numa_nodes

[Ross Zwisler]

If the kernel has CONFIG_NUMA unset namespaces in sysfs will lack a
numa_node attribute.  In such cases ndctl will report a value of 0 for the
namespace numa_node in 'ndctl list'.  Instead of reporting potentially bad
data just hide the numa_node field if it is unsupported.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Fixes: commit f7d3de80a121 ("ndctl: support machines without numa")
---
 ndctl/lib/libndctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ed5a65b..b7180e8 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -3008,6 +3008,8 @@ static void *add_namespace(void *parent, int id, const char *ndns_base)
 	sprintf(path, "%s/numa_node", ndns_base);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		ndns->numa_node = strtol(buf, NULL, 0);
+	else
+		ndns->numa_node = -1;
 
 	sprintf(path, "%s/holder_class", ndns_base);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
-- 
2.14.3

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

[PATCH 2/3] ndctl: add numa_node support for regions

[Ross Zwisler]

Add an API for getting the numa node of a given region and add a numa_node
field to the "ndctl list" region output.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 ndctl/lib/libndctl.c   | 13 +++++++++++++
 ndctl/lib/libndctl.sym |  1 +
 ndctl/libndctl.h       |  1 +
 ndctl/list.c           |  8 ++++++++
 4 files changed, 23 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index b7180e8..a165e69 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -132,6 +132,7 @@ struct ndctl_mapping {
  * @type_name: 'pmem' or 'block'
  * @generation: incremented everytime the region is disabled
  * @nstype: the resulting type of namespace this region produces
+ * @numa_node: numa node attribute
  *
  * A region may alias between pmem and block-window access methods.  The
  * region driver is tasked with parsing the label (if their is one) and
@@ -157,6 +158,7 @@ struct ndctl_region {
 	char *region_buf;
 	int buf_len;
 	int generation;
+	int numa_node;
 	struct list_head btts;
 	struct list_head pfns;
 	struct list_head daxs;
@@ -1808,6 +1810,12 @@ static void *add_region(void *parent, int id, const char *region_base)
 		goto err_read;
 	region->module = to_module(ctx, buf);
 
+	sprintf(path, "%s/numa_node", region_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0)
+		region->numa_node = strtol(buf, NULL, 0);
+	else
+		region->numa_node = -1;
+
 	if (region_set_type(region, path) < 0)
 		goto err_read;
 
@@ -2011,6 +2019,11 @@ NDCTL_EXPORT struct ndctl_dimm *ndctl_region_get_next_dimm(struct ndctl_region *
 	return NULL;
 }
 
+NDCTL_EXPORT int ndctl_region_get_numa_node(struct ndctl_region *region)
+{
+	return region->numa_node;
+}
+
 static int regions_badblocks_init(struct ndctl_region *region)
 {
 	struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index af9b7d5..175a482 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -344,4 +344,5 @@ global:
 	ndctl_cmd_fw_fquery_get_fw_rev;
 	ndctl_cmd_fw_xlat_firmware_status;
 	ndctl_dimm_cmd_new_ack_shutdown_count;
+	ndctl_region_get_numa_node;
 } LIBNDCTL_13;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 9db775b..017c9cc 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -340,6 +340,7 @@ struct ndctl_ctx *ndctl_region_get_ctx(struct ndctl_region *region);
 struct ndctl_dimm *ndctl_region_get_first_dimm(struct ndctl_region *region);
 struct ndctl_dimm *ndctl_region_get_next_dimm(struct ndctl_region *region,
 		struct ndctl_dimm *dimm);
+int ndctl_region_get_numa_node(struct ndctl_region *region);
 struct ndctl_region *ndctl_bus_get_region_by_physical_address(struct ndctl_bus *bus,
 		unsigned long long address);
 #define ndctl_dimm_foreach_in_region(region, dimm) \
diff --git a/ndctl/list.c b/ndctl/list.c
index af1c024..4cb66de 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -73,6 +73,7 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 	struct ndctl_interleave_set *iset;
 	struct ndctl_mapping *mapping;
 	unsigned int bb_count = 0;
+	int numa;
 
 	if (!jregion)
 		return NULL;
@@ -107,6 +108,13 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 		goto err;
 	json_object_object_add(jregion, "type", jobj);
 
+	numa = ndctl_region_get_numa_node(region);
+	if (numa >= 0) {
+		jobj = json_object_new_int(numa);
+		if (jobj)
+			json_object_object_add(jregion, "numa_node", jobj);
+	}
+
 	iset = ndctl_region_get_interleave_set(region);
 	if (iset) {
 		jobj = util_json_object_hex(
-- 
2.14.3

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

Re: [PATCH 2/3] ndctl: add numa_node support for regions

[Dan Williams]

On Wed, Mar 7, 2018 at 10:02 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> Add an API for getting the numa node of a given region and add a numa_node
> field to the "ndctl list" region output.
>

Looks good,

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH 3/3] ndctl: add filtering based on numa_node

[Ross Zwisler]

Add support to 'ndctl list' so that we can filter DIMMs, regions and
namespaces based on numa node.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 Documentation/ndctl/ndctl-list.txt |  4 ++++
 ndctl/list.c                       |  2 ++
 util/filter.c                      | 38 ++++++++++++++++++++++++++++++++++++--
 util/filter.h                      |  1 +
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index 02d4f04..c711c1f 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -83,6 +83,10 @@ include::xable-region-options.txt[]
 	Filter listing by the mode ('raw', 'fsdax', 'sector' or 'devdax')
 	of the namespace(s).
 
+-U::
+--numa_node=::
+	Filter listing by numa node
+
 -B::
 --buses::
 	Include bus info in the listing
diff --git a/ndctl/list.c b/ndctl/list.c
index 4cb66de..e861f8b 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -400,6 +400,8 @@ int cmd_list(int argc, const char **argv, void *ctx)
 				"filter by namespace mode"),
 		OPT_STRING('t', "type", &param.type, "region-type",
 				"filter by region-type"),
+		OPT_STRING('U', "numa_node", &param.numa_node, "numa node",
+				"filter by numa node"),
 		OPT_BOOLEAN('B', "buses", &list.buses, "include bus info"),
 		OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm info"),
 		OPT_BOOLEAN('F', "firmware", &list.firmware, "include firmware info"),
diff --git a/util/filter.c b/util/filter.c
index b0b7fdf..85630ca 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -146,7 +146,6 @@ struct ndctl_bus *util_bus_filter_by_region(struct ndctl_bus *bus,
 	return NULL;
 }
 
-
 struct ndctl_bus *util_bus_filter_by_namespace(struct ndctl_bus *bus,
 		const char *ident)
 {
@@ -223,6 +222,25 @@ struct ndctl_dimm *util_dimm_filter_by_namespace(struct ndctl_dimm *dimm,
 	return NULL;
 }
 
+struct ndctl_dimm *util_dimm_filter_by_numa_node(struct ndctl_dimm *dimm,
+		int numa_node)
+{
+	struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
+	struct ndctl_region *region;
+	struct ndctl_dimm *check;
+
+	if (numa_node == -1)
+		return dimm;
+
+	ndctl_region_foreach(bus, region)
+		ndctl_dimm_foreach_in_region(region, check)
+			if (check == dimm &&
+			    ndctl_region_get_numa_node(region) == numa_node)
+				return dimm;
+
+	return NULL;
+}
+
 struct ndctl_region *util_region_filter_by_namespace(struct ndctl_region *region,
 		const char *ident)
 {
@@ -285,6 +303,8 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 {
 	struct ndctl_bus *bus;
 	unsigned int type = 0;
+	int numa_node = -1;
+	char *end = NULL;
 
 	if (param->type && (strcmp(param->type, "pmem") != 0
 				&& strcmp(param->type, "blk") != 0)) {
@@ -305,6 +325,14 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 		return -EINVAL;
 	}
 
+	if (param->numa_node && strcmp(param->numa_node, "all") != 0) {
+		numa_node = strtol(param->numa_node, &end, 0);
+		if (end == param->numa_node || end[0]) {
+			error("invalid numa_node: '%s'\n", param->numa_node);
+			return -EINVAL;
+		}
+	}
+
 	ndctl_bus_foreach(ctx, bus) {
 		struct ndctl_region *region;
 		struct ndctl_dimm *dimm;
@@ -326,7 +354,9 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 					|| !util_dimm_filter_by_region(dimm,
 						param->region)
 					|| !util_dimm_filter_by_namespace(dimm,
-						param->namespace))
+						param->namespace)
+					|| !util_dimm_filter_by_numa_node(dimm,
+						numa_node))
 				continue;
 
 			fctx->filter_dimm(dimm, fctx);
@@ -342,6 +372,10 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 						param->namespace))
 				continue;
 
+			if (numa_node != -1 &&
+			    ndctl_region_get_numa_node(region) != numa_node)
+				continue;
+
 			if (type && ndctl_region_get_type(region) != type)
 				continue;
 
diff --git a/util/filter.h b/util/filter.h
index aea5a71..effda24 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -77,6 +77,7 @@ struct util_filter_params {
 	const char *dimm;
 	const char *mode;
 	const char *namespace;
+	const char *numa_node;
 };
 
 struct ndctl_ctx;
-- 
2.14.3

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

Re: [PATCH 3/3] ndctl: add filtering based on numa_node

[Dan Williams]

On Wed, Mar 7, 2018 at 10:02 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> Add support to 'ndctl list' so that we can filter DIMMs, regions and
> namespaces based on numa node.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
>  Documentation/ndctl/ndctl-list.txt |  4 ++++
>  ndctl/list.c                       |  2 ++
>  util/filter.c                      | 38 ++++++++++++++++++++++++++++++++++++--
>  util/filter.h                      |  1 +
>  4 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
> index 02d4f04..c711c1f 100644
> --- a/Documentation/ndctl/ndctl-list.txt
> +++ b/Documentation/ndctl/ndctl-list.txt
> @@ -83,6 +83,10 @@ include::xable-region-options.txt[]
>         Filter listing by the mode ('raw', 'fsdax', 'sector' or 'devdax')
>         of the namespace(s).
>
> +-U::
> +--numa_node=::
> +       Filter listing by numa node
> +
>  -B::
>  --buses::
>         Include bus info in the listing
> diff --git a/ndctl/list.c b/ndctl/list.c
> index 4cb66de..e861f8b 100644
> --- a/ndctl/list.c
> +++ b/ndctl/list.c
> @@ -400,6 +400,8 @@ int cmd_list(int argc, const char **argv, void *ctx)
>                                 "filter by namespace mode"),
>                 OPT_STRING('t', "type", &param.type, "region-type",
>                                 "filter by region-type"),
> +               OPT_STRING('U', "numa_node", &param.numa_node, "numa node",
> +                               "filter by numa node"),
>                 OPT_BOOLEAN('B', "buses", &list.buses, "include bus info"),
>                 OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm info"),
>                 OPT_BOOLEAN('F', "firmware", &list.firmware, "include firmware info"),
> diff --git a/util/filter.c b/util/filter.c
> index b0b7fdf..85630ca 100644
> --- a/util/filter.c
> +++ b/util/filter.c
> @@ -146,7 +146,6 @@ struct ndctl_bus *util_bus_filter_by_region(struct ndctl_bus *bus,
>         return NULL;
>  }
>
> -
>  struct ndctl_bus *util_bus_filter_by_namespace(struct ndctl_bus *bus,
>                 const char *ident)
>  {
> @@ -223,6 +222,25 @@ struct ndctl_dimm *util_dimm_filter_by_namespace(struct ndctl_dimm *dimm,
>         return NULL;
>  }
>
> +struct ndctl_dimm *util_dimm_filter_by_numa_node(struct ndctl_dimm *dimm,
> +               int numa_node)
> +{
> +       struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
> +       struct ndctl_region *region;
> +       struct ndctl_dimm *check;
> +
> +       if (numa_node == -1)
> +               return dimm;
> +
> +       ndctl_region_foreach(bus, region)
> +               ndctl_dimm_foreach_in_region(region, check)
> +                       if (check == dimm &&
> +                           ndctl_region_get_numa_node(region) == numa_node)
> +                               return dimm;
> +
> +       return NULL;
> +}

Awesome, thanks for remembering to include dimms in this filtering.

> +
>  struct ndctl_region *util_region_filter_by_namespace(struct ndctl_region *region,
>                 const char *ident)
>  {
> @@ -285,6 +303,8 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
>  {
>         struct ndctl_bus *bus;
>         unsigned int type = 0;
> +       int numa_node = -1;
> +       char *end = NULL;
>
>         if (param->type && (strcmp(param->type, "pmem") != 0
>                                 && strcmp(param->type, "blk") != 0)) {
> @@ -305,6 +325,14 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
>                 return -EINVAL;
>         }
>
> +       if (param->numa_node && strcmp(param->numa_node, "all") != 0) {
> +               numa_node = strtol(param->numa_node, &end, 0);
> +               if (end == param->numa_node || end[0]) {
> +                       error("invalid numa_node: '%s'\n", param->numa_node);
> +                       return -EINVAL;
> +               }
> +       }
> +
>         ndctl_bus_foreach(ctx, bus) {
>                 struct ndctl_region *region;
>                 struct ndctl_dimm *dimm;
> @@ -326,7 +354,9 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
>                                         || !util_dimm_filter_by_region(dimm,
>                                                 param->region)
>                                         || !util_dimm_filter_by_namespace(dimm,
> -                                               param->namespace))
> +                                               param->namespace)
> +                                       || !util_dimm_filter_by_numa_node(dimm,
> +                                               numa_node))
>                                 continue;
>
>                         fctx->filter_dimm(dimm, fctx);
> @@ -342,6 +372,10 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
>                                                 param->namespace))
>                                 continue;
>
> +                       if (numa_node != -1 &&
> +                           ndctl_region_get_numa_node(region) != numa_node)
> +                               continue;

Maybe "numa_node >= 0", or "#define NUMA_NO_NODE  (-1)" and use that?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH 3/3] ndctl: add filtering based on numa_node

[Ross Zwisler]

On Wed, Mar 07, 2018 at 10:36:47AM -0800, Dan Williams wrote:
> > @@ -342,6 +372,10 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
> >                                                 param->namespace))
> >                                 continue;
> >
> > +                       if (numa_node != -1 &&
> > +                           ndctl_region_get_numa_node(region) != numa_node)
> > +                               continue;
> 
> Maybe "numa_node >= 0", or "#define NUMA_NO_NODE  (-1)" and use that?

Yea, that does make it easier to read.  Thanks for the review.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v2 3/3] ndctl: add filtering based on numa_node

[Ross Zwisler]

Add support to 'ndctl list' so that we can filter DIMMs, regions and
namespaces based on numa node.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan).
---
 Documentation/ndctl/ndctl-list.txt |  4 ++++
 ndctl/list.c                       |  2 ++
 util/filter.c                      | 40 ++++++++++++++++++++++++++++++++++++--
 util/filter.h                      |  1 +
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index 02d4f04..c711c1f 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -83,6 +83,10 @@ include::xable-region-options.txt[]
 	Filter listing by the mode ('raw', 'fsdax', 'sector' or 'devdax')
 	of the namespace(s).
 
+-U::
+--numa_node=::
+	Filter listing by numa node
+
 -B::
 --buses::
 	Include bus info in the listing
diff --git a/ndctl/list.c b/ndctl/list.c
index 4cb66de..e861f8b 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -400,6 +400,8 @@ int cmd_list(int argc, const char **argv, void *ctx)
 				"filter by namespace mode"),
 		OPT_STRING('t', "type", &param.type, "region-type",
 				"filter by region-type"),
+		OPT_STRING('U', "numa_node", &param.numa_node, "numa node",
+				"filter by numa node"),
 		OPT_BOOLEAN('B', "buses", &list.buses, "include bus info"),
 		OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm info"),
 		OPT_BOOLEAN('F', "firmware", &list.firmware, "include firmware info"),
diff --git a/util/filter.c b/util/filter.c
index b0b7fdf..291d7ed 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -20,6 +20,8 @@
 #include <ndctl/libndctl.h>
 #include <daxctl/libdaxctl.h>
 
+#define NUMA_NO_NODE    (-1)
+
 struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char *ident)
 {
 	char *end = NULL;
@@ -146,7 +148,6 @@ struct ndctl_bus *util_bus_filter_by_region(struct ndctl_bus *bus,
 	return NULL;
 }
 
-
 struct ndctl_bus *util_bus_filter_by_namespace(struct ndctl_bus *bus,
 		const char *ident)
 {
@@ -223,6 +224,25 @@ struct ndctl_dimm *util_dimm_filter_by_namespace(struct ndctl_dimm *dimm,
 	return NULL;
 }
 
+struct ndctl_dimm *util_dimm_filter_by_numa_node(struct ndctl_dimm *dimm,
+		int numa_node)
+{
+	struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
+	struct ndctl_region *region;
+	struct ndctl_dimm *check;
+
+	if (numa_node == NUMA_NO_NODE)
+		return dimm;
+
+	ndctl_region_foreach(bus, region)
+		ndctl_dimm_foreach_in_region(region, check)
+			if (check == dimm &&
+			    ndctl_region_get_numa_node(region) == numa_node)
+				return dimm;
+
+	return NULL;
+}
+
 struct ndctl_region *util_region_filter_by_namespace(struct ndctl_region *region,
 		const char *ident)
 {
@@ -285,6 +305,8 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 {
 	struct ndctl_bus *bus;
 	unsigned int type = 0;
+	int numa_node = NUMA_NO_NODE;
+	char *end = NULL;
 
 	if (param->type && (strcmp(param->type, "pmem") != 0
 				&& strcmp(param->type, "blk") != 0)) {
@@ -305,6 +327,14 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 		return -EINVAL;
 	}
 
+	if (param->numa_node && strcmp(param->numa_node, "all") != 0) {
+		numa_node = strtol(param->numa_node, &end, 0);
+		if (end == param->numa_node || end[0]) {
+			error("invalid numa_node: '%s'\n", param->numa_node);
+			return -EINVAL;
+		}
+	}
+
 	ndctl_bus_foreach(ctx, bus) {
 		struct ndctl_region *region;
 		struct ndctl_dimm *dimm;
@@ -326,7 +356,9 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 					|| !util_dimm_filter_by_region(dimm,
 						param->region)
 					|| !util_dimm_filter_by_namespace(dimm,
-						param->namespace))
+						param->namespace)
+					|| !util_dimm_filter_by_numa_node(dimm,
+						numa_node))
 				continue;
 
 			fctx->filter_dimm(dimm, fctx);
@@ -342,6 +374,10 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 						param->namespace))
 				continue;
 
+			if (numa_node != NUMA_NO_NODE &&
+			    ndctl_region_get_numa_node(region) != numa_node)
+				continue;
+
 			if (type && ndctl_region_get_type(region) != type)
 				continue;
 
diff --git a/util/filter.h b/util/filter.h
index aea5a71..effda24 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -77,6 +77,7 @@ struct util_filter_params {
 	const char *dimm;
 	const char *mode;
 	const char *namespace;
+	const char *numa_node;
 };
 
 struct ndctl_ctx;
-- 
2.14.3

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

Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node

[Dan Williams]

On Wed, Mar 7, 2018 at 10:53 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> Add support to 'ndctl list' so that we can filter DIMMs, regions and
> namespaces based on numa node.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
> v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan).

Looks good,

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node

[Verma, Vishal L]

On Wed, 2018-03-07 at 11:53 -0700, Ross Zwisler wrote:
> Add support to 'ndctl list' so that we can filter DIMMs, regions and
> namespaces based on numa node.

Something for the future - perhaps we can add this same numa node based
filtering to all the operations on namespaces/regions/dimms.

> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
> v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan).
> ---
>  Documentation/ndctl/ndctl-list.txt |  4 ++++
>  ndctl/list.c                       |  2 ++
>  util/filter.c                      | 40
> ++++++++++++++++++++++++++++++++++++--
>  util/filter.h                      |  1 +
>  4 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ndctl/ndctl-list.txt
> b/Documentation/ndctl/ndctl-list.txt
> index 02d4f04..c711c1f 100644
> --- a/Documentation/ndctl/ndctl-list.txt
> +++ b/Documentation/ndctl/ndctl-list.txt
> @@ -83,6 +83,10 @@ include::xable-region-options.txt[]
>  	Filter listing by the mode ('raw', 'fsdax', 'sector' or
> 'devdax')
>  	of the namespace(s).
>  
> +-U::
> +--numa_node=::
> +	Filter listing by numa node
> +
>  -B::
>  --buses::
>  	Include bus info in the listing
> diff --git a/ndctl/list.c b/ndctl/list.c
> index 4cb66de..e861f8b 100644
> --- a/ndctl/list.c
> +++ b/ndctl/list.c
> @@ -400,6 +400,8 @@ int cmd_list(int argc, const char **argv, void
> *ctx)
>  				"filter by namespace mode"),
>  		OPT_STRING('t', "type", &param.type, "region-type",
>  				"filter by region-type"),
> +		OPT_STRING('U', "numa_node", &param.numa_node, "numa
> node",
> +				"filter by numa node"),
>  		OPT_BOOLEAN('B', "buses", &list.buses, "include bus
> info"),
>  		OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm
> info"),
>  		OPT_BOOLEAN('F', "firmware", &list.firmware,
> "include firmware info"),
> diff --git a/util/filter.c b/util/filter.c
> index b0b7fdf..291d7ed 100644
> --- a/util/filter.c
> +++ b/util/filter.c
> @@ -20,6 +20,8 @@
>  #include <ndctl/libndctl.h>
>  #include <daxctl/libdaxctl.h>
>  
> +#define NUMA_NO_NODE    (-1)
> +
>  struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char
> *ident)
>  {
>  	char *end = NULL;
> @@ -146,7 +148,6 @@ struct ndctl_bus
> *util_bus_filter_by_region(struct ndctl_bus *bus,
>  	return NULL;
>  }
>  
> -
>  struct ndctl_bus *util_bus_filter_by_namespace(struct ndctl_bus
> *bus,
>  		const char *ident)
>  {
> @@ -223,6 +224,25 @@ struct ndctl_dimm
> *util_dimm_filter_by_namespace(struct ndctl_dimm *dimm,
>  	return NULL;
>  }
>  
> +struct ndctl_dimm *util_dimm_filter_by_numa_node(struct ndctl_dimm
> *dimm,
> +		int numa_node)
> +{
> +	struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
> +	struct ndctl_region *region;
> +	struct ndctl_dimm *check;
> +
> +	if (numa_node == NUMA_NO_NODE)
> +		return dimm;
> +
> +	ndctl_region_foreach(bus, region)
> +		ndctl_dimm_foreach_in_region(region, check)
> +			if (check == dimm &&
> +			    ndctl_region_get_numa_node(region) ==
> numa_node)
> +				return dimm;
> +
> +	return NULL;
> +}
> +
>  struct ndctl_region *util_region_filter_by_namespace(struct
> ndctl_region *region,
>  		const char *ident)
>  {
> @@ -285,6 +305,8 @@ int util_filter_walk(struct ndctl_ctx *ctx,
> struct util_filter_ctx *fctx,
>  {
>  	struct ndctl_bus *bus;
>  	unsigned int type = 0;
> +	int numa_node = NUMA_NO_NODE;
> +	char *end = NULL;
>  
>  	if (param->type && (strcmp(param->type, "pmem") != 0
>  				&& strcmp(param->type, "blk") != 0))
> {
> @@ -305,6 +327,14 @@ int util_filter_walk(struct ndctl_ctx *ctx,
> struct util_filter_ctx *fctx,
>  		return -EINVAL;
>  	}
>  
> +	if (param->numa_node && strcmp(param->numa_node, "all") !=
> 0) {

Does it make sense to accept an 'all' option for numa node? We're only
using it for filtering, and 'all' == not supplying the option at all..

> +		numa_node = strtol(param->numa_node, &end, 0);
> +		if (end == param->numa_node || end[0]) {
> +			error("invalid numa_node: '%s'\n", param-
> >numa_node);
> +			return -EINVAL;
> +		}
> +	}
> +
>  	ndctl_bus_foreach(ctx, bus) {
>  		struct ndctl_region *region;
>  		struct ndctl_dimm *dimm;
> @@ -326,7 +356,9 @@ int util_filter_walk(struct ndctl_ctx *ctx,
> struct util_filter_ctx *fctx,
>  					||
> !util_dimm_filter_by_region(dimm,
>  						param->region)
>  					||
> !util_dimm_filter_by_namespace(dimm,
> -						param->namespace))
> +						param->namespace)
> +					||
> !util_dimm_filter_by_numa_node(dimm,
> +						numa_node))
>  				continue;
>  
>  			fctx->filter_dimm(dimm, fctx);
> @@ -342,6 +374,10 @@ int util_filter_walk(struct ndctl_ctx *ctx,
> struct util_filter_ctx *fctx,
>  						param->namespace))
>  				continue;
>  
> +			if (numa_node != NUMA_NO_NODE &&
> +			    ndctl_region_get_numa_node(region) !=
> numa_node)
> +				continue;
> +
>  			if (type && ndctl_region_get_type(region) !=
> type)
>  				continue;
>  
> diff --git a/util/filter.h b/util/filter.h
> index aea5a71..effda24 100644
> --- a/util/filter.h
> +++ b/util/filter.h
> @@ -77,6 +77,7 @@ struct util_filter_params {
>  	const char *dimm;
>  	const char *mode;
>  	const char *namespace;
> +	const char *numa_node;
>  };
>  
>  struct ndctl_ctx;
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node

[Dan Williams]

On Wed, Mar 7, 2018 at 12:42 PM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Wed, 2018-03-07 at 11:53 -0700, Ross Zwisler wrote:
>> Add support to 'ndctl list' so that we can filter DIMMs, regions and
>> namespaces based on numa node.
>
> Something for the future - perhaps we can add this same numa node based
> filtering to all the operations on namespaces/regions/dimms.

This does have the region filtering, and that should also
automatically filter namespaces since we wouldn't even consider the
namespaces on a region where the numa node doesn't match.

>
>>
>> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> ---
>> v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan).
>> ---
>>  Documentation/ndctl/ndctl-list.txt |  4 ++++
>>  ndctl/list.c                       |  2 ++
>>  util/filter.c                      | 40
>> ++++++++++++++++++++++++++++++++++++--
>>  util/filter.h                      |  1 +
>>  4 files changed, 45 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/ndctl/ndctl-list.txt
>> b/Documentation/ndctl/ndctl-list.txt
>> index 02d4f04..c711c1f 100644
>> --- a/Documentation/ndctl/ndctl-list.txt
>> +++ b/Documentation/ndctl/ndctl-list.txt
>> @@ -83,6 +83,10 @@ include::xable-region-options.txt[]
>>       Filter listing by the mode ('raw', 'fsdax', 'sector' or
>> 'devdax')
>>       of the namespace(s).
>>
>> +-U::
>> +--numa_node=::
>> +     Filter listing by numa node
>> +
>>  -B::
>>  --buses::
>>       Include bus info in the listing
>> diff --git a/ndctl/list.c b/ndctl/list.c
>> index 4cb66de..e861f8b 100644
>> --- a/ndctl/list.c
>> +++ b/ndctl/list.c
>> @@ -400,6 +400,8 @@ int cmd_list(int argc, const char **argv, void
>> *ctx)
>>                               "filter by namespace mode"),
>>               OPT_STRING('t', "type", &param.type, "region-type",
>>                               "filter by region-type"),
>> +             OPT_STRING('U', "numa_node", &param.numa_node, "numa
>> node",
>> +                             "filter by numa node"),
>>               OPT_BOOLEAN('B', "buses", &list.buses, "include bus
>> info"),
>>               OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm
>> info"),
>>               OPT_BOOLEAN('F', "firmware", &list.firmware,
>> "include firmware info"),
>> diff --git a/util/filter.c b/util/filter.c
>> index b0b7fdf..291d7ed 100644
>> --- a/util/filter.c
>> +++ b/util/filter.c
>> @@ -20,6 +20,8 @@
>>  #include <ndctl/libndctl.h>
>>  #include <daxctl/libdaxctl.h>
>>
>> +#define NUMA_NO_NODE    (-1)
>> +
>>  struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char
>> *ident)
>>  {
>>       char *end = NULL;
>> @@ -146,7 +148,6 @@ struct ndctl_bus
>> *util_bus_filter_by_region(struct ndctl_bus *bus,
>>       return NULL;
>>  }
>>
>> -
>>  struct ndctl_bus *util_bus_filter_by_namespace(struct ndctl_bus
>> *bus,
>>               const char *ident)
>>  {
>> @@ -223,6 +224,25 @@ struct ndctl_dimm
>> *util_dimm_filter_by_namespace(struct ndctl_dimm *dimm,
>>       return NULL;
>>  }
>>
>> +struct ndctl_dimm *util_dimm_filter_by_numa_node(struct ndctl_dimm
>> *dimm,
>> +             int numa_node)
>> +{
>> +     struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
>> +     struct ndctl_region *region;
>> +     struct ndctl_dimm *check;
>> +
>> +     if (numa_node == NUMA_NO_NODE)
>> +             return dimm;
>> +
>> +     ndctl_region_foreach(bus, region)
>> +             ndctl_dimm_foreach_in_region(region, check)
>> +                     if (check == dimm &&
>> +                         ndctl_region_get_numa_node(region) ==
>> numa_node)
>> +                             return dimm;
>> +
>> +     return NULL;
>> +}
>> +
>>  struct ndctl_region *util_region_filter_by_namespace(struct
>> ndctl_region *region,
>>               const char *ident)
>>  {
>> @@ -285,6 +305,8 @@ int util_filter_walk(struct ndctl_ctx *ctx,
>> struct util_filter_ctx *fctx,
>>  {
>>       struct ndctl_bus *bus;
>>       unsigned int type = 0;
>> +     int numa_node = NUMA_NO_NODE;
>> +     char *end = NULL;
>>
>>       if (param->type && (strcmp(param->type, "pmem") != 0
>>                               && strcmp(param->type, "blk") != 0))
>> {
>> @@ -305,6 +327,14 @@ int util_filter_walk(struct ndctl_ctx *ctx,
>> struct util_filter_ctx *fctx,
>>               return -EINVAL;
>>       }
>>
>> +     if (param->numa_node && strcmp(param->numa_node, "all") !=
>> 0) {
>
> Does it make sense to accept an 'all' option for numa node? We're only
> using it for filtering, and 'all' == not supplying the option at all..

Same could be said for all the other places we accept all, I think it
should be "all or nothing" (heh heh heh), i.e. if we accept it as an
option for dimms regions and namespaces, why not numa nodes?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node

[Verma, Vishal L]

On Wed, 2018-03-07 at 12:48 -0800, Dan Williams wrote:
> On Wed, Mar 7, 2018 at 12:42 PM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> > 
> > On Wed, 2018-03-07 at 11:53 -0700, Ross Zwisler wrote:
> > > Add support to 'ndctl list' so that we can filter DIMMs, regions
> > > and
> > > namespaces based on numa node.
> > 
> > Something for the future - perhaps we can add this same numa node
> > based
> > filtering to all the operations on namespaces/regions/dimms.
> 
> This does have the region filtering, and that should also
> automatically filter namespaces since we wouldn't even consider the
> namespaces on a region where the numa node doesn't match.
> 
> > 
> > > 
> > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > ---
> > > v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan).
> > > ---
> > >  Documentation/ndctl/ndctl-list.txt |  4 ++++
> > >  ndctl/list.c                       |  2 ++
> > >  util/filter.c                      | 40
> > > ++++++++++++++++++++++++++++++++++++--
> > >  util/filter.h                      |  1 +
> > >  4 files changed, 45 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/ndctl/ndctl-list.txt
> > > b/Documentation/ndctl/ndctl-list.txt
> > > index 02d4f04..c711c1f 100644
> > > --- a/Documentation/ndctl/ndctl-list.txt
> > > +++ b/Documentation/ndctl/ndctl-list.txt
> > > @@ -83,6 +83,10 @@ include::xable-region-options.txt[]
> > >       Filter listing by the mode ('raw', 'fsdax', 'sector' or
> > > 'devdax')
> > >       of the namespace(s).
> > > 
> > > +-U::
> > > +--numa_node=::
> > > +     Filter listing by numa node
> > > +
> > >  -B::
> > >  --buses::
> > >       Include bus info in the listing
> > > diff --git a/ndctl/list.c b/ndctl/list.c
> > > index 4cb66de..e861f8b 100644
> > > --- a/ndctl/list.c
> > > +++ b/ndctl/list.c
> > > @@ -400,6 +400,8 @@ int cmd_list(int argc, const char **argv,
> > > void
> > > *ctx)
> > >                               "filter by namespace mode"),
> > >               OPT_STRING('t', "type", &param.type, "region-type",
> > >                               "filter by region-type"),
> > > +             OPT_STRING('U', "numa_node", &param.numa_node,
> > > "numa
> > > node",
> > > +                             "filter by numa node"),
> > >               OPT_BOOLEAN('B', "buses", &list.buses, "include bus
> > > info"),
> > >               OPT_BOOLEAN('D', "dimms", &list.dimms, "include
> > > dimm
> > > info"),
> > >               OPT_BOOLEAN('F', "firmware", &list.firmware,
> > > "include firmware info"),
> > > diff --git a/util/filter.c b/util/filter.c
> > > index b0b7fdf..291d7ed 100644
> > > --- a/util/filter.c
> > > +++ b/util/filter.c
> > > @@ -20,6 +20,8 @@
> > >  #include <ndctl/libndctl.h>
> > >  #include <daxctl/libdaxctl.h>
> > > 
> > > +#define NUMA_NO_NODE    (-1)
> > > +
> > >  struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const
> > > char
> > > *ident)
> > >  {
> > >       char *end = NULL;
> > > @@ -146,7 +148,6 @@ struct ndctl_bus
> > > *util_bus_filter_by_region(struct ndctl_bus *bus,
> > >       return NULL;
> > >  }
> > > 
> > > -
> > >  struct ndctl_bus *util_bus_filter_by_namespace(struct ndctl_bus
> > > *bus,
> > >               const char *ident)
> > >  {
> > > @@ -223,6 +224,25 @@ struct ndctl_dimm
> > > *util_dimm_filter_by_namespace(struct ndctl_dimm *dimm,
> > >       return NULL;
> > >  }
> > > 
> > > +struct ndctl_dimm *util_dimm_filter_by_numa_node(struct
> > > ndctl_dimm
> > > *dimm,
> > > +             int numa_node)
> > > +{
> > > +     struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
> > > +     struct ndctl_region *region;
> > > +     struct ndctl_dimm *check;
> > > +
> > > +     if (numa_node == NUMA_NO_NODE)
> > > +             return dimm;
> > > +
> > > +     ndctl_region_foreach(bus, region)
> > > +             ndctl_dimm_foreach_in_region(region, check)
> > > +                     if (check == dimm &&
> > > +                         ndctl_region_get_numa_node(region) ==
> > > numa_node)
> > > +                             return dimm;
> > > +
> > > +     return NULL;
> > > +}
> > > +
> > >  struct ndctl_region *util_region_filter_by_namespace(struct
> > > ndctl_region *region,
> > >               const char *ident)
> > >  {
> > > @@ -285,6 +305,8 @@ int util_filter_walk(struct ndctl_ctx *ctx,
> > > struct util_filter_ctx *fctx,
> > >  {
> > >       struct ndctl_bus *bus;
> > >       unsigned int type = 0;
> > > +     int numa_node = NUMA_NO_NODE;
> > > +     char *end = NULL;
> > > 
> > >       if (param->type && (strcmp(param->type, "pmem") != 0
> > >                               && strcmp(param->type, "blk") !=
> > > 0))
> > > {
> > > @@ -305,6 +327,14 @@ int util_filter_walk(struct ndctl_ctx *ctx,
> > > struct util_filter_ctx *fctx,
> > >               return -EINVAL;
> > >       }
> > > 
> > > +     if (param->numa_node && strcmp(param->numa_node, "all") !=
> > > 0) {
> > 
> > Does it make sense to accept an 'all' option for numa node? We're
> > only
> > using it for filtering, and 'all' == not supplying the option at
> > all..
> 
> Same could be said for all the other places we accept all, I think it
> should be "all or nothing" (heh heh heh), i.e. if we accept it as an
> option for dimms regions and namespaces, why not numa nodes?

Ah I didn't know we accepted all for the other things. /me wonders
whether it merits a bash completion update :)
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node

[Ross Zwisler]

On Wed, Mar 07, 2018 at 12:48:28PM -0800, Dan Williams wrote:
> On Wed, Mar 7, 2018 at 12:42 PM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> >
> > On Wed, 2018-03-07 at 11:53 -0700, Ross Zwisler wrote:
> >> Add support to 'ndctl list' so that we can filter DIMMs, regions and
> >> namespaces based on numa node.
> >
> > Something for the future - perhaps we can add this same numa node based
> > filtering to all the operations on namespaces/regions/dimms.
> 
> This does have the region filtering, and that should also
> automatically filter namespaces since we wouldn't even consider the
> namespaces on a region where the numa node doesn't match.

What other supports of operations were you thinking about?  Like

ndctl disable-region --numa_node=0

or something?  It doesn't look like we use other ndctl list type filters for
other operations like that today?  This seems powerful if we were to implement
it, but easy to accidentally operate on namespaces/regions you didn't intend.

> > Does it make sense to accept an 'all' option for numa node? We're only
> > using it for filtering, and 'all' == not supplying the option at all..
> 
> Same could be said for all the other places we accept all, I think it
> should be "all or nothing" (heh heh heh), i.e. if we accept it as an
> option for dimms regions and namespaces, why not numa nodes?

On a somewhat related note, what do you guys think of an option like this:

ndctl list --all

Which would just give you a full dump of all the various bits of info, so it
would currently be equivalent to:

ndctl list --buses --dimms --health --device-dax  --regions --namespaces
	--idle --media-errors

Providing you with a single short command to get as much info about a system
as possible?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node

[Dan Williams]

On Thu, Mar 8, 2018 at 9:21 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Wed, Mar 07, 2018 at 12:48:28PM -0800, Dan Williams wrote:
>> On Wed, Mar 7, 2018 at 12:42 PM, Verma, Vishal L
>> <vishal.l.verma@intel.com> wrote:
>> >
>> > On Wed, 2018-03-07 at 11:53 -0700, Ross Zwisler wrote:
>> >> Add support to 'ndctl list' so that we can filter DIMMs, regions and
>> >> namespaces based on numa node.
>> >
>> > Something for the future - perhaps we can add this same numa node based
>> > filtering to all the operations on namespaces/regions/dimms.
>>
>> This does have the region filtering, and that should also
>> automatically filter namespaces since we wouldn't even consider the
>> namespaces on a region where the numa node doesn't match.
>
> What other supports of operations were you thinking about?  Like
>
> ndctl disable-region --numa_node=0
>
> or something?  It doesn't look like we use other ndctl list type filters for
> other operations like that today?  This seems powerful if we were to implement
> it, but easy to accidentally operate on namespaces/regions you didn't intend.

This is Linux, defer to giving users all the rope they want.

>
>> > Does it make sense to accept an 'all' option for numa node? We're only
>> > using it for filtering, and 'all' == not supplying the option at all..
>>
>> Same could be said for all the other places we accept all, I think it
>> should be "all or nothing" (heh heh heh), i.e. if we accept it as an
>> option for dimms regions and namespaces, why not numa nodes?
>
> On a somewhat related note, what do you guys think of an option like this:
>
> ndctl list --all
>
> Which would just give you a full dump of all the various bits of info, so it
> would currently be equivalent to:
>
> ndctl list --buses --dimms --health --device-dax  --regions --namespaces
>         --idle --media-errors
>
> Providing you with a single short command to get as much info about a system
> as possible?

Sounds good to me... should also include --firmware in that list. I'd
probably call it --everything since "all" is already reserved as a
keyword for specific object types. I.e. "ndct list" by default is
"all" namespaces so I would expect "ndctl list --all" to also be all
namespaces.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node

[Vishal Verma]

On 03/07, Ross Zwisler wrote:
> Add support to 'ndctl list' so that we can filter DIMMs, regions and
> namespaces based on numa node.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
> v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan).
> ---
>  Documentation/ndctl/ndctl-list.txt |  4 ++++
>  ndctl/list.c                       |  2 ++
>  util/filter.c                      | 40 ++++++++++++++++++++++++++++++++++++--
>  util/filter.h                      |  1 +
>  4 files changed, 45 insertions(+), 2 deletions(-)
> 

Here is the bash completion update for this option.
I've tested it on my single-node VM, but Ross, can you test it on your
setup and let me know if it completes for the available nodes fine?

Thanks,
	-Vishal

8<-----

>From c744e77f26a27e10cc9dd8ca9b1092a45d9d9dcd Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.verma@intel.com>
Date: Thu, 8 Mar 2018 13:56:59 -0700
Subject: [ndctl PATCH] ndctl, bash-completion: add completion for --numa-node

ndctl list can now accept a --numa-node option for filtering by numa
node. Use the nodes in /sys/devices/system/node to provide completion
options for available nodes.

Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 contrib/ndctl | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/contrib/ndctl b/contrib/ndctl
index 89e0252..b1f5e59 100755
--- a/contrib/ndctl
+++ b/contrib/ndctl
@@ -91,7 +91,7 @@ __ndctlcomp()
 
 	COMPREPLY=( $( compgen -W "$1" -- "$2" ) )
 	for cword in "${COMPREPLY[@]}"; do
-		if [[ "$cword" == @(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace|--input|--output|--label-version|--align|--block|--count|--firmware|--media-temperature|--ctrl-temperature|--spares|--media-temperature-threshold|--ctrl-temperature-threshold|--spares-threshold|--media-temperature-alarm|--ctrl-temperature-alarm|--spares-alarm) ]]; then
+		if [[ "$cword" == @(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace|--input|--output|--label-version|--align|--block|--count|--firmware|--media-temperature|--ctrl-temperature|--spares|--media-temperature-threshold|--ctrl-temperature-threshold|--spares-threshold|--media-temperature-alarm|--ctrl-temperature-alarm|--spares-alarm|--numa_node) ]]; then
 			COMPREPLY[$i]="${cword}="
 		else
 			COMPREPLY[$i]="${cword} "
@@ -152,6 +152,21 @@ __ndctl_get_sector_sizes()
 	fi
 }
 
+__ndctl_get_nodes()
+{
+	local nlist=""
+
+	for node in /sys/devices/system/node/node*; do
+		node="$(basename $node)"
+		if [[ $node =~ node([0-9]+) ]]; then
+			nlist="$nlist ${BASH_REMATCH[1]}"
+		else
+			continue
+		fi
+	done
+	echo "$nlist"
+}
+
 __ndctl_file_comp()
 {
 	local cur="$1"
@@ -217,6 +232,9 @@ __ndctl_comp_options()
 		--spares-alarm)
 			opts="on off"
 			;;
+		--numa_node)
+			opts=$(__ndctl_get_nodes)
+			;;
 		*)
 			return
 			;;
-- 
2.14.3

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

Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node

[Ross Zwisler]

On Thu, Mar 08, 2018 at 02:08:09PM -0700, Vishal Verma wrote:
> On 03/07, Ross Zwisler wrote:
> > Add support to 'ndctl list' so that we can filter DIMMs, regions and
> > namespaces based on numa node.
> > 
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > ---
> > v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan).
> > ---
> >  Documentation/ndctl/ndctl-list.txt |  4 ++++
> >  ndctl/list.c                       |  2 ++
> >  util/filter.c                      | 40 ++++++++++++++++++++++++++++++++++++--
> >  util/filter.h                      |  1 +
> >  4 files changed, 45 insertions(+), 2 deletions(-)
> > 
> 
> Here is the bash completion update for this option.
> I've tested it on my single-node VM, but Ross, can you test it on your
> setup and let me know if it completes for the available nodes fine?
> 
> Thanks,
> 	-Vishal
> 
> 8<-----
> 
> From c744e77f26a27e10cc9dd8ca9b1092a45d9d9dcd Mon Sep 17 00:00:00 2001
> From: Vishal Verma <vishal.l.verma@intel.com>
> Date: Thu, 8 Mar 2018 13:56:59 -0700
> Subject: [ndctl PATCH] ndctl, bash-completion: add completion for --numa-node
> 
> ndctl list can now accept a --numa-node option for filtering by numa
> node. Use the nodes in /sys/devices/system/node to provide completion
> options for available nodes.
> 
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

Yep, this works like a charm.  You can add:

Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>

For v3 of my patch which changed numa_node to numa-node I had to make the
following changes to your patch (just s/numa_node/numa-node/ in 2 places):

diff --git a/contrib/ndctl b/contrib/ndctl
index b1f5e59..a3a8675 100755
--- a/contrib/ndctl
+++ b/contrib/ndctl
@@ -91,7 +91,7 @@ __ndctlcomp()
 
        COMPREPLY=( $( compgen -W "$1" -- "$2" ) )
        for cword in "${COMPREPLY[@]}"; do
-               if [[ "$cword" == @(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace|--input|--output|--label-version|--align|--block|--count|--firmware|--media-temperature|--ctrl-temperature|--spares|--media-temperature-threshold|--ctrl-temperature-threshold|--spares-threshold|--media-temperature-alarm|--ctrl-temperature-alarm|--spares-alarm|--numa_node) ]]; then
+               if [[ "$cword" == @(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace|--input|--output|--label-version|--align|--block|--count|--firmware|--media-temperature|--ctrl-temperature|--spares|--media-temperature-threshold|--ctrl-temperature-threshold|--spares-threshold|--media-temperature-alarm|--ctrl-temperature-alarm|--spares-alarm|--numa-node) ]]; then
                        COMPREPLY[$i]="${cword}="
                else
                        COMPREPLY[$i]="${cword} "
@@ -232,7 +232,7 @@ __ndctl_comp_options()
                --spares-alarm)
                        opts="on off"
                        ;;
-               --numa_node)
+               --numa-node)
                        opts=$(__ndctl_get_nodes)
                        ;;
                *)

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

[PATCH v3 3/3] ndctl: add filtering based on numa node

[Ross Zwisler]

Add support to 'ndctl list' so that we can filter DIMMs, regions and
namespaces based on numa node.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
v3: Changed the command line option from "numa_node" to "numa-node".
    (Vishal)
---
 Documentation/ndctl/ndctl-list.txt |  4 ++++
 ndctl/list.c                       |  2 ++
 util/filter.c                      | 40 ++++++++++++++++++++++++++++++++++++--
 util/filter.h                      |  1 +
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index 02d4f04..c4b38f9 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -83,6 +83,10 @@ include::xable-region-options.txt[]
 	Filter listing by the mode ('raw', 'fsdax', 'sector' or 'devdax')
 	of the namespace(s).
 
+-U::
+--numa-node=::
+	Filter listing by numa node
+
 -B::
 --buses::
 	Include bus info in the listing
diff --git a/ndctl/list.c b/ndctl/list.c
index 4cb66de..0ca5b6d 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -400,6 +400,8 @@ int cmd_list(int argc, const char **argv, void *ctx)
 				"filter by namespace mode"),
 		OPT_STRING('t', "type", &param.type, "region-type",
 				"filter by region-type"),
+		OPT_STRING('U', "numa-node", &param.numa_node, "numa node",
+				"filter by numa node"),
 		OPT_BOOLEAN('B', "buses", &list.buses, "include bus info"),
 		OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm info"),
 		OPT_BOOLEAN('F', "firmware", &list.firmware, "include firmware info"),
diff --git a/util/filter.c b/util/filter.c
index b0b7fdf..291d7ed 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -20,6 +20,8 @@
 #include <ndctl/libndctl.h>
 #include <daxctl/libdaxctl.h>
 
+#define NUMA_NO_NODE    (-1)
+
 struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char *ident)
 {
 	char *end = NULL;
@@ -146,7 +148,6 @@ struct ndctl_bus *util_bus_filter_by_region(struct ndctl_bus *bus,
 	return NULL;
 }
 
-
 struct ndctl_bus *util_bus_filter_by_namespace(struct ndctl_bus *bus,
 		const char *ident)
 {
@@ -223,6 +224,25 @@ struct ndctl_dimm *util_dimm_filter_by_namespace(struct ndctl_dimm *dimm,
 	return NULL;
 }
 
+struct ndctl_dimm *util_dimm_filter_by_numa_node(struct ndctl_dimm *dimm,
+		int numa_node)
+{
+	struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
+	struct ndctl_region *region;
+	struct ndctl_dimm *check;
+
+	if (numa_node == NUMA_NO_NODE)
+		return dimm;
+
+	ndctl_region_foreach(bus, region)
+		ndctl_dimm_foreach_in_region(region, check)
+			if (check == dimm &&
+			    ndctl_region_get_numa_node(region) == numa_node)
+				return dimm;
+
+	return NULL;
+}
+
 struct ndctl_region *util_region_filter_by_namespace(struct ndctl_region *region,
 		const char *ident)
 {
@@ -285,6 +305,8 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 {
 	struct ndctl_bus *bus;
 	unsigned int type = 0;
+	int numa_node = NUMA_NO_NODE;
+	char *end = NULL;
 
 	if (param->type && (strcmp(param->type, "pmem") != 0
 				&& strcmp(param->type, "blk") != 0)) {
@@ -305,6 +327,14 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 		return -EINVAL;
 	}
 
+	if (param->numa_node && strcmp(param->numa_node, "all") != 0) {
+		numa_node = strtol(param->numa_node, &end, 0);
+		if (end == param->numa_node || end[0]) {
+			error("invalid numa_node: '%s'\n", param->numa_node);
+			return -EINVAL;
+		}
+	}
+
 	ndctl_bus_foreach(ctx, bus) {
 		struct ndctl_region *region;
 		struct ndctl_dimm *dimm;
@@ -326,7 +356,9 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 					|| !util_dimm_filter_by_region(dimm,
 						param->region)
 					|| !util_dimm_filter_by_namespace(dimm,
-						param->namespace))
+						param->namespace)
+					|| !util_dimm_filter_by_numa_node(dimm,
+						numa_node))
 				continue;
 
 			fctx->filter_dimm(dimm, fctx);
@@ -342,6 +374,10 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 						param->namespace))
 				continue;
 
+			if (numa_node != NUMA_NO_NODE &&
+			    ndctl_region_get_numa_node(region) != numa_node)
+				continue;
+
 			if (type && ndctl_region_get_type(region) != type)
 				continue;
 
diff --git a/util/filter.h b/util/filter.h
index aea5a71..effda24 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -77,6 +77,7 @@ struct util_filter_params {
 	const char *dimm;
 	const char *mode;
 	const char *namespace;
+	const char *numa_node;
 };
 
 struct ndctl_ctx;
-- 
2.14.3

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

Re: [PATCH 1/3] ndctl: don't print erroneous namespace numa_nodes

[Dan Williams]

On Wed, Mar 7, 2018 at 10:02 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> If the kernel has CONFIG_NUMA unset namespaces in sysfs will lack a
> numa_node attribute.  In such cases ndctl will report a value of 0 for the
> namespace numa_node in 'ndctl list'.  Instead of reporting potentially bad
> data just hide the numa_node field if it is unsupported.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Fixes: commit f7d3de80a121 ("ndctl: support machines without numa")
> ---
>  ndctl/lib/libndctl.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index ed5a65b..b7180e8 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -3008,6 +3008,8 @@ static void *add_namespace(void *parent, int id, const char *ndns_base)
>         sprintf(path, "%s/numa_node", ndns_base);
>         if (sysfs_read_attr(ctx, path, buf) == 0)
>                 ndns->numa_node = strtol(buf, NULL, 0);
> +       else
> +               ndns->numa_node = -1;

Looks good.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm