Message ID | 155568805354.600470.13376593185688810607.stgit@bahia.lan (mailing list archive) |
---|---|
State | Accepted |
Commit | 02c5f5394918b9b47ff4357b1b18335768cd867d |
Headers | show |
Series | powerpc/powernv/npu: Fix reference leak | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (8c2ffd9174779014c3fe1f96d9dc3641d9175f00) |
snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 23 lines checked |
On 20/04/2019 01:34, Greg Kurz wrote: > Since 902bdc57451c, get_pci_dev() calls pci_get_domain_bus_and_slot(). This > has the effect of incrementing the reference count of the PCI device, as > explained in drivers/pci/search.c: > > * Given a PCI domain, bus, and slot/function number, the desired PCI > * device is located in the list of PCI devices. If the device is > * found, its reference count is increased and this function returns a > * pointer to its data structure. The caller must decrement the > * reference count by calling pci_dev_put(). If no device is found, > * %NULL is returned. > > Nothing was done to call pci_dev_put() and the reference count of GPU and > NPU PCI devices rockets up. > > A natural way to fix this would be to teach the callers about the change, > so that they call pci_dev_put() when done with the pointer. This turns > out to be quite intrusive, as it affects many paths in npu-dma.c, > pci-ioda.c and vfio_pci_nvlink2.c. afaict this referencing is only done to protect the current traverser and what you've done is actually a natural way (and the generic pci_get_dev_by_id() does exactly the same), although this looks a bit weird. > Also, the issue appeared in 4.16 and > some affected code got moved around since then: it would be problematic > to backport the fix to stable releases. > > All that code never cared for reference counting anyway. Call pci_dev_put() > from get_pci_dev() to revert to the previous behavior. >> Fixes: 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary pcidev from pci_dn") > Cc: stable@vger.kernel.org # v4.16 > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > arch/powerpc/platforms/powernv/npu-dma.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c > index e713ade30087..d8f3647e8fb2 100644 > --- a/arch/powerpc/platforms/powernv/npu-dma.c > +++ b/arch/powerpc/platforms/powernv/npu-dma.c > @@ -31,9 +31,22 @@ static DEFINE_SPINLOCK(npu_context_lock); > static struct pci_dev *get_pci_dev(struct device_node *dn) > { > struct pci_dn *pdn = PCI_DN(dn); > + struct pci_dev *pdev; > > - return pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), > + pdev = pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), > pdn->busno, pdn->devfn); > + > + /* > + * pci_get_domain_bus_and_slot() increased the reference count of > + * the PCI device, but callers don't need that actually as the PE > + * already holds a reference to the device. Imho this would be just enough. Anyway, Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> How did you find it? :) > Since callers aren't > + * aware of the reference count change, call pci_dev_put() now to > + * avoid leaks. > + */ > + if (pdev) > + pci_dev_put(pdev); > + > + return pdev; > } > > /* Given a NPU device get the associated PCI device. */ >
On Mon, 29 Apr 2019 16:01:29 +1000 Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > On 20/04/2019 01:34, Greg Kurz wrote: > > Since 902bdc57451c, get_pci_dev() calls pci_get_domain_bus_and_slot(). This > > has the effect of incrementing the reference count of the PCI device, as > > explained in drivers/pci/search.c: > > > > * Given a PCI domain, bus, and slot/function number, the desired PCI > > * device is located in the list of PCI devices. If the device is > > * found, its reference count is increased and this function returns a > > * pointer to its data structure. The caller must decrement the > > * reference count by calling pci_dev_put(). If no device is found, > > * %NULL is returned. > > > > Nothing was done to call pci_dev_put() and the reference count of GPU and > > NPU PCI devices rockets up. > > > > A natural way to fix this would be to teach the callers about the change, > > so that they call pci_dev_put() when done with the pointer. This turns > > out to be quite intrusive, as it affects many paths in npu-dma.c, > > pci-ioda.c and vfio_pci_nvlink2.c. > > > afaict this referencing is only done to protect the current traverser > and what you've done is actually a natural way (and the generic > pci_get_dev_by_id() does exactly the same), although this looks a bit weird. > Not exactly the same: pci_get_dev_by_id() always increment the refcount of the returned PCI device. The refcount is only decremented when this device is passed to pci_get_dev_by_id() to continue searching. That means that the users of the PCI device pointer returned by pci_get_dev_by_id() or its exported variants pci_get_subsys(), pci_get_device() and pci_get_class() do handle the refcount. They all pass the pointer to pci_dev_put() or continue the search, which calls pci_dev_put() internally. Direct and indirect callers of get_pci_dev() don't care for the refcount at all unless I'm missing something. > > > Also, the issue appeared in 4.16 and > > some affected code got moved around since then: it would be problematic > > to backport the fix to stable releases. > > > > All that code never cared for reference counting anyway. Call pci_dev_put() > > from get_pci_dev() to revert to the previous behavior. > >> Fixes: 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary pcidev > from pci_dn") > > Cc: stable@vger.kernel.org # v4.16 > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > arch/powerpc/platforms/powernv/npu-dma.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c > > index e713ade30087..d8f3647e8fb2 100644 > > --- a/arch/powerpc/platforms/powernv/npu-dma.c > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c > > @@ -31,9 +31,22 @@ static DEFINE_SPINLOCK(npu_context_lock); > > static struct pci_dev *get_pci_dev(struct device_node *dn) > > { > > struct pci_dn *pdn = PCI_DN(dn); > > + struct pci_dev *pdev; > > > > - return pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), > > + pdev = pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), > > pdn->busno, pdn->devfn); > > + > > + /* > > + * pci_get_domain_bus_and_slot() increased the reference count of > > + * the PCI device, but callers don't need that actually as the PE > > + * already holds a reference to the device. > > Imho this would be just enough. > > Anyway, > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > Thanks ! I now realize that I forgot to add the --cc option for stable on my stgit command line :-\. Cc'ing now. > > How did you find it? :) > While reading code to find some inspiration for OpenCAPI passthrough. :) I saw the following in vfio_pci_ibm_npu2_init(): if (!pnv_pci_get_gpu_dev(vdev->pdev)) return -ENODEV; and simply followed the function calls. > > > Since callers aren't > > + * aware of the reference count change, call pci_dev_put() now to > > + * avoid leaks. > > + */ > > + if (pdev) > > + pci_dev_put(pdev); > > + > > + return pdev; > > } > > > > /* Given a NPU device get the associated PCI device. */ > > >
Michael, Any comments on this patch ? Should I repost with a shorter comment as suggested by Alexey ? Cheers, -- Greg On Mon, 29 Apr 2019 12:36:59 +0200 Greg Kurz <groug@kaod.org> wrote: > On Mon, 29 Apr 2019 16:01:29 +1000 > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > > > On 20/04/2019 01:34, Greg Kurz wrote: > > > Since 902bdc57451c, get_pci_dev() calls pci_get_domain_bus_and_slot(). This > > > has the effect of incrementing the reference count of the PCI device, as > > > explained in drivers/pci/search.c: > > > > > > * Given a PCI domain, bus, and slot/function number, the desired PCI > > > * device is located in the list of PCI devices. If the device is > > > * found, its reference count is increased and this function returns a > > > * pointer to its data structure. The caller must decrement the > > > * reference count by calling pci_dev_put(). If no device is found, > > > * %NULL is returned. > > > > > > Nothing was done to call pci_dev_put() and the reference count of GPU and > > > NPU PCI devices rockets up. > > > > > > A natural way to fix this would be to teach the callers about the change, > > > so that they call pci_dev_put() when done with the pointer. This turns > > > out to be quite intrusive, as it affects many paths in npu-dma.c, > > > pci-ioda.c and vfio_pci_nvlink2.c. > > > > > > afaict this referencing is only done to protect the current traverser > > and what you've done is actually a natural way (and the generic > > pci_get_dev_by_id() does exactly the same), although this looks a bit weird. > > > > Not exactly the same: pci_get_dev_by_id() always increment the refcount > of the returned PCI device. The refcount is only decremented when this > device is passed to pci_get_dev_by_id() to continue searching. > > That means that the users of the PCI device pointer returned by > pci_get_dev_by_id() or its exported variants pci_get_subsys(), > pci_get_device() and pci_get_class() do handle the refcount. They > all pass the pointer to pci_dev_put() or continue the search, > which calls pci_dev_put() internally. > > Direct and indirect callers of get_pci_dev() don't care for the > refcount at all unless I'm missing something. > > > > > > Also, the issue appeared in 4.16 and > > > some affected code got moved around since then: it would be problematic > > > to backport the fix to stable releases. > > > > > > All that code never cared for reference counting anyway. Call pci_dev_put() > > > from get_pci_dev() to revert to the previous behavior. > > >> Fixes: 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary pcidev > > from pci_dn") > > > Cc: stable@vger.kernel.org # v4.16 > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > > --- > > > arch/powerpc/platforms/powernv/npu-dma.c | 15 ++++++++++++++- > > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c > > > index e713ade30087..d8f3647e8fb2 100644 > > > --- a/arch/powerpc/platforms/powernv/npu-dma.c > > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c > > > @@ -31,9 +31,22 @@ static DEFINE_SPINLOCK(npu_context_lock); > > > static struct pci_dev *get_pci_dev(struct device_node *dn) > > > { > > > struct pci_dn *pdn = PCI_DN(dn); > > > + struct pci_dev *pdev; > > > > > > - return pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), > > > + pdev = pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), > > > pdn->busno, pdn->devfn); > > > + > > > + /* > > > + * pci_get_domain_bus_and_slot() increased the reference count of > > > + * the PCI device, but callers don't need that actually as the PE > > > + * already holds a reference to the device. > > > > Imho this would be just enough. > > > > Anyway, > > > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > > > Thanks ! > > I now realize that I forgot to add the --cc option for stable on my stgit > command line :-\. > > Cc'ing now. > > > > > How did you find it? :) > > > > While reading code to find some inspiration for OpenCAPI passthrough. :) > > I saw the following in vfio_pci_ibm_npu2_init(): > > if (!pnv_pci_get_gpu_dev(vdev->pdev)) > return -ENODEV; > > and simply followed the function calls. > > > > > > Since callers aren't > > > + * aware of the reference count change, call pci_dev_put() now to > > > + * avoid leaks. > > > + */ > > > + if (pdev) > > > + pci_dev_put(pdev); > > > + > > > + return pdev; > > > } > > > > > > /* Given a NPU device get the associated PCI device. */ > > > > > >
Greg Kurz <groug@kaod.org> writes: > Michael, > > Any comments on this patch ? Should I repost with a shorter comment > as suggested by Alexey ? No the longer comment seems fine to me. I'm not a big fan of the patch, it's basically a hack :) But for a backportable fix I guess it is OK. I would be happier though if we eventually fix up the code to do the refcounting properly. cheers > On Mon, 29 Apr 2019 12:36:59 +0200 > Greg Kurz <groug@kaod.org> wrote: >> On Mon, 29 Apr 2019 16:01:29 +1000 >> Alexey Kardashevskiy <aik@ozlabs.ru> wrote: >> >> > On 20/04/2019 01:34, Greg Kurz wrote: >> > > Since 902bdc57451c, get_pci_dev() calls pci_get_domain_bus_and_slot(). This >> > > has the effect of incrementing the reference count of the PCI device, as >> > > explained in drivers/pci/search.c: >> > > >> > > * Given a PCI domain, bus, and slot/function number, the desired PCI >> > > * device is located in the list of PCI devices. If the device is >> > > * found, its reference count is increased and this function returns a >> > > * pointer to its data structure. The caller must decrement the >> > > * reference count by calling pci_dev_put(). If no device is found, >> > > * %NULL is returned. >> > > >> > > Nothing was done to call pci_dev_put() and the reference count of GPU and >> > > NPU PCI devices rockets up. >> > > >> > > A natural way to fix this would be to teach the callers about the change, >> > > so that they call pci_dev_put() when done with the pointer. This turns >> > > out to be quite intrusive, as it affects many paths in npu-dma.c, >> > > pci-ioda.c and vfio_pci_nvlink2.c. >> > >> > >> > afaict this referencing is only done to protect the current traverser >> > and what you've done is actually a natural way (and the generic >> > pci_get_dev_by_id() does exactly the same), although this looks a bit weird. >> > >> >> Not exactly the same: pci_get_dev_by_id() always increment the refcount >> of the returned PCI device. The refcount is only decremented when this >> device is passed to pci_get_dev_by_id() to continue searching. >> >> That means that the users of the PCI device pointer returned by >> pci_get_dev_by_id() or its exported variants pci_get_subsys(), >> pci_get_device() and pci_get_class() do handle the refcount. They >> all pass the pointer to pci_dev_put() or continue the search, >> which calls pci_dev_put() internally. >> >> Direct and indirect callers of get_pci_dev() don't care for the >> refcount at all unless I'm missing something. >> >> > >> > > Also, the issue appeared in 4.16 and >> > > some affected code got moved around since then: it would be problematic >> > > to backport the fix to stable releases. >> > > >> > > All that code never cared for reference counting anyway. Call pci_dev_put() >> > > from get_pci_dev() to revert to the previous behavior. >> > >> Fixes: 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary pcidev >> > from pci_dn") >> > > Cc: stable@vger.kernel.org # v4.16 >> > > Signed-off-by: Greg Kurz <groug@kaod.org> >> > > --- >> > > arch/powerpc/platforms/powernv/npu-dma.c | 15 ++++++++++++++- >> > > 1 file changed, 14 insertions(+), 1 deletion(-) >> > > >> > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c >> > > index e713ade30087..d8f3647e8fb2 100644 >> > > --- a/arch/powerpc/platforms/powernv/npu-dma.c >> > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c >> > > @@ -31,9 +31,22 @@ static DEFINE_SPINLOCK(npu_context_lock); >> > > static struct pci_dev *get_pci_dev(struct device_node *dn) >> > > { >> > > struct pci_dn *pdn = PCI_DN(dn); >> > > + struct pci_dev *pdev; >> > > >> > > - return pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), >> > > + pdev = pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), >> > > pdn->busno, pdn->devfn); >> > > + >> > > + /* >> > > + * pci_get_domain_bus_and_slot() increased the reference count of >> > > + * the PCI device, but callers don't need that actually as the PE >> > > + * already holds a reference to the device. >> > >> > Imho this would be just enough. >> > >> > Anyway, >> > >> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> > >> >> Thanks ! >> >> I now realize that I forgot to add the --cc option for stable on my stgit >> command line :-\. >> >> Cc'ing now. >> >> > >> > How did you find it? :) >> > >> >> While reading code to find some inspiration for OpenCAPI passthrough. :) >> >> I saw the following in vfio_pci_ibm_npu2_init(): >> >> if (!pnv_pci_get_gpu_dev(vdev->pdev)) >> return -ENODEV; >> >> and simply followed the function calls. >> >> > >> > > Since callers aren't >> > > + * aware of the reference count change, call pci_dev_put() now to >> > > + * avoid leaks. >> > > + */ >> > > + if (pdev) >> > > + pci_dev_put(pdev); >> > > + >> > > + return pdev; >> > > } >> > > >> > > /* Given a NPU device get the associated PCI device. */ >> > > >> > >>
On Tue, 14 May 2019 21:13:40 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote: > Greg Kurz <groug@kaod.org> writes: > > Michael, > > > > Any comments on this patch ? Should I repost with a shorter comment > > as suggested by Alexey ? > > No the longer comment seems fine to me. > > I'm not a big fan of the patch, it's basically a hack :) > Yeah :) > But for a backportable fix I guess it is OK. > > I would be happier though if we eventually fix up the code to do the > refcounting properly. > I had started to do just that before deciding to go for the backportable hack. Should I rebase the other patches I have on top of this patch and repost the whole thing, so that we have both the ugly fix for stable and the pretty one for 5.2 ? Cheers, -- Greg > cheers > > > On Mon, 29 Apr 2019 12:36:59 +0200 > > Greg Kurz <groug@kaod.org> wrote: > >> On Mon, 29 Apr 2019 16:01:29 +1000 > >> Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > >> > >> > On 20/04/2019 01:34, Greg Kurz wrote: > >> > > Since 902bdc57451c, get_pci_dev() calls pci_get_domain_bus_and_slot(). This > >> > > has the effect of incrementing the reference count of the PCI device, as > >> > > explained in drivers/pci/search.c: > >> > > > >> > > * Given a PCI domain, bus, and slot/function number, the desired PCI > >> > > * device is located in the list of PCI devices. If the device is > >> > > * found, its reference count is increased and this function returns a > >> > > * pointer to its data structure. The caller must decrement the > >> > > * reference count by calling pci_dev_put(). If no device is found, > >> > > * %NULL is returned. > >> > > > >> > > Nothing was done to call pci_dev_put() and the reference count of GPU and > >> > > NPU PCI devices rockets up. > >> > > > >> > > A natural way to fix this would be to teach the callers about the change, > >> > > so that they call pci_dev_put() when done with the pointer. This turns > >> > > out to be quite intrusive, as it affects many paths in npu-dma.c, > >> > > pci-ioda.c and vfio_pci_nvlink2.c. > >> > > >> > > >> > afaict this referencing is only done to protect the current traverser > >> > and what you've done is actually a natural way (and the generic > >> > pci_get_dev_by_id() does exactly the same), although this looks a bit weird. > >> > > >> > >> Not exactly the same: pci_get_dev_by_id() always increment the refcount > >> of the returned PCI device. The refcount is only decremented when this > >> device is passed to pci_get_dev_by_id() to continue searching. > >> > >> That means that the users of the PCI device pointer returned by > >> pci_get_dev_by_id() or its exported variants pci_get_subsys(), > >> pci_get_device() and pci_get_class() do handle the refcount. They > >> all pass the pointer to pci_dev_put() or continue the search, > >> which calls pci_dev_put() internally. > >> > >> Direct and indirect callers of get_pci_dev() don't care for the > >> refcount at all unless I'm missing something. > >> > >> > > >> > > Also, the issue appeared in 4.16 and > >> > > some affected code got moved around since then: it would be problematic > >> > > to backport the fix to stable releases. > >> > > > >> > > All that code never cared for reference counting anyway. Call pci_dev_put() > >> > > from get_pci_dev() to revert to the previous behavior. > >> > >> Fixes: 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary pcidev > >> > from pci_dn") > >> > > Cc: stable@vger.kernel.org # v4.16 > >> > > Signed-off-by: Greg Kurz <groug@kaod.org> > >> > > --- > >> > > arch/powerpc/platforms/powernv/npu-dma.c | 15 ++++++++++++++- > >> > > 1 file changed, 14 insertions(+), 1 deletion(-) > >> > > > >> > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c > >> > > index e713ade30087..d8f3647e8fb2 100644 > >> > > --- a/arch/powerpc/platforms/powernv/npu-dma.c > >> > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c > >> > > @@ -31,9 +31,22 @@ static DEFINE_SPINLOCK(npu_context_lock); > >> > > static struct pci_dev *get_pci_dev(struct device_node *dn) > >> > > { > >> > > struct pci_dn *pdn = PCI_DN(dn); > >> > > + struct pci_dev *pdev; > >> > > > >> > > - return pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), > >> > > + pdev = pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), > >> > > pdn->busno, pdn->devfn); > >> > > + > >> > > + /* > >> > > + * pci_get_domain_bus_and_slot() increased the reference count of > >> > > + * the PCI device, but callers don't need that actually as the PE > >> > > + * already holds a reference to the device. > >> > > >> > Imho this would be just enough. > >> > > >> > Anyway, > >> > > >> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > >> > > >> > >> Thanks ! > >> > >> I now realize that I forgot to add the --cc option for stable on my stgit > >> command line :-\. > >> > >> Cc'ing now. > >> > >> > > >> > How did you find it? :) > >> > > >> > >> While reading code to find some inspiration for OpenCAPI passthrough. :) > >> > >> I saw the following in vfio_pci_ibm_npu2_init(): > >> > >> if (!pnv_pci_get_gpu_dev(vdev->pdev)) > >> return -ENODEV; > >> > >> and simply followed the function calls. > >> > >> > > >> > > Since callers aren't > >> > > + * aware of the reference count change, call pci_dev_put() now to > >> > > + * avoid leaks. > >> > > + */ > >> > > + if (pdev) > >> > > + pci_dev_put(pdev); > >> > > + > >> > > + return pdev; > >> > > } > >> > > > >> > > /* Given a NPU device get the associated PCI device. */ > >> > > > >> > > >>
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c index e713ade30087..d8f3647e8fb2 100644 --- a/arch/powerpc/platforms/powernv/npu-dma.c +++ b/arch/powerpc/platforms/powernv/npu-dma.c @@ -31,9 +31,22 @@ static DEFINE_SPINLOCK(npu_context_lock); static struct pci_dev *get_pci_dev(struct device_node *dn) { struct pci_dn *pdn = PCI_DN(dn); + struct pci_dev *pdev; - return pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), + pdev = pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus), pdn->busno, pdn->devfn); + + /* + * pci_get_domain_bus_and_slot() increased the reference count of + * the PCI device, but callers don't need that actually as the PE + * already holds a reference to the device. Since callers aren't + * aware of the reference count change, call pci_dev_put() now to + * avoid leaks. + */ + if (pdev) + pci_dev_put(pdev); + + return pdev; } /* Given a NPU device get the associated PCI device. */
Since 902bdc57451c, get_pci_dev() calls pci_get_domain_bus_and_slot(). This has the effect of incrementing the reference count of the PCI device, as explained in drivers/pci/search.c: * Given a PCI domain, bus, and slot/function number, the desired PCI * device is located in the list of PCI devices. If the device is * found, its reference count is increased and this function returns a * pointer to its data structure. The caller must decrement the * reference count by calling pci_dev_put(). If no device is found, * %NULL is returned. Nothing was done to call pci_dev_put() and the reference count of GPU and NPU PCI devices rockets up. A natural way to fix this would be to teach the callers about the change, so that they call pci_dev_put() when done with the pointer. This turns out to be quite intrusive, as it affects many paths in npu-dma.c, pci-ioda.c and vfio_pci_nvlink2.c. Also, the issue appeared in 4.16 and some affected code got moved around since then: it would be problematic to backport the fix to stable releases. All that code never cared for reference counting anyway. Call pci_dev_put() from get_pci_dev() to revert to the previous behavior. Fixes: 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary pcidev from pci_dn") Cc: stable@vger.kernel.org # v4.16 Signed-off-by: Greg Kurz <groug@kaod.org> --- arch/powerpc/platforms/powernv/npu-dma.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)