On Fri, Aug 17, 2018 at 02:48:58PM +1000, Benjamin Herrenschmidt wrote: > This re-fixes the bug reported by Hari Vyas <hari.v...@broadcom.com> > after my revert of his commit but in a much simpler way. > > The main issues is that is_added was being set after the driver > got bound and started, and thus setting it could race with other > changes to struct pci_dev.
The "bind driver, then set dev->added = 1" order seems to have been there since the beginning of dev->is_added: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8a1bc9013a03 This patch seems reasonable, but I'm a little dubious about the existence of "is_added" in the first place. As far as I can tell, the only other buses with something similar are the MEN Chameleon bus and the Intel Management Engine Interface. The PCI uses of "is_added" don't seem *that* critical or unique to PCI, so I'm not 100% convinced we need it at all. But I haven't looked into it enough to be able to propose an alternative. > This fixes it by setting the flag first, which also has the > advantage of matching the fact that we are clearing it *after* > unbinding in the remove path, thus the flag is now symtetric > and always set while the driver code is running. > > Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> > --- > drivers/pci/bus.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 35b7fc87eac5..48ae63673aa8 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -321,16 +321,16 @@ void pci_bus_add_device(struct pci_dev *dev) > pci_proc_attach_device(dev); > pci_bridge_d3_update(dev); > > + dev->is_added = 1; > dev->match_driver = true; > retval = device_attach(&dev->dev); > if (retval < 0 && retval != -EPROBE_DEFER) { > + dev->is_added = 0; > pci_warn(dev, "device attach failed (%d)\n", retval); > pci_proc_detach_device(dev); > pci_remove_sysfs_dev_files(dev); > return; > } > - > - dev->is_added = 1; > } > EXPORT_SYMBOL_GPL(pci_bus_add_device); > > -- > 2.17.1 >
