On Wed, 27 Nov 2019 18:09:40 +1100 Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
> > > On 20/11/2019 12:28, Oliver O'Halloran wrote: > > The comment here implies that we don't need to take a ref to the pci_dev > > because the ioda_pe will always have one. This implies that the current > > expection is that the pci_dev for an NPU device will *never* be torn > > down since the ioda_pe having a ref to the device will prevent the > > release function from being called. > > > > In other words, the desired behaviour here appears to be leaking a ref. > > > > Nice! > > > There is a history: https://patchwork.ozlabs.org/patch/1088078/ > > We did not fix anything in particular then, we do not seem to be fixing > anything now (in other words - we cannot test it in a normal natural > way). I'd drop this one. > Yeah, I didn't fix anything at the time. Just reverted to the ref count behavior we had before: https://patchwork.ozlabs.org/patch/829172/ Frederic recently posted his take on the same topic from the OpenCAPI point of view: http://patchwork.ozlabs.org/patch/1198947/ He seems to indicate the NPU devices as the real culprit because nobody ever cared for them to be removable. Fixing that seems be a chore nobody really wants to address obviously... :-\ > > > > > > Signed-off-by: Oliver O'Halloran <ooh...@gmail.com> > > --- > > arch/powerpc/platforms/powernv/npu-dma.c | 11 +++-------- > > 1 file changed, 3 insertions(+), 8 deletions(-) > > > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c > > b/arch/powerpc/platforms/powernv/npu-dma.c > > index 72d3749da02c..2eb6e6d45a98 100644 > > --- a/arch/powerpc/platforms/powernv/npu-dma.c > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c > > @@ -28,15 +28,10 @@ static struct pci_dev *get_pci_dev(struct device_node > > *dn) > > break; > > > > /* > > - * 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. > > + * NB: for_each_pci_dev() elevates the pci_dev refcount. > > + * Caller is responsible for dropping the ref when it's > > + * finished with it. > > */ > > - if (pdev) > > - pci_dev_put(pdev); > > - > > return pdev; > > } > > > > >
