[PATCH v2 01/11] powerpc/powernv/ioda: Fix ref count for devices with their own PE

[Frederic Barrat]

Le 08/01/2020 à 08:33, Andrew Donnellan a écrit :
> On 22/11/19 12:49 am, Frederic Barrat wrote:
>> The pci_dn structure used to store a pointer to the struct pci_dev, so
>> taking a reference on the device was required. However, the pci_dev
>> pointer was later removed from the pci_dn structure, but the reference
>> was kept for the npu device.
>> See commit 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary
>> pcidev from pci_dn").
>>
>> We don't need to take a reference on the device when assigning the PE
>> as the struct pnv_ioda_pe is cleaned up at the same time as
>> the (physical) device is released. Doing so prevents the device from
>> being released, which is a problem for opencapi devices, since we want
>> to be able to remove them through PCI hotplug.
>>
>> Now the ugly part: nvlink npu devices are not meant to be
>> released. Because of the above, we've always leaked a reference and
>> simply removing it now is dangerous and would likely require more
>> work. There's currently no release device callback for nvlink devices
>> for example. So to be safe, this patch leaks a reference on the npu
>> device, but only for nvlink and not opencapi.
>>
>> CC: aik at ozlabs.ru
>> CC: oohall at gmail.com
>> Signed-off-by: Frederic Barrat <fbarrat at linux.ibm.com>
> 
> It took me a while to parse exactly what you're doing here.
> 
> - In pnv_ioda_setup_dev_PE(), we take a reference on the pci_dev, this 
> is to protect a pointer in the pci_dn structure, but not to protect the 
> pointer in the pci_dev structure (which doesn't need to be protected by 
> taking a reference, because the lifetime of the pnv_ioda_pe is the same 
> as the pci_dev).
> 
> - The pointer in the pci_dn structure has now been removed, so we should 
> remove the pci_dev_get() accordingly.


Correct. Did I do such a bad job explaining it in the commit message 
that I need to rephrase?

   Fred


> This seems okay to me, though anything PE-related is irritatingly hairy 
> so one can never be truly certain...
> 
> Reviewed-by: Andrew Donnellan <ajd at linux.ibm.com>
>