On Sat, Sep 01, 2018 at 10:20:11AM -0700, Lukas Wunner wrote:
> On Fri, Aug 31, 2018 at 03:26:27PM -0600, Keith Busch wrote:
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > +int pci_bus_error_reset(struct pci_dev *bridge)
> > +{
> > + struct pci_bus *bus = bridge->subordinate;
> > +
> > + if (!bus)
> > + return -ENOTTY;
> > +
> > + if (!list_empty(&bus->slots)) {
> > + struct pci_slot *slot;
> > +
> > + list_for_each_entry(slot, &bus->slots, list)
> > + if (pci_probe_reset_slot(slot))
> > + goto bus_reset;
> > + list_for_each_entry(slot, &bus->slots, list)
> > + if (pci_slot_reset(slot, 0))
> > + goto bus_reset;
> > + return 0;
> > + }
>
> I don't see any locking to protect the slots list access. It seems
> pci_slot_mutex is meant to serve this purpose, but it's scoped to slot.c.
> Which begs the question if this function should live there as well?
Thanks for pointing that out. I think we ought to have all the needed
locking between pci_bus_sem and pci_rescan_remove_lock. I'll see if we
can make pci_slot_mutex unnecessary.
