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: a...@ozlabs.ru
CC: ooh...@gmail.com
Signed-off-by: Frederic Barrat <fbar...@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 <a...@linux.ibm.com>
