driver core: platform: expose numa_node to users in sysfs
diff mbox series

Message ID 20200602030139.73012-1-song.bao.hua@hisilicon.com
State New
Headers show
Series
  • driver core: platform: expose numa_node to users in sysfs
Related show

Commit Message

Song Bao Hua (Barry Song) June 2, 2020, 3:01 a.m. UTC
For some platform devices like iommu, particually ARM smmu, users may
care about the numa locality. for example, if threads and drivers run
near iommu, they may get much better performance on dma_unmap_sg.
For other platform devices, users may still want to know the hardware
topology.

Cc: Prime Zeng <prime.zeng@hisilicon.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 drivers/base/platform.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Greg Kroah-Hartman June 2, 2020, 4:23 a.m. UTC | #1
On Tue, Jun 02, 2020 at 03:01:39PM +1200, Barry Song wrote:
> For some platform devices like iommu, particually ARM smmu, users may
> care about the numa locality. for example, if threads and drivers run
> near iommu, they may get much better performance on dma_unmap_sg.
> For other platform devices, users may still want to know the hardware
> topology.
> 
> Cc: Prime Zeng <prime.zeng@hisilicon.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> ---
>  drivers/base/platform.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index b27d0f6c18c9..7794b9a38d82 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1062,13 +1062,37 @@ static ssize_t driver_override_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(driver_override);
>  
> +static ssize_t numa_node_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "%d\n", dev_to_node(dev));
> +}
> +static DEVICE_ATTR_RO(numa_node);
> +
> +static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a,
> +		int n)
> +{
> +	struct device *dev = container_of(kobj, typeof(*dev), kobj);
> +
> +	if (a == &dev_attr_numa_node.attr &&
> +			dev_to_node(dev) == NUMA_NO_NODE)
> +		return 0;
> +
> +	return a->mode;
> +}
>  
>  static struct attribute *platform_dev_attrs[] = {
>  	&dev_attr_modalias.attr,
> +	&dev_attr_numa_node.attr,
>  	&dev_attr_driver_override.attr,
>  	NULL,
>  };
> -ATTRIBUTE_GROUPS(platform_dev);
> +
> +static struct attribute_group platform_dev_group = {
> +	.attrs = platform_dev_attrs,
> +	.is_visible = platform_dev_attrs_visible,
> +};
> +__ATTRIBUTE_GROUPS(platform_dev);
>  
>  static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>  {

Platform devices are NUMA?  That's crazy, and feels like a total abuse
of platform devices and drivers that really should belong on a "real"
bus.

What drivers in the kernel today have this issue?

thanks,

greg k-h
Greg Kroah-Hartman June 2, 2020, 4:24 a.m. UTC | #2
On Tue, Jun 02, 2020 at 03:01:39PM +1200, Barry Song wrote:
> For some platform devices like iommu, particually ARM smmu, users may
> care about the numa locality. for example, if threads and drivers run
> near iommu, they may get much better performance on dma_unmap_sg.
> For other platform devices, users may still want to know the hardware
> topology.
> 
> Cc: Prime Zeng <prime.zeng@hisilicon.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> ---
>  drivers/base/platform.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index b27d0f6c18c9..7794b9a38d82 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1062,13 +1062,37 @@ static ssize_t driver_override_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(driver_override);
>  
> +static ssize_t numa_node_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "%d\n", dev_to_node(dev));
> +}
> +static DEVICE_ATTR_RO(numa_node);
> +
> +static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a,
> +		int n)
> +{
> +	struct device *dev = container_of(kobj, typeof(*dev), kobj);
> +
> +	if (a == &dev_attr_numa_node.attr &&
> +			dev_to_node(dev) == NUMA_NO_NODE)
> +		return 0;
> +
> +	return a->mode;
> +}
>  
>  static struct attribute *platform_dev_attrs[] = {
>  	&dev_attr_modalias.attr,
> +	&dev_attr_numa_node.attr,
>  	&dev_attr_driver_override.attr,
>  	NULL,
>  };
> -ATTRIBUTE_GROUPS(platform_dev);
> +
> +static struct attribute_group platform_dev_group = {
> +	.attrs = platform_dev_attrs,
> +	.is_visible = platform_dev_attrs_visible,
> +};
> +__ATTRIBUTE_GROUPS(platform_dev);
>  
>  static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>  {

Also you forgot a new entry in Documentation/ABI/ :(
Song Bao Hua (Barry Song) June 2, 2020, 4:42 a.m. UTC | #3
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, June 2, 2020 4:24 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>
> Cc: rafael@kernel.org; iommu@lists.linux-foundation.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Linuxarm
> <linuxarm@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; Robin
> Murphy <robin.murphy@arm.com>
> Subject: Re: [PATCH] driver core: platform: expose numa_node to users in sysfs
> 
> On Tue, Jun 02, 2020 at 03:01:39PM +1200, Barry Song wrote:
> > For some platform devices like iommu, particually ARM smmu, users may
> > care about the numa locality. for example, if threads and drivers run
> > near iommu, they may get much better performance on dma_unmap_sg.
> > For other platform devices, users may still want to know the hardware
> > topology.
> >
> > Cc: Prime Zeng <prime.zeng@hisilicon.com>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> > ---
> >  drivers/base/platform.c | 26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index b27d0f6c18c9..7794b9a38d82 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -1062,13 +1062,37 @@ static ssize_t driver_override_show(struct
> device *dev,
> >  }
> >  static DEVICE_ATTR_RW(driver_override);
> >
> > +static ssize_t numa_node_show(struct device *dev,
> > +		struct device_attribute *attr, char *buf)
> > +{
> > +	return sprintf(buf, "%d\n", dev_to_node(dev));
> > +}
> > +static DEVICE_ATTR_RO(numa_node);
> > +
> > +static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct
> attribute *a,
> > +		int n)
> > +{
> > +	struct device *dev = container_of(kobj, typeof(*dev), kobj);
> > +
> > +	if (a == &dev_attr_numa_node.attr &&
> > +			dev_to_node(dev) == NUMA_NO_NODE)
> > +		return 0;
> > +
> > +	return a->mode;
> > +}
> >
> >  static struct attribute *platform_dev_attrs[] = {
> >  	&dev_attr_modalias.attr,
> > +	&dev_attr_numa_node.attr,
> >  	&dev_attr_driver_override.attr,
> >  	NULL,
> >  };
> > -ATTRIBUTE_GROUPS(platform_dev);
> > +
> > +static struct attribute_group platform_dev_group = {
> > +	.attrs = platform_dev_attrs,
> > +	.is_visible = platform_dev_attrs_visible,
> > +};
> > +__ATTRIBUTE_GROUPS(platform_dev);
> >
> >  static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
> >  {
> 
> Platform devices are NUMA?  That's crazy, and feels like a total abuse
> of platform devices and drivers that really should belong on a "real"
> bus.

I am not sure if it is an abuse of platform device. But smmu is a platform device,
drivers/iommu/arm-smmu-v3.c is a platform driver.
In a typical ARM server, there are maybe multiple SMMU devices which can support
IO virtual address and page tables for other devices on PCI-like busses.
Each different SMMU device might be close to different NUMA node. There is
really a hardware topology.

If you have multiple CPU packages in a NUMA server, some platform devices might
Belong to CPU0, some other might belong to CPU1.

-barry

> 
> What drivers in the kernel today have this issue?
> 
> thanks,
> 
> greg k-h
Song Bao Hua (Barry Song) June 2, 2020, 5:09 a.m. UTC | #4
> >
> > Platform devices are NUMA?  That's crazy, and feels like a total abuse
> > of platform devices and drivers that really should belong on a "real"
> > bus.
> 
> I am not sure if it is an abuse of platform device. But smmu is a platform
> device,
> drivers/iommu/arm-smmu-v3.c is a platform driver.
> In a typical ARM server, there are maybe multiple SMMU devices which can
> support
> IO virtual address and page tables for other devices on PCI-like busses.
> Each different SMMU device might be close to different NUMA node. There is
> really a hardware topology.
> 
> If you have multiple CPU packages in a NUMA server, some platform devices
> might
> Belong to CPU0, some other might belong to CPU1.

Those devices are populated by acpi_iort for an ARM server:

drivers/acpi/arm64/iort.c:

static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
        .name = "arm-smmu-v3",
        .dev_dma_configure = arm_smmu_v3_dma_configure,
        .dev_count_resources = arm_smmu_v3_count_resources,
        .dev_init_resources = arm_smmu_v3_init_resources,
        .dev_set_proximity = arm_smmu_v3_set_proximity,
};

void __init acpi_iort_init(void)
{
        acpi_status status;

        status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
        ...
        iort_check_id_count_workaround(iort_table);
        iort_init_platform_devices();
}

static void __init iort_init_platform_devices(void)
{
        ...

        for (i = 0; i < iort->node_count; i++) {
                if (iort_node >= iort_end) {
                        pr_err("iort node pointer overflows, bad table\n");
                        return;
                }

                iort_enable_acs(iort_node);

                ops = iort_get_dev_cfg(iort_node);
                if (ops) {
                        fwnode = acpi_alloc_fwnode_static();
                        if (!fwnode)
                                return;

                        iort_set_fwnode(iort_node, fwnode);

                        ret = iort_add_platform_device(iort_node, ops);
                        if (ret) {
                                iort_delete_fwnode(iort_node);
                                acpi_free_fwnode_static(fwnode);
                                return;
                        }
                }

                ...
        }
...
}

NUMA node is got from ACPI:

static int  __init arm_smmu_v3_set_proximity(struct device *dev,
                                              struct acpi_iort_node *node)
{
        struct acpi_iort_smmu_v3 *smmu;

        smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
        if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
                int dev_node = acpi_map_pxm_to_node(smmu->pxm);

                ...

                set_dev_node(dev, dev_node);
                ...
        }
        return 0;
}

Barry
Greg Kroah-Hartman June 2, 2020, 6:11 a.m. UTC | #5
On Tue, Jun 02, 2020 at 05:09:57AM +0000, Song Bao Hua (Barry Song) wrote:
> > >
> > > Platform devices are NUMA?  That's crazy, and feels like a total abuse
> > > of platform devices and drivers that really should belong on a "real"
> > > bus.
> > 
> > I am not sure if it is an abuse of platform device. But smmu is a platform
> > device,
> > drivers/iommu/arm-smmu-v3.c is a platform driver.
> > In a typical ARM server, there are maybe multiple SMMU devices which can
> > support
> > IO virtual address and page tables for other devices on PCI-like busses.
> > Each different SMMU device might be close to different NUMA node. There is
> > really a hardware topology.
> > 
> > If you have multiple CPU packages in a NUMA server, some platform devices
> > might
> > Belong to CPU0, some other might belong to CPU1.
> 
> Those devices are populated by acpi_iort for an ARM server:
> 
> drivers/acpi/arm64/iort.c:
> 
> static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
>         .name = "arm-smmu-v3",
>         .dev_dma_configure = arm_smmu_v3_dma_configure,
>         .dev_count_resources = arm_smmu_v3_count_resources,
>         .dev_init_resources = arm_smmu_v3_init_resources,
>         .dev_set_proximity = arm_smmu_v3_set_proximity,
> };
> 
> void __init acpi_iort_init(void)
> {
>         acpi_status status;
> 
>         status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
>         ...
>         iort_check_id_count_workaround(iort_table);
>         iort_init_platform_devices();
> }
> 
> static void __init iort_init_platform_devices(void)
> {
>         ...
> 
>         for (i = 0; i < iort->node_count; i++) {
>                 if (iort_node >= iort_end) {
>                         pr_err("iort node pointer overflows, bad table\n");
>                         return;
>                 }
> 
>                 iort_enable_acs(iort_node);
> 
>                 ops = iort_get_dev_cfg(iort_node);
>                 if (ops) {
>                         fwnode = acpi_alloc_fwnode_static();
>                         if (!fwnode)
>                                 return;
> 
>                         iort_set_fwnode(iort_node, fwnode);
> 
>                         ret = iort_add_platform_device(iort_node, ops);
>                         if (ret) {
>                                 iort_delete_fwnode(iort_node);
>                                 acpi_free_fwnode_static(fwnode);
>                                 return;
>                         }
>                 }
> 
>                 ...
>         }
> ...
> }
> 
> NUMA node is got from ACPI:
> 
> static int  __init arm_smmu_v3_set_proximity(struct device *dev,
>                                               struct acpi_iort_node *node)
> {
>         struct acpi_iort_smmu_v3 *smmu;
> 
>         smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>         if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
>                 int dev_node = acpi_map_pxm_to_node(smmu->pxm);
> 
>                 ...
> 
>                 set_dev_node(dev, dev_node);
>                 ...
>         }
>         return 0;
> }
> 
> Barry

That's fine, but those are "real" devices, not platform devices, right?

What platform device has this issue?  What one will show up this way
with the new patch?

thanks,

greg k-h
Song Bao Hua (Barry Song) June 2, 2020, 6:26 a.m. UTC | #6
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, June 2, 2020 6:11 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>
> Cc: rafael@kernel.org; iommu@lists.linux-foundation.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Linuxarm
> <linuxarm@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; Robin
> Murphy <robin.murphy@arm.com>
> Subject: Re: [PATCH] driver core: platform: expose numa_node to users in sysfs
> 
> On Tue, Jun 02, 2020 at 05:09:57AM +0000, Song Bao Hua (Barry Song) wrote:
> > > >
> > > > Platform devices are NUMA?  That's crazy, and feels like a total
> > > > abuse of platform devices and drivers that really should belong on a
> "real"
> > > > bus.
> > >
> > > I am not sure if it is an abuse of platform device. But smmu is a
> > > platform device, drivers/iommu/arm-smmu-v3.c is a platform driver.
> > > In a typical ARM server, there are maybe multiple SMMU devices which
> > > can support IO virtual address and page tables for other devices on
> > > PCI-like busses.
> > > Each different SMMU device might be close to different NUMA node.
> > > There is really a hardware topology.
> > >
> > > If you have multiple CPU packages in a NUMA server, some platform
> > > devices might Belong to CPU0, some other might belong to CPU1.
> >
> > Those devices are populated by acpi_iort for an ARM server:
> >
> > drivers/acpi/arm64/iort.c:
> >
> > static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
> >         .name = "arm-smmu-v3",
> >         .dev_dma_configure = arm_smmu_v3_dma_configure,
> >         .dev_count_resources = arm_smmu_v3_count_resources,
> >         .dev_init_resources = arm_smmu_v3_init_resources,
> >         .dev_set_proximity = arm_smmu_v3_set_proximity, };
> >
> > void __init acpi_iort_init(void)
> > {
> >         acpi_status status;
> >
> >         status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
> >         ...
> >         iort_check_id_count_workaround(iort_table);
> >         iort_init_platform_devices();
> > }
> >
> > static void __init iort_init_platform_devices(void) {
> >         ...
> >
> >         for (i = 0; i < iort->node_count; i++) {
> >                 if (iort_node >= iort_end) {
> >                         pr_err("iort node pointer overflows, bad
> table\n");
> >                         return;
> >                 }
> >
> >                 iort_enable_acs(iort_node);
> >
> >                 ops = iort_get_dev_cfg(iort_node);
> >                 if (ops) {
> >                         fwnode = acpi_alloc_fwnode_static();
> >                         if (!fwnode)
> >                                 return;
> >
> >                         iort_set_fwnode(iort_node, fwnode);
> >
> >                         ret = iort_add_platform_device(iort_node, ops);
> >                         if (ret) {
> >                                 iort_delete_fwnode(iort_node);
> >                                 acpi_free_fwnode_static(fwnode);
> >                                 return;
> >                         }
> >                 }
> >
> >                 ...
> >         }
> > ...
> > }
> >
> > NUMA node is got from ACPI:
> >
> > static int  __init arm_smmu_v3_set_proximity(struct device *dev,
> >                                               struct acpi_iort_node
> > *node) {
> >         struct acpi_iort_smmu_v3 *smmu;
> >
> >         smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> >         if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> >                 int dev_node = acpi_map_pxm_to_node(smmu->pxm);
> >
> >                 ...
> >
> >                 set_dev_node(dev, dev_node);
> >                 ...
> >         }
> >         return 0;
> > }
> >
> > Barry
> 
> That's fine, but those are "real" devices, not platform devices, right?
> 

Most platform devices are "real" memory-mapped hardware devices. For an embedded system, almost all "simple-bus"
devices are populated from device trees as platform devices. Only a part of platform devices are not "real" hardware.

Smmu is a memory-mapped device. It is totally like most other platform devices populated in a 
memory space mapped in cpu's local space. It uses ioremap to map registers, use readl/writel to read/write its
space.

> What platform device has this issue?  What one will show up this way with
> the new patch?

if platform device shouldn't be a real hardware, there is no platform device with a hardware topology.
But platform devices are "real" hardware at most time. Smmu is a "real" device, but it is a platform device in Linux.

> 
> thanks,
> 
> greg k-h

-barry
Song Bao Hua (Barry Song) June 2, 2020, 7:02 a.m. UTC | #7
> >
> > On Tue, Jun 02, 2020 at 05:09:57AM +0000, Song Bao Hua (Barry Song)
> wrote:
> > > > >
> > > > > Platform devices are NUMA?  That's crazy, and feels like a total
> > > > > abuse of platform devices and drivers that really should belong
> > > > > on a
> > "real"
> > > > > bus.
> > > >
> > > > I am not sure if it is an abuse of platform device. But smmu is a
> > > > platform device, drivers/iommu/arm-smmu-v3.c is a platform driver.
> > > > In a typical ARM server, there are maybe multiple SMMU devices
> > > > which can support IO virtual address and page tables for other
> > > > devices on PCI-like busses.
> > > > Each different SMMU device might be close to different NUMA node.
> > > > There is really a hardware topology.
> > > >
> > > > If you have multiple CPU packages in a NUMA server, some platform
> > > > devices might Belong to CPU0, some other might belong to CPU1.
> > >
> > > Those devices are populated by acpi_iort for an ARM server:
> > >
> > > drivers/acpi/arm64/iort.c:
> > >
> > > static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
> > >         .name = "arm-smmu-v3",
> > >         .dev_dma_configure = arm_smmu_v3_dma_configure,
> > >         .dev_count_resources = arm_smmu_v3_count_resources,
> > >         .dev_init_resources = arm_smmu_v3_init_resources,
> > >         .dev_set_proximity = arm_smmu_v3_set_proximity, };
> > >
> > > void __init acpi_iort_init(void)
> > > {
> > >         acpi_status status;
> > >
> > >         status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
> > >         ...
> > >         iort_check_id_count_workaround(iort_table);
> > >         iort_init_platform_devices(); }
> > >
> > > static void __init iort_init_platform_devices(void) {
> > >         ...
> > >
> > >         for (i = 0; i < iort->node_count; i++) {
> > >                 if (iort_node >= iort_end) {
> > >                         pr_err("iort node pointer overflows, bad
> > table\n");
> > >                         return;
> > >                 }
> > >
> > >                 iort_enable_acs(iort_node);
> > >
> > >                 ops = iort_get_dev_cfg(iort_node);
> > >                 if (ops) {
> > >                         fwnode = acpi_alloc_fwnode_static();
> > >                         if (!fwnode)
> > >                                 return;
> > >
> > >                         iort_set_fwnode(iort_node, fwnode);
> > >
> > >                         ret = iort_add_platform_device(iort_node,
> ops);
> > >                         if (ret) {
> > >                                 iort_delete_fwnode(iort_node);
> > >                                 acpi_free_fwnode_static(fwnode);
> > >                                 return;
> > >                         }
> > >                 }
> > >
> > >                 ...
> > >         }
> > > ...
> > > }
> > >
> > > NUMA node is got from ACPI:
> > >
> > > static int  __init arm_smmu_v3_set_proximity(struct device *dev,
> > >                                               struct
> acpi_iort_node
> > > *node) {
> > >         struct acpi_iort_smmu_v3 *smmu;
> > >
> > >         smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> > >         if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> > >                 int dev_node = acpi_map_pxm_to_node(smmu->pxm);
> > >
> > >                 ...
> > >
> > >                 set_dev_node(dev, dev_node);
> > >                 ...
> > >         }
> > >         return 0;
> > > }
> > >
> > > Barry
> >
> > That's fine, but those are "real" devices, not platform devices, right?
> >
> 
> Most platform devices are "real" memory-mapped hardware devices. For an
> embedded system, almost all "simple-bus"
> devices are populated from device trees as platform devices. Only a part of
> platform devices are not "real" hardware.
> 
> Smmu is a memory-mapped device. It is totally like most other platform
> devices populated in a memory space mapped in cpu's local space. It uses
> ioremap to map registers, use readl/writel to read/write its space.
> 
> > What platform device has this issue?  What one will show up this way
> > with the new patch?

Meanwhile, this patch also only shows numa_node for those platform devices with "real"
hardware and acpi support. For those platform devices which are not "real", numa_node
sys entry will not be created as dev_to_node(dev) == NUMA_NO_NODE.

For instances, 
smmu is a platform device with "real" hardware backend, it gets a numa_node like:

root@ubuntu:/sys/bus/platform/devices/arm-smmu-v3.0.auto# cat numa_node
0

root@ubuntu:/sys/bus/platform/devices/arm-smmu-v3.7.auto# cat numa_node
2

snd-soc-dummy is a platform device without "real" hardware, then it gets no numa_node:

root@ubuntu:/sys/bus/platform/devices/snd-soc-dummy# ls
driver  driver_override  modalias  power  subsystem  uevent

-barry

> 
> if platform device shouldn't be a real hardware, there is no platform device
> with a hardware topology.
> But platform devices are "real" hardware at most time. Smmu is a "real" device,
> but it is a platform device in Linux.
> 
> >
> > thanks,
> >
> > greg k-h
> 
> -barry

Patch
diff mbox series

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b27d0f6c18c9..7794b9a38d82 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1062,13 +1062,37 @@  static ssize_t driver_override_show(struct device *dev,
 }
 static DEVICE_ATTR_RW(driver_override);
 
+static ssize_t numa_node_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", dev_to_node(dev));
+}
+static DEVICE_ATTR_RO(numa_node);
+
+static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a,
+		int n)
+{
+	struct device *dev = container_of(kobj, typeof(*dev), kobj);
+
+	if (a == &dev_attr_numa_node.attr &&
+			dev_to_node(dev) == NUMA_NO_NODE)
+		return 0;
+
+	return a->mode;
+}
 
 static struct attribute *platform_dev_attrs[] = {
 	&dev_attr_modalias.attr,
+	&dev_attr_numa_node.attr,
 	&dev_attr_driver_override.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(platform_dev);
+
+static struct attribute_group platform_dev_group = {
+	.attrs = platform_dev_attrs,
+	.is_visible = platform_dev_attrs_visible,
+};
+__ATTRIBUTE_GROUPS(platform_dev);
 
 static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 {