Re: [PATCH v3] PCI: Data corruption happening due to race condition

On Thu, 2018-07-19 at 20:55 +0200, Lukas Wunner wrote:

On Thu, Jul 19, 2018 at 9:48 AM, Benjamin Herrenschmidt <benh@kernel.crashing.org wrote:

quoted

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.

quoted

quoted

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.