| Subject | Re: [PATCH AUTOSEL 5.4 101/330] powerpc/powernv/ioda: Fix ref count for devices with their own PE | From | Frederic Barrat <> | Date | Fri, 18 Sep 2020 08:35:06 +0200 |
| |
Le 18/09/2020 à 03:57, Sasha Levin a écrit : > From: Frederic Barrat <fbarrat@linux.ibm.com> > > [ Upstream commit 05dd7da76986937fb288b4213b1fa10dbe0d1b33 ] >
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
> 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 <fbarrat@linux.ibm.com> > Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> > 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(-) > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index 058223233088e..e9cda7e316a50 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -1062,14 +1062,13 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) > return NULL; > } > > - /* NOTE: We get only one ref to the pci_dev for the pdn, not for the > - * pointer in the PE data structure, both should be destroyed at the > - * same time. However, this needs to be looked at more closely again > - * once we actually start removing things (Hotplug, SR-IOV, ...) > + /* NOTE: We don't get a reference for the pointer in the PE > + * data structure, both the device and PE structures should be > + * destroyed at the same time. However, removing nvlink > + * devices will need some work. > * > * At some point we want to remove the PDN completely anyways > */ > - pci_dev_get(dev); > pdn->pe_number = pe->pe_number; > pe->flags = PNV_IODA_PE_DEV; > pe->pdev = dev; > @@ -1084,7 +1083,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) > pnv_ioda_free_pe(pe); > pdn->pe_number = IODA_INVALID_PE; > pe->pdev = NULL; > - pci_dev_put(dev); > return NULL; > } > > @@ -1205,6 +1203,14 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev) > struct pci_controller *hose = pci_bus_to_host(npu_pdev->bus); > struct pnv_phb *phb = hose->private_data; > > + /* > + * Intentionally leak a reference on the npu device (for > + * nvlink only; this is not an opencapi path) to make sure it > + * never goes away, as it's been the case all along and some > + * work is needed otherwise. > + */ > + pci_dev_get(npu_pdev); > + > /* > * Due to a hardware errata PE#0 on the NPU is reserved for > * error handling. This means we only have three PEs remaining > @@ -1228,7 +1234,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev) > */ > dev_info(&npu_pdev->dev, > "Associating to existing PE %x\n", pe_num); > - pci_dev_get(npu_pdev); > npu_pdn = pci_get_pdn(npu_pdev); > rid = npu_pdev->bus->number << 8 | npu_pdn->devfn; > npu_pdn->pe_number = pe_num; >
|