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

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2018-08-16 at 20:10 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2018-08-16 at 14:52 +0530, Hari Vyas wrote:
> > 
> > There was an issue reported by my colleague srinath while enabling pci
> > bridge and a race
> > condition was happening while setting memory and master bits i.e. bits
> > were over-written.
> > As per my understanding is_busmaster and is_added bit race issue was
> > at internal data
> > management and is quite different from pci bridge enabling issue.
> > Am I missing some thing ? Would be interested to know what exactly was
> > affected due to
> > is_busmaster fix.
> 
> The is_busmaster fix isn't I think affecting anything, however I don't
> like the use of atomics for these things. It's a band-aid. If we grow a
> proper pci_dev mutex, which is what I'm introducing here, it should be
> able to also handle the is_added race etc..
> 
> > In any case, one bug is already filed and may propose a patch soon
> > about pci bridge enabling scenario.
> 
> I already did, see the patch I posted earlier.

https://lore.kernel.org/lkml/08bc40a6af3e614e97a78fbaab688bfcd14520ac.camel@kernel.crashing.org/

> >  In summary, bit manipulation is not working fine due to race
> > conditions in SMP
> > environment.
> 
> Right, among other things.
> 
> My proposed patch (which I will try to break down tomorrow or next week
> into smaller bits) introduces a per-pci_dev mutex and uses it to fix
> the enable & set_master races.
> 
> My proposal is to then progressively move more things under the
> umbrella of that mutex in order to properly protect the pci_dev
> internal state.
> 
> Cheers,
> Ben.
> 
> > Regards,
> > hari