From: Frederic Barrat <redacted>
[ Upstream commit 05dd7da76986937fb288b4213b1fa10dbe0d1b33 ]
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.
Signed-off-by: Frederic Barrat <redacted>
Reviewed-by: Andrew Donnellan <redacted>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191121134918.7155-2-fbarrat@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
@@ -1062,14 +1062,13 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)returnNULL;}-/* NOTE: We get only one ref to the pci_dev for the pdn, not for the-*pointerinthePEdatastructure,bothshouldbedestroyedatthe-*sametime.However,thisneedstobelookedatmorecloselyagain-*onceweactuallystartremovingthings(Hotplug,SR-IOV,...)+/* NOTE: We don't get a reference for the pointer in the PE+*datastructure,boththedeviceandPEstructuresshouldbe+*destroyedatthesametime.However,removingnvlink+*deviceswillneedsomework.**AtsomepointwewanttoremovethePDNcompletelyanyways*/-pci_dev_get(dev);pdn->pe_number=pe->pe_number;pe->flags=PNV_IODA_PE_DEV;pe->pdev=dev;
This patch is not desirable for stable, for 5.4 and 4.19 (it was already
flagged by autosel back in April. Not sure why it's showing again now)
Fred
quoted hunk
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.
Signed-off-by: Frederic Barrat <redacted>
Reviewed-by: Andrew Donnellan <redacted>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191121134918.7155-2-fbarrat@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
@@ -1062,14 +1062,13 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)returnNULL;}-/* NOTE: We get only one ref to the pci_dev for the pdn, not for the-*pointerinthePEdatastructure,bothshouldbedestroyedatthe-*sametime.However,thisneedstobelookedatmorecloselyagain-*onceweactuallystartremovingthings(Hotplug,SR-IOV,...)+/* NOTE: We don't get a reference for the pointer in the PE+*datastructure,boththedeviceandPEstructuresshouldbe+*destroyedatthesametime.However,removingnvlink+*deviceswillneedsomework.**AtsomepointwewanttoremovethePDNcompletelyanyways*/-pci_dev_get(dev);pdn->pe_number=pe->pe_number;pe->flags=PNV_IODA_PE_DEV;pe->pdev=dev;
This patch is not desirable for stable, for 5.4 and 4.19 (it was
already flagged by autosel back in April. Not sure why it's showing
again now)
Hey Fred,
This was a bit of a "lie", it wasn't a run of AUTOSEL, but rather an
audit of patches that went into distro/vendor trees but not into the
upstream stable trees.
I can see that this patch was pulled into Ubuntu's 5.4 tree, is it not
needed in the upstream stable tree?
--
Thanks,
Sasha
This patch is not desirable for stable, for 5.4 and 4.19 (it was
already flagged by autosel back in April. Not sure why it's showing
again now)
Hey Fred,
This was a bit of a "lie", it wasn't a run of AUTOSEL, but rather an
audit of patches that went into distro/vendor trees but not into the
upstream stable trees.
I can see that this patch was pulled into Ubuntu's 5.4 tree, is it not
needed in the upstream stable tree?
That patch in itself is useless (it replaces a ref counter leak by
another one). It was part of a longer series that we backported to
Ubuntu's 5.4 tree.
So it's really not needed on the stable trees. It likely wouldn't hurt
or break anything, but there's really no point.
Fred