On Thu, 2018-07-19 at 20:55 +0200, Lukas Wunner wrote: > On Thu, Jul 19, 2018 at 9:48 AM, Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx wrote: > > Indeed. However I'm not fan of the solution. Shouldn't we instead have > > some locking for the content of pci_dev? I've always been wary of us > > having other similar races in there. > > The solution presented is perfectly fine as it uses atomic bitops which > obviate the need for locking. Why do you want to add unnecessary locking > on top? Atomic bitops tend to be *more* expensive than a lock. My concern is that the PCIe code historically had no locking and I worry we may have other fields in there with similar issues. But maybe I'm wrong. > Certain other parts of struct pci_dev use their own locking, e.g. > pci_bus_sem to protect bus_list. Most elements can and should > be accessed lockless for performance. > > > > > The powerpc PCI code contains a lot of cruft coming from the depth of > > > history, including rather nasty assumptions. We want to progressively > > > clean it up, starting with EEH, but it will take time. > > Then I suggest using the #include "../../../drivers/pci/pci.h" for now > until the powerpc arch code has been consolidated. There's also the need both in powerpc and sparc to access the guts of pci_dev because those archs will "fabricate" as pci_dev from the device-tree rather than probing it under some circumstances. Cheers, Ben.
