Re: [PATCH] PCI: Init driver override spinlock in new_id_store()

[Samiullah Khawaja <skhawaja@google.com>]

On Mon, Apr 27, 2026 at 09:46:20PM +0200, Danilo Krummrich wrote:
>On Mon Apr 27, 2026 at 9:31 PM CEST, Samiullah Khawaja wrote:
>> Fixes: cb3d1049f4ea ("driver core: generalize driver_override in struct device")
>
>I don't think anything is wrong with this commit, and it seems unrelated.

I will remove this one.
>
>> Fixes: 10a4206a2401 ("PCI: use generic driver_override infrastructure")
>
>I'm also not sure that this one contains the root cause, despite revealing the
>actual issue, more below.

I see your point. I understand the actual issue is that the temporary
pci_dev struct was not init and that was added in 2014. I added this
since this is where the regression happened.
>
>> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>> ---
>>  drivers/pci/pci-driver.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> index d10ece0889f0..5f453213b8c5 100644
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
>> @@ -215,6 +215,11 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
>>  		pdev->subsystem_device = subdevice;
>>  		pdev->class = class;
>>
>> +		/*
>> +		 * Initialize the embedded struct device driver_override lock to
>> +		 * avoid the lockdep errors.
>> +		 */
>> +		spin_lock_init(&pdev->dev.driver_override.lock);
>
>Can't we just call device_initialize() and set pdev->dev.release to a new
>function that just calls kfree()?
>
>This way nothing of that kind can ever happen again; it is hard to predict that
>a device is used without ever being initialized.

Agreed.

yes we can set the release function and call device_initialize().

Should I send v2 with something like following:

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index d10ece0889f0..634b4311bf5d 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -179,6 +179,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
         return NULL;
  }

+static void _release_temp_device(struct device *dev)
+{
+       kfree(to_pci_dev(dev));
+}
+
  /**
   * new_id_store - sysfs frontend to pci_add_dynid()
   * @driver: target device driver
@@ -214,11 +219,14 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
                 pdev->subsystem_vendor = subvendor;
                 pdev->subsystem_device = subdevice;
                 pdev->class = class;
+               pdev->dev.release = _release_temp_device;

+               /* Initialize the embedded struct device. */
+               device_initialize(&pdev->dev);
                 if (pci_match_device(pdrv, pdev))
                         retval = -EEXIST;

-               kfree(pdev);
+               put_device(&pdev->dev);

                 if (retval)
                         return retval;
>
>>  		if (pci_match_device(pdrv, pdev))
>>  			retval = -EEXIST;
>>
>>
>> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
>> --
>> 2.54.0.545.g6539524ca2-goog