| Message ID | 1348243658-5127-1-git-send-email-jiang.liu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Fri, Sep 21, 2012 at 10:07 AM, Jiang Liu <liuj97@gmail.com> wrote: > The pci_find_next_bus() is not hotplug safe, so introduce PCI hotplug > safe interfaces to walk PCI buses. To avoid some deadlock scenarios, > two interfaces are introduced. > > The first one is pci_for_each_bus(), which walks all PCI buses holding > read lock on the pci_bus_sem. > > The second one is pci_for_each_started_bus(), which walks all started > PCI buses without holding any global locks. Started PCI buses are those > which have been added to the device tree by calling device_add(). > --- > Hi Bjorn, > How about this PCI bus iterator design? It's a little ugly that > we need to two interfaces to work around some deadlock scenarios. > And I plan to split the task into two parts: > 1) a hotplug safe PCI bus iteraror to replace pci_find_next_bus > 2) handle hotplug notifications to update bus related states. > > My patchset at http://www.spinics.net/lists/linux-pci/msg17515.html has > patially solved issue 2 above for x86/ACPI, and will add more supports > for other platforms. > Thanks! > Gerry I like this interface: int pci_for_each_bus(int (* cb)(struct pci_bus *, void *), void *data) quite a bit because it has the potential for removing all the list knowledge from the callers, but there are two things I don't like: 1) The interface would allow the PCI core to call the callback for future hot-added buses, but your implementation doesn't have that yet. 2) I'd rather have something like "pci_for_each_dev()" so this is device-based instead of bus-based. The struct pci_bus is of limited usefulness outside the core, except as a container for a set of devices. So the users of pci_for_each_bus() would often iterate through that set of devices, and given the complications of SR-IOV virtual buses, I don't think it's really clear how to do that iteration correctly. > --- > drivers/pci/bus.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/pci.h | 1 + > 2 files changed, 43 insertions(+) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index e16a8f0f..21b0ade 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -327,6 +327,48 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), > } > EXPORT_SYMBOL_GPL(pci_walk_bus); > > +static int pci_bus_iter(struct pci_bus *bus, > + int (* cb)(struct pci_bus *, void *), void *data) > +{ > + int rc; > + > + rc = cb(bus, data); > + if (rc == 0) > + list_for_each_entry(bus, &bus->children, node) { > + rc = pci_bus_iter(bus, cb, data); > + if (rc) > + break; > + } > + > + return rc; > +} > + > +/** pci_for_each_bus - walk all PCI buses and call the provided callback. > + * @cb callback to be called for each bus found > + * @data arbitrary pointer to be passed to callback. > + * > + * Walk all PCI buses and call the provided callback with pci_bus_sem held. > + * > + * We check the return of @cb each time. If it returns anything > + * other than 0, we break out. > + */ > +int pci_for_each_bus(int (* cb)(struct pci_bus *, void *), void *data) > +{ > + int rc = 0; > + struct pci_bus *bus; > + > + down_read(&pci_bus_sem); > + list_for_each_entry(bus, &pci_root_buses, node) { > + rc = pci_bus_iter(bus, cb, data); > + if (rc) > + break; > + } > + up_read(&pci_bus_sem); > + > + return rc; > +} > +EXPORT_SYMBOL(pci_for_each_bus); > + > struct pci_bus *pci_bus_get(struct pci_bus *bus) > { > if (bus) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 3c5017d..1423a24 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1060,6 +1060,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, > > void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), > void *userdata); > +int pci_for_each_bus(int (* cb)(struct pci_bus *, void *), void *data); > int pci_cfg_space_size_ext(struct pci_dev *dev); > int pci_cfg_space_size(struct pci_dev *dev); > unsigned char pci_bus_max_busnr(struct pci_bus *bus); > -- > 1.7.9.5 > -- 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/bus.c b/drivers/pci/bus.c index e16a8f0f..21b0ade 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -327,6 +327,48 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), } EXPORT_SYMBOL_GPL(pci_walk_bus); +static int pci_bus_iter(struct pci_bus *bus, + int (* cb)(struct pci_bus *, void *), void *data) +{ + int rc; + + rc = cb(bus, data); + if (rc == 0) + list_for_each_entry(bus, &bus->children, node) { + rc = pci_bus_iter(bus, cb, data); + if (rc) + break; + } + + return rc; +} + +/** pci_for_each_bus - walk all PCI buses and call the provided callback. + * @cb callback to be called for each bus found + * @data arbitrary pointer to be passed to callback. + * + * Walk all PCI buses and call the provided callback with pci_bus_sem held. + * + * We check the return of @cb each time. If it returns anything + * other than 0, we break out. + */ +int pci_for_each_bus(int (* cb)(struct pci_bus *, void *), void *data) +{ + int rc = 0; + struct pci_bus *bus; + + down_read(&pci_bus_sem); + list_for_each_entry(bus, &pci_root_buses, node) { + rc = pci_bus_iter(bus, cb, data); + if (rc) + break; + } + up_read(&pci_bus_sem); + + return rc; +} +EXPORT_SYMBOL(pci_for_each_bus); + struct pci_bus *pci_bus_get(struct pci_bus *bus) { if (bus) diff --git a/include/linux/pci.h b/include/linux/pci.h index 3c5017d..1423a24 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1060,6 +1060,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), void *userdata); +int pci_for_each_bus(int (* cb)(struct pci_bus *, void *), void *data); int pci_cfg_space_size_ext(struct pci_dev *dev); int pci_cfg_space_size(struct pci_dev *dev); unsigned char pci_bus_max_busnr(struct pci_bus *bus);