| Message ID | 157309901655.1582359.18126990555058555754.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Memory Hierarchy: Enable target node lookups for reserved memory | expand |
Dan Williams <dan.j.williams@intel.com> writes: > A 'struct device_type' instance can carry default attributes for the > device. Use this facility to remove the export of > nd_numa_attribute_group and put the responsibility on the core rather > than leaf implementations to define this attribute. > > Cc: Ira Weiny <ira.weiny@intel.com> > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: "Oliver O'Halloran" <oohall@gmail.com> > Cc: Vishal Verma <vishal.l.verma@intel.com> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> can we also expose target_node in a similar way? This allows application to better understand the node locality of the SCM device. -aneesh
On Tue, Nov 12, 2019 at 1:23 AM Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> wrote: > > Dan Williams <dan.j.williams@intel.com> writes: > > > A 'struct device_type' instance can carry default attributes for the > > device. Use this facility to remove the export of > > nd_numa_attribute_group and put the responsibility on the core rather > > than leaf implementations to define this attribute. > > > > Cc: Ira Weiny <ira.weiny@intel.com> > > Cc: Michael Ellerman <mpe@ellerman.id.au> > > Cc: "Oliver O'Halloran" <oohall@gmail.com> > > Cc: Vishal Verma <vishal.l.verma@intel.com> > > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > > can we also expose target_node in a similar way? This allows application > to better understand the node locality of the SCM device. It is already exported for device-dax instances. See DEVICE_ATTR_RO(target_node) in drivers/dax/bus.c. I did not see a use case for it to be exported for other nvdimm device types.
On 11/13/19 6:56 AM, Dan Williams wrote: > On Tue, Nov 12, 2019 at 1:23 AM Aneesh Kumar K.V > <aneesh.kumar@linux.ibm.com> wrote: >> >> Dan Williams <dan.j.williams@intel.com> writes: >> >>> A 'struct device_type' instance can carry default attributes for the >>> device. Use this facility to remove the export of >>> nd_numa_attribute_group and put the responsibility on the core rather >>> than leaf implementations to define this attribute. >>> >>> Cc: Ira Weiny <ira.weiny@intel.com> >>> Cc: Michael Ellerman <mpe@ellerman.id.au> >>> Cc: "Oliver O'Halloran" <oohall@gmail.com> >>> Cc: Vishal Verma <vishal.l.verma@intel.com> >>> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> >>> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> >> >> can we also expose target_node in a similar way? This allows application >> to better understand the node locality of the SCM device. > > It is already exported for device-dax instances. See > DEVICE_ATTR_RO(target_node) in drivers/dax/bus.c. I did not see a use > case for it to be exported for other nvdimm device types. > some applications do want to access the fsdax namspace as different mount points based on numa affinity. If can differentiate the two regions with different target_node and same numa_node, that will help them better isolate these mounts. -aneesh
On Tue, Nov 12, 2019 at 10:02 PM Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> wrote: > > On 11/13/19 6:56 AM, Dan Williams wrote: > > On Tue, Nov 12, 2019 at 1:23 AM Aneesh Kumar K.V > > <aneesh.kumar@linux.ibm.com> wrote: > >> > >> Dan Williams <dan.j.williams@intel.com> writes: > >> > >>> A 'struct device_type' instance can carry default attributes for the > >>> device. Use this facility to remove the export of > >>> nd_numa_attribute_group and put the responsibility on the core rather > >>> than leaf implementations to define this attribute. > >>> > >>> Cc: Ira Weiny <ira.weiny@intel.com> > >>> Cc: Michael Ellerman <mpe@ellerman.id.au> > >>> Cc: "Oliver O'Halloran" <oohall@gmail.com> > >>> Cc: Vishal Verma <vishal.l.verma@intel.com> > >>> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > >>> Signed-off-by: Dan Williams <dan.j.williams@intel.com> > >> > >> > >> can we also expose target_node in a similar way? This allows application > >> to better understand the node locality of the SCM device. > > > > It is already exported for device-dax instances. See > > DEVICE_ATTR_RO(target_node) in drivers/dax/bus.c. I did not see a use > > case for it to be exported for other nvdimm device types. > > > > some applications do want to access the fsdax namspace as different > mount points based on numa affinity. If can differentiate the two > regions with different target_node and same numa_node, that will help > them better isolate these mounts. Can you be more explicit than "some", and what's the impact if the kernel continues with the status quo and does not export this data? I'm trying to come up with the justification to include into the changelog that adds that information. At least on the pmem platforms I am working with the pmem ranges with different target nodes also appear on different numa nodes so there is no incremental benefit for target node there.
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c index 04726f8fd189..6ffda03a6349 100644 --- a/arch/powerpc/platforms/pseries/papr_scm.c +++ b/arch/powerpc/platforms/pseries/papr_scm.c @@ -287,7 +287,6 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, static const struct attribute_group *region_attr_groups[] = { &nd_region_attribute_group, &nd_mapping_attribute_group, - &nd_numa_attribute_group, NULL, }; diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index dec7c2b08672..b3213faf37b5 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -2198,7 +2198,6 @@ static const struct attribute_group acpi_nfit_region_attribute_group = { static const struct attribute_group *acpi_nfit_region_attribute_groups[] = { &nd_region_attribute_group, &nd_mapping_attribute_group, - &nd_numa_attribute_group, &acpi_nfit_region_attribute_group, NULL, }; diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c index eb422527dd57..28e1b265aa63 100644 --- a/drivers/nvdimm/bus.c +++ b/drivers/nvdimm/bus.c @@ -697,11 +697,10 @@ static umode_t nd_numa_attr_visible(struct kobject *kobj, struct attribute *a, /* * nd_numa_attribute_group - NUMA attributes for all devices on an nd bus */ -struct attribute_group nd_numa_attribute_group = { +const struct attribute_group nd_numa_attribute_group = { .attrs = nd_numa_attributes, .is_visible = nd_numa_attr_visible, }; -EXPORT_SYMBOL_GPL(nd_numa_attribute_group); int nvdimm_bus_create_ndctl(struct nvdimm_bus *nvdimm_bus) { diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h index 3f509bb6b5c0..e7f3a2fee2ab 100644 --- a/drivers/nvdimm/nd.h +++ b/drivers/nvdimm/nd.h @@ -299,6 +299,7 @@ struct device *nd_pfn_devinit(struct nd_pfn *nd_pfn, int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig); extern const struct attribute_group *nd_pfn_attribute_groups[]; extern const struct attribute_group nd_device_attribute_group; +extern const struct attribute_group nd_numa_attribute_group; #else static inline int nd_pfn_probe(struct device *dev, struct nd_namespace_common *ndns) diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c index 710b5111eaa8..e4281f806adc 100644 --- a/drivers/nvdimm/region_devs.c +++ b/drivers/nvdimm/region_devs.c @@ -765,6 +765,7 @@ EXPORT_SYMBOL_GPL(nd_region_attribute_group); static const struct attribute_group *nd_region_attribute_groups[] = { &nd_device_attribute_group, + &nd_numa_attribute_group, NULL, }; diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h index d7dbf42498af..e9a4e25fc708 100644 --- a/include/linux/libnvdimm.h +++ b/include/linux/libnvdimm.h @@ -67,7 +67,6 @@ enum { extern struct attribute_group nvdimm_bus_attribute_group; extern struct attribute_group nvdimm_attribute_group; -extern struct attribute_group nd_numa_attribute_group; extern struct attribute_group nd_region_attribute_group; extern struct attribute_group nd_mapping_attribute_group;
A 'struct device_type' instance can carry default attributes for the device. Use this facility to remove the export of nd_numa_attribute_group and put the responsibility on the core rather than leaf implementations to define this attribute. Cc: Ira Weiny <ira.weiny@intel.com> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: "Oliver O'Halloran" <oohall@gmail.com> Cc: Vishal Verma <vishal.l.verma@intel.com> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- arch/powerpc/platforms/pseries/papr_scm.c | 1 - drivers/acpi/nfit/core.c | 1 - drivers/nvdimm/bus.c | 3 +-- drivers/nvdimm/nd.h | 1 + drivers/nvdimm/region_devs.c | 1 + include/linux/libnvdimm.h | 1 - 6 files changed, 3 insertions(+), 5 deletions(-)