Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node
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
> > ---
> > 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
> 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
> Signed-off-by: Vishal Verma
Yep, this works like a charm. You can add:
Tested-by: Ross Zwisler
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
Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node
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
> ---
> 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
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
Signed-off-by: Vishal Verma
---
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
On Thu, Mar 8, 2018 at 9:21 AM, Ross Zwisler 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 >> 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
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 > 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
On Wed, 2018-03-07 at 12:48 -0800, Dan Williams wrote:
> On Wed, Mar 7, 2018 at 12:42 PM, Verma, Vishal L
> 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
> > > ---
> > > 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", ¶m.type, "region-type",
> > > "filter by region-type"),
> > > + OPT_STRING('U', "numa_node", ¶m.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
> > > #include
> > >
> > > +#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 sa
Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node
On Wed, Mar 7, 2018 at 12:42 PM, Verma, Vishal L
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
>> ---
>> 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", ¶m.type, "region-type",
>> "filter by region-type"),
>> + OPT_STRING('U', "numa_node", ¶m.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
>> #include
>>
>> +#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
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
> ---
> 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", ¶m.type, "region-type",
> "filter by region-type"),
> + OPT_STRING('U', "numa_node", ¶m.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
> #include
>
> +#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)
> + ||
> !uti
Re: [PATCH v2 3/3] ndctl: add filtering based on numa_node
On Wed, Mar 7, 2018 at 10:53 AM, 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 > --- > v2: Use NUMA_NO_NODE intead of hard coding -1 (Dan). Looks good, Reviewed-by: Dan Williams ___ 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
Add support to 'ndctl list' so that we can filter DIMMs, regions and
namespaces based on numa node.
Signed-off-by: Ross Zwisler
---
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", ¶m.type, "region-type",
"filter by region-type"),
+ OPT_STRING('U', "numa_node", ¶m.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
#include
+#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_no
