| Message ID | 20181012155219.15360-1-keith.busch@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/P2PDMA: Fix freeing dev_pagemap on error | expand |
On Fri, Oct 12, 2018 at 09:52:19AM -0600, Keith Busch wrote: > The devres_free() API is only to be used after the resource has been > unlinked from the device tracking it. Calling this functino directly will > hit a kernel BUG_ON. This patch fixes this to use the managed resource > release function, devm_kfree(). > > Fixes: 1380472e7b855 ("PCI/P2PDMA: Support peer-to-peer memory") > Cc: Logan Gunthorpe <logang@deltatee.com> > Signed-off-by: Keith Busch <keith.busch@intel.com> > --- > drivers/pci/p2pdma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > index da66c7e31730..9c56a10eff15 100644 > --- a/drivers/pci/p2pdma.c > +++ b/drivers/pci/p2pdma.c > @@ -222,7 +222,7 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size, > return 0; > > pgmap_free: > - devres_free(pgmap); > + devm_kfree(&pci->dev, pgmap); ^^^ That should be 'pdev', not 'pci'. I need to slow down the workflow, I'm pointing send-email to the wrong patches again...
On 2018-10-12 9:52 a.m., Keith Busch wrote: > The devres_free() API is only to be used after the resource has been > unlinked from the device tracking it. Calling this functino directly will > hit a kernel BUG_ON. This patch fixes this to use the managed resource > release function, devm_kfree(). > > Fixes: 1380472e7b855 ("PCI/P2PDMA: Support peer-to-peer memory") > Cc: Logan Gunthorpe <logang@deltatee.com> > Signed-off-by: Keith Busch <keith.busch@intel.com> Oops, yes, nice catch! Thanks! Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > --- > drivers/pci/p2pdma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > index da66c7e31730..9c56a10eff15 100644 > --- a/drivers/pci/p2pdma.c > +++ b/drivers/pci/p2pdma.c > @@ -222,7 +222,7 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size, > return 0; > > pgmap_free: > - devres_free(pgmap); > + devm_kfree(&pci->dev, pgmap); > return error; > } > EXPORT_SYMBOL_GPL(pci_p2pdma_add_resource); >
On Fri, Oct 12, 2018 at 09:58:14AM -0600, Logan Gunthorpe wrote: > On 2018-10-12 9:52 a.m., Keith Busch wrote: > > The devres_free() API is only to be used after the resource has been > > unlinked from the device tracking it. Calling this functino directly will > > hit a kernel BUG_ON. This patch fixes this to use the managed resource > > release function, devm_kfree(). > > > > Fixes: 1380472e7b855 ("PCI/P2PDMA: Support peer-to-peer memory") > > Cc: Logan Gunthorpe <logang@deltatee.com> > > Signed-off-by: Keith Busch <keith.busch@intel.com> > > Oops, yes, nice catch! Thanks! Yeah, I hit the bug after an nvme controller reset. The nvme driver tries to add its CMB memory a second time, and it's correctly detected as an error by devm_memremap_pages(), but I'm also thinking nvme should have known better too. Anyway, this patch (its v2, really) is the simple error handling fix, but there will be a follow up nvme patch to avoid the error in the first place. :)
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index da66c7e31730..9c56a10eff15 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -222,7 +222,7 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size, return 0; pgmap_free: - devres_free(pgmap); + devm_kfree(&pci->dev, pgmap); return error; } EXPORT_SYMBOL_GPL(pci_p2pdma_add_resource);
The devres_free() API is only to be used after the resource has been unlinked from the device tracking it. Calling this functino directly will hit a kernel BUG_ON. This patch fixes this to use the managed resource release function, devm_kfree(). Fixes: 1380472e7b855 ("PCI/P2PDMA: Support peer-to-peer memory") Cc: Logan Gunthorpe <logang@deltatee.com> Signed-off-by: Keith Busch <keith.busch@intel.com> --- drivers/pci/p2pdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)