| Message ID | 1370538609-28903-2-git-send-email-jiang.liu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, Jun 6, 2013 at 10:10 AM, Jiang Liu <liuj97@gmail.com> wrote: > This patch makes PCI host bridge/bus creating and destroying logic > symmetric by using device_initialize()/device_add()/device_del()/put_device() > pairs as discussed in thread at: > http://comments.gmane.org/gmane.linux.kernel.pci/22073 > > It also fixes a bug in error recovery path in pci_create_root_bus() > which may kfree() an in-use host bridge object. > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > Cc: Yinghai Lu <yinghai@kernel.org> > --- > drivers/pci/probe.c | 92 +++++++++++++++++++++++----------------------------- > drivers/pci/remove.c | 3 +- > 2 files changed, 42 insertions(+), 53 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 2f81a0a..9e9cdfe 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -470,7 +470,7 @@ void pci_read_bridge_bases(struct pci_bus *child) > } > } > > -static struct pci_bus * pci_alloc_bus(void) > +static struct pci_bus *pci_alloc_bus(struct pci_ops *ops, void *sd, int bus) > { > struct pci_bus *b; > > @@ -483,10 +483,32 @@ static struct pci_bus * pci_alloc_bus(void) > INIT_LIST_HEAD(&b->resources); > b->max_bus_speed = PCI_SPEED_UNKNOWN; > b->cur_bus_speed = PCI_SPEED_UNKNOWN; > + b->sysdata = sd; > + b->ops = ops; > + b->number = bus; > + b->busn_res.start = bus; > + b->busn_res.end = 0xff; > + b->busn_res.flags = IORESOURCE_BUS; > + b->dev.class = &pcibus_class; > + dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus); > + device_initialize(&b->dev); > } > + > return b; > } > > +static void pci_release_host_bridge_dev(struct device *dev) > +{ > + struct pci_host_bridge *bridge = to_pci_host_bridge(dev); > + > + if (bridge->release_fn) > + bridge->release_fn(bridge); > + > + pci_free_resource_list(&bridge->windows); > + > + kfree(bridge); > +} > + should split pci_release_host_bridge_dev renaming and moving to another patch. > static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b) > { > struct pci_host_bridge *bridge; > @@ -495,6 +517,10 @@ static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b) > if (bridge) { > INIT_LIST_HEAD(&bridge->windows); > bridge->bus = b; > + bridge->dev.release = pci_release_host_bridge_dev; > + dev_set_name(&bridge->dev, "pci%04x:%02x", > + pci_domain_nr(b), b->number); > + device_initialize(&bridge->dev); > } > > return bridge; > @@ -647,28 +673,13 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, > /* > * Allocate a new bus, and inherit stuff from the parent.. > */ > - child = pci_alloc_bus(); > + child = pci_alloc_bus(parent->ops, parent->sysdata, busnr); > if (!child) > return NULL; > > child->parent = parent; > - child->ops = parent->ops; > - child->sysdata = parent->sysdata; > child->bus_flags = parent->bus_flags; > - > - /* initialize some portions of the bus device, but don't register it > - * now as the parent is not properly set up yet. > - */ > - child->dev.class = &pcibus_class; > - dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr); > - > - /* > - * Set up the primary, secondary and subordinate > - * bus numbers. > - */ > - child->number = child->busn_res.start = busnr; > child->primary = parent->busn_res.start; > - child->busn_res.end = 0xff; > > if (!bridge) { > child->dev.parent = parent->bridge; > @@ -689,7 +700,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, > bridge->subordinate = child; > > add_dev: > - ret = device_register(&child->dev); > + ret = device_add(&child->dev); > WARN_ON(ret < 0); > > pcibios_add_bus(child); > @@ -1208,18 +1219,6 @@ int pci_cfg_space_size(struct pci_dev *dev) > return PCI_CFG_SPACE_SIZE; > } > > -static void pci_release_bus_bridge_dev(struct device *dev) > -{ > - struct pci_host_bridge *bridge = to_pci_host_bridge(dev); > - > - if (bridge->release_fn) > - bridge->release_fn(bridge); > - > - pci_free_resource_list(&bridge->windows); > - > - kfree(bridge); > -} > - > struct pci_dev *pci_alloc_dev(struct pci_bus *bus) > { > struct pci_dev *dev; > @@ -1707,13 +1706,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, > char bus_addr[64]; > char *fmt; > > - b = pci_alloc_bus(); > + b = pci_alloc_bus(ops, sysdata, bus); > if (!b) > return NULL; > > - b->sysdata = sysdata; > - b->ops = ops; > - b->number = b->busn_res.start = bus; > b2 = pci_find_bus(pci_domain_nr(b), bus); > if (b2) { > /* If we already got to this bus through a different bridge, ignore it */ > @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, > goto err_out; > > bridge->dev.parent = parent; > - bridge->dev.release = pci_release_bus_bridge_dev; > - dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); > error = pcibios_root_bridge_prepare(bridge); > - if (error) { > - kfree(bridge); > - goto err_out; > - } > + if (error) > + goto bridge_dev_reg_err; > + > + error = device_add(&bridge->dev); > + if (error) > + goto bridge_dev_reg_err; > > - error = device_register(&bridge->dev); > - if (error) { > - kfree(bridge); > - goto err_out; > - } > b->bridge = get_device(&bridge->dev); > device_enable_async_suspend(b->bridge); > pci_set_bus_of_node(b); > - > if (!parent) > set_dev_node(b->bridge, pcibus_to_node(b)); > - > - b->dev.class = &pcibus_class; > b->dev.parent = b->bridge; > - dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus); > - error = device_register(&b->dev); > + error = device_add(&b->dev); > if (error) > goto class_dev_reg_err; > > @@ -1792,10 +1779,11 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, > return b; > > class_dev_reg_err: > + device_del(&bridge->dev); > +bridge_dev_reg_err: > put_device(&bridge->dev); > - device_unregister(&bridge->dev); > err_out: > - kfree(b); > + put_device(&b->dev); > return NULL; > } > > diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c > index 8fc54b7..b0ce875 100644 > --- a/drivers/pci/remove.c > +++ b/drivers/pci/remove.c > @@ -52,7 +52,8 @@ void pci_remove_bus(struct pci_bus *bus) > up_write(&pci_bus_sem); > pci_remove_legacy_files(bus); > pcibios_remove_bus(bus); > - device_unregister(&bus->dev); > + device_del(&bus->dev); > + put_device(&bus->dev); > } > EXPORT_SYMBOL(pci_remove_bus); > looks like we don't need to split device_register to device_del and put_device. we can make root bus removal to use device_unregister too, in the patches, https://patchwork.kernel.org/patch/2562431/ [1/7] PCI: move back pci_proc_attach_devices calling https://patchwork.kernel.org/patch/2562461/ [2/7] PCI: move resources and bus_list releasing to pci_release_dev https://patchwork.kernel.org/patch/2562451/ [3/7] PCI: Detach driver in pci_stop_device Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 06, 2013 at 01:01:23PM -0700, Yinghai Lu wrote: > On Thu, Jun 6, 2013 at 10:10 AM, Jiang Liu <liuj97@gmail.com> wrote: > > This patch makes PCI host bridge/bus creating and destroying logic > > symmetric by using device_initialize()/device_add()/device_del()/put_device() > > pairs as discussed in thread at: > > http://comments.gmane.org/gmane.linux.kernel.pci/22073 > > > > It also fixes a bug in error recovery path in pci_create_root_bus() > > which may kfree() an in-use host bridge object. > > > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > Cc: Yinghai Lu <yinghai@kernel.org> > > --- > > drivers/pci/probe.c | 92 +++++++++++++++++++++++----------------------------- > > drivers/pci/remove.c | 3 +- > > 2 files changed, 42 insertions(+), 53 deletions(-) > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index 2f81a0a..9e9cdfe 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > ... > > +static void pci_release_host_bridge_dev(struct device *dev) > > +{ > > + struct pci_host_bridge *bridge = to_pci_host_bridge(dev); > > + > > + if (bridge->release_fn) > > + bridge->release_fn(bridge); > > + > > + pci_free_resource_list(&bridge->windows); > > + > > + kfree(bridge); > > +} > > + > > should split pci_release_host_bridge_dev renaming and moving to another patch. I split the rename/move to another patch and added both to my pci/jiang-bus-lock-v3 branch. > > @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, > > goto err_out; > > > > bridge->dev.parent = parent; > > - bridge->dev.release = pci_release_bus_bridge_dev; > > - dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); > > error = pcibios_root_bridge_prepare(bridge); > > - if (error) { > > - kfree(bridge); > > - goto err_out; > > - } > > + if (error) > > + goto bridge_dev_reg_err; > > + > > + error = device_add(&bridge->dev); > > + if (error) > > + goto bridge_dev_reg_err; > > > > - error = device_register(&bridge->dev); > > - if (error) { > > - kfree(bridge); > > - goto err_out; > > - } I think this function would be better if we created the host bridge first, then the bus. But your patch is an improvement, so we can look at doing that later. > looks like we don't need to split device_register to device_del and put_device. > > we can make root bus removal to use device_unregister too, in the patches, If we can use device_register()/device_unregister() directly instead of splitting them into initialize/add/del/put, that would be awesome. Anyway, http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3 is the current state of what I've got. If you want to iterate on this some more and improve things, that's great. But please start with what's on my branch and post your updates as a complete v4 because I did tweak some changelogs and code formatting, and I don't want to lose that work. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 7, 2013 at 6:51 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> >> should split pci_release_host_bridge_dev renaming and moving to another patch. > > I split the rename/move to another patch and added both to my > pci/jiang-bus-lock-v3 branch. good. > >> > @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct .. ... > > I think this function would be better if we created the host bridge > first, then the bus. But your patch is an improvement, so we can look > at doing that later. Yes, that will more straightforward, create hostbidge before root bus, like we have pci bridge before subordinate bus under it. current logic is b = pci_alloc_bus(); if (!b) return NULL; b->sysdata = sysdata; b->ops = ops; b->number = b->busn_res.start = bus; b2 = pci_find_bus(pci_domain_nr(b), bus); if (b2) { /* If we already got to this bus through a different bridge, ignore it */ dev_dbg(&b2->dev, "bus already known\n"); goto err_out; } bridge = pci_alloc_host_bridge(b); We need to find the duplicated hostbridge or racing. If we need call pci_alloc_host_bridge at first, we will need my for_each_host_bridge patchset, and use it to check if there is existing hostbridge. > >> looks like we don't need to split device_register to device_del and put_device. >> >> we can make root bus removal to use device_unregister too, in the patches, > > If we can use device_register()/device_unregister() directly instead of > splitting them into initialize/add/del/put, that would be awesome. > > Anyway, http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3 > is the current state of what I've got. If you want to iterate on this some > more and improve things, that's great. But please start with what's on my > branch and post your updates as a complete v4 because I did tweak some > changelogs and code formatting, and I don't want to lose that work. ok, please consider drop commit d751727a1792262cb464d8d9608500facb763301 PCI: Make host bridge/bus creating and destroying logic symmetric from pci/jiang-bus-lock-v3, and apply my three patches at https://patchwork.kernel.org/patch/2562431/ [1/7] PCI: move back pci_proc_attach_devices calling https://patchwork.kernel.org/patch/2562461/ [2/7] PCI: move resources and bus_list releasing to pci_release_dev https://patchwork.kernel.org/patch/2562451/ [3/7] PCI: Detach driver in pci_stop_device Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jun 8, 2013 at 1:01 AM, Yinghai Lu <yinghai@kernel.org> wrote: > On Fri, Jun 7, 2013 at 6:51 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>> >>> should split pci_release_host_bridge_dev renaming and moving to another patch. >> >> I split the rename/move to another patch and added both to my >> pci/jiang-bus-lock-v3 branch. > > good. > >> >>> > @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct .. > ... >> >> I think this function would be better if we created the host bridge >> first, then the bus. But your patch is an improvement, so we can look >> at doing that later. > > Yes, that will more straightforward, create hostbidge before root bus, > like we have pci bridge before subordinate bus under it. > > current logic is > > b = pci_alloc_bus(); > if (!b) > return NULL; > > b->sysdata = sysdata; > b->ops = ops; > b->number = b->busn_res.start = bus; > b2 = pci_find_bus(pci_domain_nr(b), bus); > if (b2) { > /* If we already got to this bus through a different > bridge, ignore it */ > dev_dbg(&b2->dev, "bus already known\n"); > goto err_out; > } > > bridge = pci_alloc_host_bridge(b); > > We need to find the duplicated hostbridge or racing. > > If we need call pci_alloc_host_bridge at first, we will need my > for_each_host_bridge patchset, and use it to check if there is > existing hostbridge. > >> >>> looks like we don't need to split device_register to device_del and put_device. >>> >>> we can make root bus removal to use device_unregister too, in the patches, >> >> If we can use device_register()/device_unregister() directly instead of >> splitting them into initialize/add/del/put, that would be awesome. >> >> Anyway, http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3 >> is the current state of what I've got. If you want to iterate on this some >> more and improve things, that's great. But please start with what's on my >> branch and post your updates as a complete v4 because I did tweak some >> changelogs and code formatting, and I don't want to lose that work. > > ok, please consider drop > commit d751727a1792262cb464d8d9608500facb763301 > PCI: Make host bridge/bus creating and destroying logic symmetric > from pci/jiang-bus-lock-v3, and apply my three patches at > > https://patchwork.kernel.org/patch/2562431/ > [1/7] PCI: move back pci_proc_attach_devices calling > https://patchwork.kernel.org/patch/2562461/ > [2/7] PCI: move resources and bus_list releasing to pci_release_dev > https://patchwork.kernel.org/patch/2562451/ > [3/7] PCI: Detach driver in pci_stop_device I dropped the "Make host bridge/bus creating and destroying logic symmetric" patch for now. I don't think the subsequent patches depend on that one, so I merged the rest to my -next branch for v3.11, so we can make at least a tiny bit of progress in this cycle. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat 15 Jun 2013 07:53:48 AM CST, Bjorn Helgaas wrote: > On Sat, Jun 8, 2013 at 1:01 AM, Yinghai Lu <yinghai@kernel.org> wrote: >> On Fri, Jun 7, 2013 at 6:51 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>>> >>>> should split pci_release_host_bridge_dev renaming and moving to another patch. >>> >>> I split the rename/move to another patch and added both to my >>> pci/jiang-bus-lock-v3 branch. >> >> good. >> >>> >>>>> @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct .. >> ... >>> >>> I think this function would be better if we created the host bridge >>> first, then the bus. But your patch is an improvement, so we can look >>> at doing that later. >> >> Yes, that will more straightforward, create hostbidge before root bus, >> like we have pci bridge before subordinate bus under it. >> >> current logic is >> >> b = pci_alloc_bus(); >> if (!b) >> return NULL; >> >> b->sysdata = sysdata; >> b->ops = ops; >> b->number = b->busn_res.start = bus; >> b2 = pci_find_bus(pci_domain_nr(b), bus); >> if (b2) { >> /* If we already got to this bus through a different >> bridge, ignore it */ >> dev_dbg(&b2->dev, "bus already known\n"); >> goto err_out; >> } >> >> bridge = pci_alloc_host_bridge(b); >> >> We need to find the duplicated hostbridge or racing. >> >> If we need call pci_alloc_host_bridge at first, we will need my >> for_each_host_bridge patchset, and use it to check if there is >> existing hostbridge. >> >>> >>>> looks like we don't need to split device_register to device_del and put_device. >>>> >>>> we can make root bus removal to use device_unregister too, in the patches, >>> >>> If we can use device_register()/device_unregister() directly instead of >>> splitting them into initialize/add/del/put, that would be awesome. >>> >>> Anyway, http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3 >>> is the current state of what I've got. If you want to iterate on this some >>> more and improve things, that's great. But please start with what's on my >>> branch and post your updates as a complete v4 because I did tweak some >>> changelogs and code formatting, and I don't want to lose that work. >> >> ok, please consider drop >> commit d751727a1792262cb464d8d9608500facb763301 >> PCI: Make host bridge/bus creating and destroying logic symmetric >> from pci/jiang-bus-lock-v3, and apply my three patches at >> >> https://patchwork.kernel.org/patch/2562431/ >> [1/7] PCI: move back pci_proc_attach_devices calling >> https://patchwork.kernel.org/patch/2562461/ >> [2/7] PCI: move resources and bus_list releasing to pci_release_dev >> https://patchwork.kernel.org/patch/2562451/ >> [3/7] PCI: Detach driver in pci_stop_device > > I dropped the "Make host bridge/bus creating and destroying logic > symmetric" patch for now. I don't think the subsequent patches depend > on that one, so I merged the rest to my -next branch for v3.11, so we > can make at least a tiny bit of progress in this cycle. > > Bjorn Hi Bjorn, Sorry for the delay, busy with the dock station regression and bugfix these days, will try to find time slot this weekend to handle this. Regards! Gerry -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 2f81a0a..9e9cdfe 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -470,7 +470,7 @@ void pci_read_bridge_bases(struct pci_bus *child) } } -static struct pci_bus * pci_alloc_bus(void) +static struct pci_bus *pci_alloc_bus(struct pci_ops *ops, void *sd, int bus) { struct pci_bus *b; @@ -483,10 +483,32 @@ static struct pci_bus * pci_alloc_bus(void) INIT_LIST_HEAD(&b->resources); b->max_bus_speed = PCI_SPEED_UNKNOWN; b->cur_bus_speed = PCI_SPEED_UNKNOWN; + b->sysdata = sd; + b->ops = ops; + b->number = bus; + b->busn_res.start = bus; + b->busn_res.end = 0xff; + b->busn_res.flags = IORESOURCE_BUS; + b->dev.class = &pcibus_class; + dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus); + device_initialize(&b->dev); } + return b; } +static void pci_release_host_bridge_dev(struct device *dev) +{ + struct pci_host_bridge *bridge = to_pci_host_bridge(dev); + + if (bridge->release_fn) + bridge->release_fn(bridge); + + pci_free_resource_list(&bridge->windows); + + kfree(bridge); +} + static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b) { struct pci_host_bridge *bridge; @@ -495,6 +517,10 @@ static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b) if (bridge) { INIT_LIST_HEAD(&bridge->windows); bridge->bus = b; + bridge->dev.release = pci_release_host_bridge_dev; + dev_set_name(&bridge->dev, "pci%04x:%02x", + pci_domain_nr(b), b->number); + device_initialize(&bridge->dev); } return bridge; @@ -647,28 +673,13 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, /* * Allocate a new bus, and inherit stuff from the parent.. */ - child = pci_alloc_bus(); + child = pci_alloc_bus(parent->ops, parent->sysdata, busnr); if (!child) return NULL; child->parent = parent; - child->ops = parent->ops; - child->sysdata = parent->sysdata; child->bus_flags = parent->bus_flags; - - /* initialize some portions of the bus device, but don't register it - * now as the parent is not properly set up yet. - */ - child->dev.class = &pcibus_class; - dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr); - - /* - * Set up the primary, secondary and subordinate - * bus numbers. - */ - child->number = child->busn_res.start = busnr; child->primary = parent->busn_res.start; - child->busn_res.end = 0xff; if (!bridge) { child->dev.parent = parent->bridge; @@ -689,7 +700,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, bridge->subordinate = child; add_dev: - ret = device_register(&child->dev); + ret = device_add(&child->dev); WARN_ON(ret < 0); pcibios_add_bus(child); @@ -1208,18 +1219,6 @@ int pci_cfg_space_size(struct pci_dev *dev) return PCI_CFG_SPACE_SIZE; } -static void pci_release_bus_bridge_dev(struct device *dev) -{ - struct pci_host_bridge *bridge = to_pci_host_bridge(dev); - - if (bridge->release_fn) - bridge->release_fn(bridge); - - pci_free_resource_list(&bridge->windows); - - kfree(bridge); -} - struct pci_dev *pci_alloc_dev(struct pci_bus *bus) { struct pci_dev *dev; @@ -1707,13 +1706,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, char bus_addr[64]; char *fmt; - b = pci_alloc_bus(); + b = pci_alloc_bus(ops, sysdata, bus); if (!b) return NULL; - b->sysdata = sysdata; - b->ops = ops; - b->number = b->busn_res.start = bus; b2 = pci_find_bus(pci_domain_nr(b), bus); if (b2) { /* If we already got to this bus through a different bridge, ignore it */ @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, goto err_out; bridge->dev.parent = parent; - bridge->dev.release = pci_release_bus_bridge_dev; - dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); error = pcibios_root_bridge_prepare(bridge); - if (error) { - kfree(bridge); - goto err_out; - } + if (error) + goto bridge_dev_reg_err; + + error = device_add(&bridge->dev); + if (error) + goto bridge_dev_reg_err; - error = device_register(&bridge->dev); - if (error) { - kfree(bridge); - goto err_out; - } b->bridge = get_device(&bridge->dev); device_enable_async_suspend(b->bridge); pci_set_bus_of_node(b); - if (!parent) set_dev_node(b->bridge, pcibus_to_node(b)); - - b->dev.class = &pcibus_class; b->dev.parent = b->bridge; - dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus); - error = device_register(&b->dev); + error = device_add(&b->dev); if (error) goto class_dev_reg_err; @@ -1792,10 +1779,11 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, return b; class_dev_reg_err: + device_del(&bridge->dev); +bridge_dev_reg_err: put_device(&bridge->dev); - device_unregister(&bridge->dev); err_out: - kfree(b); + put_device(&b->dev); return NULL; } diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c index 8fc54b7..b0ce875 100644 --- a/drivers/pci/remove.c +++ b/drivers/pci/remove.c @@ -52,7 +52,8 @@ void pci_remove_bus(struct pci_bus *bus) up_write(&pci_bus_sem); pci_remove_legacy_files(bus); pcibios_remove_bus(bus); - device_unregister(&bus->dev); + device_del(&bus->dev); + put_device(&bus->dev); } EXPORT_SYMBOL(pci_remove_bus);
This patch makes PCI host bridge/bus creating and destroying logic symmetric by using device_initialize()/device_add()/device_del()/put_device() pairs as discussed in thread at: http://comments.gmane.org/gmane.linux.kernel.pci/22073 It also fixes a bug in error recovery path in pci_create_root_bus() which may kfree() an in-use host bridge object. Signed-off-by: Jiang Liu <jiang.liu@huawei.com> Cc: Yinghai Lu <yinghai@kernel.org> --- drivers/pci/probe.c | 92 +++++++++++++++++++++++----------------------------- drivers/pci/remove.c | 3 +- 2 files changed, 42 insertions(+), 53 deletions(-)