From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <1435163926.11808.295.camel@misato.fc.hp.com> References: <1434737914-18466-1-git-send-email-toshi.kani@hp.com> <1434737914-18466-4-git-send-email-toshi.kani@hp.com> <1435163926.11808.295.camel@misato.fc.hp.com> Date: Wed, 24 Jun 2015 09:42:20 -0700 Message-ID: Subject: Re: [PATCH v3 3/3] libnvdimm: Add sysfs numa_node to NVDIMM devices From: Dan Williams Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org To: Toshi Kani Cc: "Rafael J. Wysocki" , Linux ACPI , "linux-nvdimm@lists.01.org" , "linux-kernel@vger.kernel.org" List-ID: On Wed, Jun 24, 2015 at 9:38 AM, Toshi Kani wrote: > On Tue, 2015-06-23 at 17:26 -0700, Dan Williams wrote: >> On Fri, Jun 19, 2015 at 11:18 AM, Toshi Kani 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 >> > --- >> > 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? > > numa_node_show() is listed in its own nd_numa_attribute_group for using > is_visible. This nd_numa_attribute_group is then listed by region > (acpi_nfit_region_attribute_groups), namespace > (nd_namespace_attribute_groups), and btt (nd_btt_attribute_groups). > Therefore, numa_node_show() is only called with these device objects. > So, I do not think we need such change. Or are you suggesting to change > the way attribute group is set? No, my mistake I missed this hunk in drivers/nvdimm/region.c @@ -47,6 +47,7 @@ static int nd_region_probe(struct device *dev) num_ns->active = rc; num_ns->count = rc + err; + set_dev_node(dev, nd_region->numa_node); dev_set_drvdata(dev, num_ns); if (rc && err && rc == err) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753997AbbFXQmd (ORCPT ); Wed, 24 Jun 2015 12:42:33 -0400 Received: from mail-wg0-f54.google.com ([74.125.82.54]:34078 "EHLO mail-wg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753308AbbFXQmV (ORCPT ); Wed, 24 Jun 2015 12:42:21 -0400 MIME-Version: 1.0 In-Reply-To: <1435163926.11808.295.camel@misato.fc.hp.com> References: <1434737914-18466-1-git-send-email-toshi.kani@hp.com> <1434737914-18466-4-git-send-email-toshi.kani@hp.com> <1435163926.11808.295.camel@misato.fc.hp.com> Date: Wed, 24 Jun 2015 09:42:20 -0700 Message-ID: Subject: Re: [PATCH v3 3/3] libnvdimm: Add sysfs numa_node to NVDIMM devices From: Dan Williams To: Toshi Kani Cc: "Rafael J. Wysocki" , Linux ACPI , "linux-nvdimm@lists.01.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 24, 2015 at 9:38 AM, Toshi Kani wrote: > On Tue, 2015-06-23 at 17:26 -0700, Dan Williams wrote: >> On Fri, Jun 19, 2015 at 11:18 AM, Toshi Kani 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 >> > --- >> > 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? > > numa_node_show() is listed in its own nd_numa_attribute_group for using > is_visible. This nd_numa_attribute_group is then listed by region > (acpi_nfit_region_attribute_groups), namespace > (nd_namespace_attribute_groups), and btt (nd_btt_attribute_groups). > Therefore, numa_node_show() is only called with these device objects. > So, I do not think we need such change. Or are you suggesting to change > the way attribute group is set? No, my mistake I missed this hunk in drivers/nvdimm/region.c @@ -47,6 +47,7 @@ static int nd_region_probe(struct device *dev) num_ns->active = rc; num_ns->count = rc + err; + set_dev_node(dev, nd_region->numa_node); dev_set_drvdata(dev, num_ns); if (rc && err && rc == err)