Re: PCIe enable device races (Was: [PATCH v3] PCI: Data corruption happening due to race condition)

[Lukas Wunner <lukas@wunner.de>]

On Thu, Aug 16, 2018 at 07:52:06AM +1000, Benjamin Herrenschmidt wrote:
> I would also like to use this rather than the bitmap atomics for is_added
> etc... (Hari's fix) in the long run. Atomics aren't significantly cheaper
> and imho makes thing even messier.

PCI device enablement isn't a hotpath, so the performance saving of
acquire/release memory barriers vs. full memory barriers is hardly
a strong argument.

Atomic bitops have the advantage of being usable in atomic context,
unlike a struct mutex.  And they don't require any spinning if the
variable is accessed concurrently.

Last not least an atomic bitop needs just 1 line of code versus 3 lines
of code to acquire a lock, update the variable and release the lock.
"Elegance is not optional."

That said, the race *you're* dealing with appears to be distinct from
Hari's in that a lock is unavoidable because (AFAIUI) atomic_inc_return()
is called before enablement actually happens, yet both needs to happen
atomically.

Thanks,

Lukas