Skip to contentSkip to conversation

ndctl: don't print erroneous namespace numa_nodes

PATCH In review Ross Zwisler 3 participants 17 messages Mar 7, 2018 – Mar 8, 2018

Subsystems (2)
Documentation libnvdimm: non-volatile memory device
lore
Share & tools
lore

Ross Zwisler

author
#0 lore raw

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

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 <[email protected]>
Fixes commit f7d3de80a121 ("ndctl: support machines without numa")
---
 ndctl/lib/libndctl.c | 2 ++
 1 file changed, 2 insertions(+)
ndctl/lib/libndctl.c +2 -0
index ed5a65b..b7180e8 100644
@@ -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)
Signature · 6 lines
2.14.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
author
#1 lore raw

PATCH 2/3 ndctl: add numa_node support for regions

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 <[email protected]>
---
 ndctl/lib/libndctl.c   | 13 +++++++++++++
 ndctl/lib/libndctl.sym |  1 +
 ndctl/libndctl.h       |  1 +
 ndctl/list.c           |  8 ++++++++
 4 files changed, 23 insertions(+)
ndctl/lib/libndctl.c +13 -0
index b7180e8..a165e69 100644
@@ -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);
ndctl/lib/libndctl.sym +1 -0
index af9b7d5..175a482 100644
@@ -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;
ndctl/libndctl.h +1 -0
index 9db775b..017c9cc 100644
@@ -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) \
ndctl/list.c +8 -0
index af1c024..4cb66de 100644
@@ -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(
Signature · 6 lines
2.14.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
author
#3 lore raw

PATCH 3/3 ndctl: add filtering based on numa_node

In reply to 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 <[email protected]>
---
 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(-)
Documentation/ndctl/ndctl-list.txt +4 -0
index 02d4f04..c711c1f 100644
@@ -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
ndctl/list.c +2 -0
index 4cb66de..e861f8b 100644
@@ -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"),
util/filter.c +36 -2
index b0b7fdf..85630ca 100644
@@ -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;
util/filter.h +1 -0
index aea5a71..effda24 100644
@@ -77,6 +77,7 @@ struct util_filter_params {
 	const char *dimm;
 	const char *mode;
 	const char *namespace;
+	const char *numa_node;
 };
 
 struct ndctl_ctx;
Signature · 6 lines
2.14.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
#4 lore raw

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

Quoted 63 lines · 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 <[email protected]>
---
 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(-)
Documentation/ndctl/ndctl-list.txt +4 -0
index 02d4f04..c711c1f 100644
@@ -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
ndctl/list.c +2 -0
index 4cb66de..e861f8b 100644
@@ -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"),
util/filter.c +8 -1
index b0b7fdf..85630ca 100644
@@ -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)

util/filter.c struct ndctl_dimm *util_dimm_filter_by_namespace(struct ndctl_dimm *dimm, · line 242

Quoted 10 lines
util/filter.c +10 -0
+               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.

Quoted 40 lines
util/filter.c +14 -1
+
 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);

util/filter.c int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx, · line 377

Quoted 6 lines
util/filter.c +3 -0
@@ -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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

author
#5 lore raw

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

Quoted 9 lines · Dan Williams

util/filter.c int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx, · line 377

Quoted 7 lines
util/filter.c +3 -0
@@ -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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

author
#6 lore raw

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

In reply to Dan Williams

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

Signed-off-by Ross Zwisler <[email protected]>

---
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(-)
Documentation/ndctl/ndctl-list.txt +4 -0
index 02d4f04..c711c1f 100644
@@ -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
ndctl/list.c +2 -0
index 4cb66de..e861f8b 100644
@@ -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"),
util/filter.c +38 -2
index b0b7fdf..291d7ed 100644
@@ -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;
util/filter.h +1 -0
index aea5a71..effda24 100644
@@ -77,6 +77,7 @@ struct util_filter_params {
 	const char *dimm;
 	const char *mode;
 	const char *namespace;
+	const char *numa_node;
 };
 
 struct ndctl_ctx;
Signature · 6 lines
2.14.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Reviewed-by on v2 3/3
#7 lore raw

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

Quoted 6 lines · 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 <[email protected]>

---
v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan).

Looks good,

Reviewed-by Dan Williams <[email protected]>

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

maintainer
#8 lore raw

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

In reply to Ross Zwisler
Quoted 2 lines · Ross Zwisler

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.

Quoted 113 lines
Signed-off-by Ross Zwisler <[email protected]>

---
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(-)
Documentation/ndctl/ndctl-list.txt +4 -0
index 02d4f04..c711c1f 100644
@@ -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::
Documentation/ndctl/ndctl-list.txt +0 -0
 	Include bus info in the listing
ndctl/list.c +2 -0
index 4cb66de..e861f8b 100644
@@ -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"),

util/filter.c +2 -0
index b0b7fdf..291d7ed 100644
@@ -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)
 {
util/filter.c +0 -1
 	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,
util/filter.c +19 -0
 		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,

util/filter.c +2 -0
 		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))
{

util/filter.c int util_filter_walk(struct ndctl_ctx *ctx, · line 332

Quoted 6 lines
util/filter.c +1 -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..

Quoted 45 lines
util/filter.c +7 -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;
+		}
+	}
+
util/filter.c +2 -1
 	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,

util/filter.c +5 -0
+						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)

util/filter.c +0 -0
 				continue;
util/filter.h +0 -0
index aea5a71..effda24 100644

util/filter.h struct util_filter_params { · line 83

Quoted 7 lines
util/filter.h +1 -0
@@ -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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

#9 lore raw

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

Quoted 7 lines · Vishal L
Quoted 2 lines · Ross Zwisler

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.

Quoted 118 lines
Quoted 118 lines
Signed-off-by Ross Zwisler <[email protected]>

---
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(-)
Documentation/ndctl/ndctl-list.txt +4 -0
index 02d4f04..c711c1f 100644
@@ -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

ndctl/list.c +2 -0
index 4cb66de..e861f8b 100644
@@ -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"),

util/filter.c +2 -0
index b0b7fdf..291d7ed 100644
@@ -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;
util/filter.c +0 -1
@@ -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)
{

util/filter.c +19 -0
@@ -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)
{

util/filter.c +2 -0
@@ -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))

{

util/filter.c +0 -0
@@ -305,6 +327,14 @@ int util_filter_walk(struct ndctl_ctx *ctx,
 struct util_filter_ctx *fctx,
              return -EINVAL;
      }
Quoted 5 lines

util/filter.c int util_filter_walk(struct ndctl_ctx *ctx, · line 332

Quoted 2 lines
util/filter.c +1 -0
+     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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

maintainer
#10 lore raw

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

Quoted 148 lines · Dan Williams
Quoted 9 lines · Vishal L
Quoted 3 lines · Ross Zwisler

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.

Quoted 124 lines
Quoted 124 lines
Signed-off-by Ross Zwisler <[email protected]>

---
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(-)
Documentation/ndctl/ndctl-list.txt +4 -0
index 02d4f04..c711c1f 100644
@@ -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

ndctl/list.c +0 -0
index 4cb66de..e861f8b 100644
@@ -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"),

util/filter.c +2 -0
index b0b7fdf..291d7ed 100644
@@ -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;
util/filter.c +0 -1
@@ -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)
{

util/filter.c +1 -0
@@ -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,

util/filter.c +13 -0
+             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)

util/filter.c +5 -0
+                             return dimm;
+
+     return NULL;
+}
+
 struct ndctl_region *util_region_filter_by_namespace(struct

ndctl_region *region,
const char *ident)
{

util/filter.c +2 -0
@@ -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))
{

util/filter.c +0 -0
@@ -305,6 +327,14 @@ int util_filter_walk(struct ndctl_ctx *ctx,
 struct util_filter_ctx *fctx,
              return -EINVAL;
      }
Quoted 7 lines

util/filter.c int util_filter_walk(struct ndctl_ctx *ctx, · line 332

Quoted 2 lines
util/filter.c +1 -0
+     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..

Quoted 3 lines

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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

author
#11 lore raw

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

In reply to Dan Williams
Quoted 7 lines · Dan Williams
Quoted 5 lines · Vishal L
Quoted 2 lines · Ross Zwisler

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

Quoted 6 lines
Quoted 2 lines

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.

Quoted 6 lines
Quoted 2 lines

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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

#12 lore raw

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

Quoted 15 lines · Ross Zwisler
Quoted 7 lines · Dan Williams
Quoted 5 lines · Vishal L
Quoted 2 lines · Ross Zwisler

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

Quoted 6 lines
Quoted 2 lines

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.

Quoted 7 lines

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.

Quoted 12 lines
Quoted 7 lines
Quoted 3 lines

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

Quoted 8 lines

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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

maintainer
#13 lore raw

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

In reply to Ross Zwisler
Quoted 4 lines · 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 <[email protected]>
Quoted 9 lines

---
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<-----

Quoted 1 line

From c744e77f26a27e10cc9dd8ca9b1092a45d9d9dcd Mon Sep 17 00:00:00 2001

From: Vishal Verma <[email protected]>
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 <[email protected]>
Signed-off-by Vishal Verma <[email protected]>
---
 contrib/ndctl | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)
contrib/ndctl +19 -1
index 89e0252..b1f5e59 100755
@@ -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
 			;;
Signature · 6 lines
2.14.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
author Tested-by
#14 lore raw

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

Quoted 29 lines · Vishal Verma
Quoted 4 lines · 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 <[email protected]>
Quoted 9 lines

---
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 <[email protected]>
Date: Thu, 8 Mar 2018 13:56:59 -0700
Subject: [ndctl PATCH] ndctl, bash-completion: add completion for --numa-node

Quoted 6 lines

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 <[email protected]>
Signed-off-by Vishal Verma <[email protected]>

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

Tested-by Ross Zwisler <[email protected]>

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):

contrib/ndctl +2 -2
index b1f5e59..a3a8675 100755
@@ -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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

author
#15 lore raw

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

In reply to 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 <[email protected]>
Reviewed-by Dan Williams <[email protected]>

---
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(-)
Documentation/ndctl/ndctl-list.txt +4 -0
index 02d4f04..c4b38f9 100644
@@ -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
ndctl/list.c +2 -0
index 4cb66de..0ca5b6d 100644
@@ -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"),
util/filter.c +38 -2
index b0b7fdf..291d7ed 100644
@@ -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;
util/filter.h +1 -0
index aea5a71..effda24 100644
@@ -77,6 +77,7 @@ struct util_filter_params {
 	const char *dimm;
 	const char *mode;
 	const char *namespace;
+	const char *numa_node;
 };
 
 struct ndctl_ctx;
Signature · 6 lines
2.14.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Reviewed-by on v1 1/3
#16 lore raw
In reply to Ross Zwisler
Quoted 16 lines · 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 <[email protected]>
Fixes commit f7d3de80a121 ("ndctl: support machines without numa")
---
 ndctl/lib/libndctl.c | 2 ++
 1 file changed, 2 insertions(+)
ndctl/lib/libndctl.c +0 -0
index ed5a65b..b7180e8 100644

ndctl/lib/libndctl.c static void *add_namespace(void *parent, int id, const char *ndns_base) · line 3012

Quoted 5 lines
ndctl/lib/libndctl.c +2 -0
@@ -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 <[email protected]>

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm