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

[Samiullah Khawaja]

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@xxxxxxxxxx>
> > ---
> >  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