On Mon, Jun 17, 2024 at 11:01:23AM +0800, guojinhui.liam wrote:
> From: Jinhui Guo <guojinhui.liam@xxxxxxxxxxxxx>
>
> Setting the devices' numa_node needs to be done in
> platform_device_register_full(), because that's where the
> platform device object is allocated.
Why in the world is a platform device on a numa node? Are you sure you
are using platform devices properly if this is an issue?
And what platform devices / drivers care about this?
> Fixes: 4a60406d3592 ("driver core: platform: expose numa_node to users in sysfs")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
The robot reported this problem?
> Closes: https://lore.kernel.org/oe-kbuild-all/202309122309.mbxAnAIe-lkp@xxxxxxxxx/
That's a problem with an older version of this patch, not this one.
> Signed-off-by: Jinhui Guo <guojinhui.liam@xxxxxxxxxxxxx>
> ---
> V6 -> V7
> 1. Fix bug directly by adding numa_node to struct
> platform_device_info (suggested by Rafael J. Wysocki).
> 2. Remove reviewer name.
>
> V5 -> V6:
> 1. Update subject to correct function name platform_device_add().
> 2. Provide a more clear and accurate description of the changes
> made in commit (suggested by Rafael J. Wysocki).
> 3. Add reviewer name.
>
> V4 -> V5:
> Add Cc: stable line and changes from the previous submited patches.
>
> V3 -> V4:
> Refactor code to be an ACPI function call (suggested by Greg Kroah-Hartman).
>
> V2 -> V3:
> Fix Signed-off name.
>
> V1 -> V2:
> Fix compile error without enabling CONFIG_ACPI.
> ---
>
> drivers/acpi/acpi_platform.c | 5 ++---
> drivers/base/platform.c | 4 ++++
> include/linux/platform_device.h | 26 ++++++++++++++++++++++++++
> 3 files changed, 32 insertions(+), 3 deletions(-)
Any reason why you didn't cc the relevent maintainers here?
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index 7a41c72c1959..78e11b79f1af 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -132,10 +132,36 @@ struct platform_device_info {
> u64 dma_mask;
>
> const struct property_entry *properties;
> +
> +#ifdef CONFIG_NUMA
> + int numa_node; /* NUMA node this platform device is close to plus 1 */
> +#endif
Ick, no, why? Again, platform devices should NOT care about this. If
they do, they should not be a platform device.
> };
> extern struct platform_device *platform_device_register_full(
> const struct platform_device_info *pdevinfo);
>
> +#ifdef CONFIG_NUMA
> +static inline int platform_devinfo_get_node(const struct platform_device_info *pdevinfo)
> +{
> + return pdevinfo ? pdevinfo->numa_node - 1 : NUMA_NO_NODE;
> +}
> +
> +static inline void platform_devinfo_set_node(struct platform_device_info *pdevinfo,
> + int node)
> +{
> + pdevinfo->numa_node = node + 1;
Why +1?
thanks,
greg k-h
