[+cc Rob]
On Thu, Aug 25, 2022 at 08:27:51PM +0800, Yang Yingliang wrote:
> If device_add() fails in pci_register_host_bridge(), the brigde device will
> be put once, and it will be put again in error path of pci_create_root_bus().
> Move the put_device() from pci_create_root_bus() to pci_register_host_bridge()
> to fix this problem. And use device_unregister() instead of del_device() and
> put_device().
s/brigde/bridge/
> Fixes: 9885440b16b8 ("PCI: Fix pci_host_bridge struct device release/free handling")
If you're fixing a commit from somebody else, please always cc: the
author because the author can help review the change.
> Signed-off-by: Yang Yingliang <yangyingliang@xxxxxxxxxx>
> ---
> drivers/pci/probe.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c5286b027f00..e500eb9d6468 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1027,7 +1027,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
>
> unregister:
> put_device(&bridge->dev);
> - device_del(&bridge->dev);
> + device_unregister(&bridge->dev);
I don't understand this part. device_unregister() looks like this:
void device_unregister(struct device *dev)
{
device_del(dev);
put_device(dev);
}
So this calls put_device(&bridge->dev) twice, doesn't it?
The "unregister" label looks poorly named. We only get there if
device_register() *failed*. We shouldn't need to unregister anything
in that case.
> free:
> kfree(bus);
> @@ -3037,13 +3037,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>
> error = pci_register_host_bridge(bridge);
> if (error < 0)
> - goto err_out;
> + return NULL;
>
> return bridge->bus;
> -
> -err_out:
> - put_device(&bridge->dev);
> - return NULL;
This part looks right to me. The get_device() is in
pci_register_host_bridge(), and if pci_register_host_bridge() returns
failure, I think it should first do the corresponding put_device().
> }
> EXPORT_SYMBOL_GPL(pci_create_root_bus);
>
> --
> 2.25.1
>
