Re: [PATCH 02/11] powerpc/powernv/ioda: Protect PE list

[Frederic Barrat <fbarrat@linux.ibm.com>]

Le 19/11/2019 à 14:22, Oliver O'Halloran a écrit :
> On Tue, Nov 19, 2019 at 11:57 PM Frederic Barrat <fbarrat@linux.ibm.com> wrote:
>>
>>> Do the other accessors of ioda.pe_list also need mutex protection?
>>> pnv_ioda_setup_bus_PE()
>>> pnv_pci_dma_bus_setup()
>>> pnv_pci_init_ioda_phb()
>>> pnv_pci_ioda_setup_PEs()
>>
>>
>> I think we could also use it there, it wouldn't hurt. Those functions
>> are called when the kernel is building part of the PCI topology, and
>> devices are not really active yet, so I don't think it's absolutely
>> required.
>>
>> I'm actually not sure my patch is needed either. With hotplug, the
>> devices can come and go, whereas the PHB remains. So it feels right to
>> start protecting the list when adding/removing a device. But I don't
>> think we can really have concurrency and have 2 different operations
>> adding/removing devices at the same time under the same PHB, at least
>> for opencapi. Maybe for PCI, if we have multiple slots under the same
>> PHB. Not sure.
> 
> Creation of new pci_dev's is serialised by the global pci rescan and
> remove lock so on the creation side it's not an issue. However, we can
> release IODA PEs in the pci_dev's release function which might be
> called without that lock held. It's pretty hard to hit that case
> though since it require something to be holding a ref to the pci_dev
> even after the driver's ->remove() function has been called.


Thanks for clarifying it! Indeed, the pci_rescan_remove_lock in hotplug 
helps for the PCI case. I guess we can keep this patch for that hard to 
hit case, just in case it doesn't blow up somewhere else.

   Fred