| Message ID | 20240827192826.710031-5-kbusch@meta.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pci cleanup/prep patches | expand |
On Tue, 27 Aug 2024, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > The original implementation purposefully chose a non-recursive walk, > presumably as a precaution on stack use. We do recursive bus walking in > other places though. For example: > > pci_bus_resettable > pci_stop_bus_device > pci_remove_bus_device > pci_bus_allocate_dev_resources > > So, recursive pci bus walking is well tested and safe, and the > implementation is easier to follow. The motivation for changing it now > is to make it easier to introduce finer grain locking in the future. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Keith Busch <kbusch@kernel.org> > --- > drivers/pci/bus.c | 36 +++++++++++------------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 7c07a141e8772..8491e9c7f0586 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -389,37 +389,23 @@ void pci_bus_add_devices(const struct pci_bus *bus) > } > EXPORT_SYMBOL(pci_bus_add_devices); > > -static void __pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), > - void *userdata) > +static int __pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), I guess the reason why this parameter was called "top" is now gone so you could just name it "bus"? Other than that, the change looks okay, Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 7c07a141e8772..8491e9c7f0586 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -389,37 +389,23 @@ void pci_bus_add_devices(const struct pci_bus *bus) } EXPORT_SYMBOL(pci_bus_add_devices); -static void __pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), - void *userdata) +static int __pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), + void *userdata) { struct pci_dev *dev; - struct pci_bus *bus; - struct list_head *next; - int retval; + int ret = 0; - bus = top; - next = top->devices.next; - for (;;) { - if (next == &bus->devices) { - /* end of this bus, go up or finish */ - if (bus == top) + list_for_each_entry(dev, &top->devices, bus_list) { + ret = cb(dev, userdata); + if (ret) + break; + if (dev->subordinate) { + ret = __pci_walk_bus(dev->subordinate, cb, userdata); + if (ret) break; - next = bus->self->bus_list.next; - bus = bus->self->bus; - continue; } - dev = list_entry(next, struct pci_dev, bus_list); - if (dev->subordinate) { - /* this is a pci-pci bridge, do its devices next */ - next = dev->subordinate->devices.next; - bus = dev->subordinate; - } else - next = dev->bus_list.next; - - retval = cb(dev, userdata); - if (retval) - break; } + return ret; } /**