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

[Andrew Donnellan]

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.

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>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd at linux.ibm.com             IBM Australia Limited