linux-kernel - Re: [PATCH v6] driver core: platform: set numa_node before platform_device

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] [day] [month] [year] [list]

Message-Id: <20230918154441.442-1-guojinhui.liam@bytedance.com>
Date:   Mon, 18 Sep 2023 23:44:41 +0800
From:   Jinhui Guo <guojinhui.liam@...edance.com>
To:     rafael@...nel.org
Cc:     guojinhui.liam@...edance.com, gregkh@...uxfoundation.org,
        lenb@...nel.org, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org, lizefan.x@...edance.com,
        lkp@...el.com, stable@...r.kernel.org
Subject: Re: [PATCH v6] driver core: platform: set numa_node before platform_device_add()

> On Mon, Sep 18, 2023 at 2:41 PM Jinhui Guo <guojinhui.liam@...edance.com> wrote:
> >
> > On Mon, 18 Sep 2023 12:30:58 +0200, Rafael J. Wysocki wrote:
> > > On Thu, Sep 14, 2023 at 11:32 PM Jinhui Guo
> > > <guojinhui.liam@...edance.com> wrote:
> > > >
> > > > platform_add_device()
> > >
> > > According to "git grep" this function is not present in 6.6-rc2.
> > >
> > > If you mean platform_device_add(), please update the patch subject and
> > > changelog accordingly.
> > >
> >
> > This is my mistake, the function name was written wrong.
> > I will fix it in the next patch.
> >
> > > > creates the numa_node attribute of sysfs according
> > > > to whether dev_to_node(dev) is equal to NUMA_NO_NODE. So set the numa node
> > > > of device before creating numa_node attribute of sysfs.
> > >
> > > It would be good to also say that this needs to be done in
> > > platform_device_register_full(), because that's where the platform
> > > device object is allocated.
> > >
> >
> > Thaks for your suggestion. I will modify my decription soon.
> >
> > > However, what about adding the NUMA node information to pdevinfo?  It
> > > would be more straightforward to handle it then AFAICS.
> > >
> >
> > I have tried three potential solutions to fix the bug:
> > 1. The first one is what the current patch do.
> >
> > 2. Add a new function interface only for acpi_create_platform_device() call.
> >    But the code will be a bit redundant.
> >
> > 3. Add an member "numa_node" in `struct platform_device_info`, just as what
> >    `struct device` done:
> >
> > ```
> > struct platform_device_info {
> >         ...;
> > #ifdef CONFIG_NUMA
> >         int             numa_node;
> > #endif
> > ```
> >
> > But not all the call to platform_device_register_full() would set numa_node,
> > and many of them use ` memset(&pdevinfo, 0, sizeof(pdevinfo));` to initialize
> > `struct platform_device_info`. It could initialize numa_node to zero and
> > result in wrong numa_node information in sysfs.
> 
> Well, platform_device_register_full() need not take that value as the
> numa node number directly.  It may, for example, take the number from
> pdevinfo, subtract 1 from it and use the result of that as the numa
> node number, if not negative.

It's a good idea. I will try to fix the bug in this way.

Thanks,

Jinhui Guo