linux-kernel - Re: [PATCH v3 3/3] libnvdimm: Add sysfs numa

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 23 Jun 2015 17:26:56 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	Toshi Kani <toshi.kani@...com>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Linux ACPI <linux-acpi@...r.kernel.org>,
	"linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/3] libnvdimm: Add sysfs numa_node to NVDIMM devices

On Fri, Jun 19, 2015 at 11:18 AM, Toshi Kani <toshi.kani@...com> wrote:
> Add support of sysfs 'numa_node' to I/O-related NVDIMM devices
> under /sys/bus/nd/devices, regionN, namespaceN.0, and bttN.
> When bttN is not set up, its numa_node returns -1 (NUMA_NO_NODE).
>
> An example of numa_node values on a 2-socket system with a single
> NVDIMM range on each socket is shown below.
>   /sys/bus/nd/devices
>   |-- btt0/numa_node:-1
>   |-- btt1/numa_node:0
>   |-- namespace0.0/numa_node:0
>   |-- namespace1.0/numa_node:1
>   |-- region0/numa_node:0
>   |-- region1/numa_node:1
>
> These numa_node files are then linked under the block class of
> their device names.
>   /sys/class/block/pmem0/device/numa_node:0
>   /sys/class/block/pmem0s/device/numa_node:0
>   /sys/class/block/pmem1/device/numa_node:1
>
> This enables numactl(8) to accept 'block:' and 'file:' paths of
> pmem and btt devices as shown in the examples below.
>   numactl --preferred block:pmem0 --show
>   numactl --preferred file:/dev/pmem0s --show
>
> Signed-off-by: Toshi Kani <toshi.kani@...com>
> ---
>  drivers/acpi/nfit.c             |    1 +
>  drivers/nvdimm/btt_devs.c       |    1 +
>  drivers/nvdimm/bus.c            |   30 ++++++++++++++++++++++++++++++
>  drivers/nvdimm/namespace_devs.c |    1 +
>  include/linux/libnvdimm.h       |    1 +
>  5 files changed, 34 insertions(+)
>
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index 5997753..9cb63ac 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c
> @@ -873,6 +873,7 @@ static const struct attribute_group *acpi_nfit_region_attribute_groups[] = {
>         &nd_region_attribute_group,
>         &nd_mapping_attribute_group,
>         &nd_device_attribute_group,
> +       &nd_numa_attribute_group,
>         &acpi_nfit_region_attribute_group,
>         NULL,
>  };
> diff --git a/drivers/nvdimm/btt_devs.c b/drivers/nvdimm/btt_devs.c
> index bcf77dc..a7b192f 100644
> --- a/drivers/nvdimm/btt_devs.c
> +++ b/drivers/nvdimm/btt_devs.c
> @@ -308,6 +308,7 @@ static struct attribute_group nd_btt_attribute_group = {
>  static const struct attribute_group *nd_btt_attribute_groups[] = {
>         &nd_btt_attribute_group,
>         &nd_device_attribute_group,
> +       &nd_numa_attribute_group,
>         NULL,
>  };
>
> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> index 67525f9..03c0ee1 100644
> --- a/drivers/nvdimm/bus.c
> +++ b/drivers/nvdimm/bus.c
> @@ -420,6 +420,36 @@ struct attribute_group nd_device_attribute_group = {
>  };
>  EXPORT_SYMBOL_GPL(nd_device_attribute_group);
>
> +static ssize_t numa_node_show(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       return sprintf(buf, "%d\n", dev_to_node(dev));
> +}

So patch 2 collided with the requested BTT stacking rework and
prompted me to take a closer look.  Shouldn't numa_node_show() be
changed like this?

@@ -273,7 +273,12 @@ EXPORT_SYMBOL_GPL(nd_device_attribute_group);
 static ssize_t numa_node_show(struct device *dev,
                struct device_attribute *attr, char *buf)
 {
-       return sprintf(buf, "%d\n", dev_to_node(dev));
+       if (is_nd_region(dev))
+               return sprintf(buf, "%d\n", dev_to_node(dev));
+       else if (is_nd_region(dev->parent))
+               return sprintf(buf, "%d\n", dev_to_node(dev->parent));
+       else
+               return sprintf(buf, "-1\n");
 }
 static DEVICE_ATTR_RO(numa_node);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/