Skip to contentSkip to conversation

PCI/P2PDMA: Match interface changes to devm_memremap_pages()

PATCH In review Logan Gunthorpe 3 participants 5 messages Nov 30, 2018 – Dec 11, 2018

Subsystems (3)
lore
Share & tools
lore

Logan Gunthorpe

author maintainer
#0 lore raw

PATCH 1/1 PCI/P2PDMA: Match interface changes to devm_memremap_pages()

"mm-hmm-mark-hmm_devmem_add-add_resource-export_symbol_gpl.patch" in the mm tree breaks p2pdma. The patch was written and reviewed before p2pdma was merged so the necessary changes were not done to the call site in that code.

Without this patch, all drivers will fail to register P2P resources because devm_memremap_pages() will return -EINVAL due to the 'kill' member of the pagemap structure not yet being set.

Cc Andrew Morton <[email protected]>
Cc Dan Williams <[email protected]>
Cc Bjorn Helgaas <[email protected]>

---

Ideally this patch should be squashed with the one mentioned above to avoid a bisect regression point.

drivers/pci/p2pdma.c | 10 ++--------

 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/pci/p2pdma.c +2 -8
index ae3c5b25dcc7..a2eb25271c96 100644
@@ -82,10 +82,8 @@ static void pci_p2pdma_percpu_release(struct percpu_ref *ref)
 	complete_all(&p2p->devmap_ref_done);
 }
 
-static void pci_p2pdma_percpu_kill(void *data)
+static void pci_p2pdma_percpu_kill(struct percpu_ref *ref)
 {
-	struct percpu_ref *ref = data;
-
 	/*
 	 * pci_p2pdma_add_resource() may be called multiple times
 	 * by a driver and may register the percpu_kill devm action multiple
@@ -198,6 +196,7 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
 	pgmap->type = MEMORY_DEVICE_PCI_P2PDMA;
 	pgmap->pci_p2pdma_bus_offset = pci_bus_address(pdev, bar) -
 		pci_resource_start(pdev, bar);
+	pgmap->kill = pci_p2pdma_percpu_kill;
 
 	addr = devm_memremap_pages(&pdev->dev, pgmap);
 	if (IS_ERR(addr)) {
@@ -211,11 +210,6 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
 	if (error)
 		goto pgmap_free;
 
-	error = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_percpu_kill,
-					  &pdev->p2pdma->devmap_ref);
-	if (error)
-		goto pgmap_free;
-
 	pci_info(pdev, "added peer-to-peer DMA memory %pR\n",
 		 &pgmap->res);
 

--
2.19.0

Reviewed-by on v1 1/1
#1 lore raw
Quoted 6 lines · Logan Gunthorpe

"mm-hmm-mark-hmm_devmem_add-add_resource-export_symbol_gpl.patch" in the mm tree breaks p2pdma. The patch was written and reviewed before p2pdma was merged so the necessary changes were not done to the call site in that code.

Quoted 7 lines

Without this patch, all drivers will fail to register P2P resources because devm_memremap_pages() will return -EINVAL due to the 'kill' member of the pagemap structure not yet being set.

Cc Andrew Morton <[email protected]>
Cc Dan Williams <[email protected]>
#2 lore raw
In reply to Logan Gunthorpe
Quoted 5 lines · Logan Gunthorpe

"mm-hmm-mark-hmm_devmem_add-add_resource-export_symbol_gpl.patch" in the mm tree breaks p2pdma. The patch was written and reviewed before p2pdma was merged so the necessary changes were not done to the call site in that code.

Quoted 8 lines

Without this patch, all drivers will fail to register P2P resources because devm_memremap_pages() will return -EINVAL due to the 'kill' member of the pagemap structure not yet being set.

Cc Andrew Morton <[email protected]>
Cc Dan Williams <[email protected]>
Cc Bjorn Helgaas <[email protected]>

Applied with Dan's reviewed-by to pci/peer-to-peer for v4.21, thanks!

If the mm patch you mention gets merged for v4.20, let me know and I can promote this to for-linus so v4.20 doesn't end up broken.

Quoted 46 lines

---

Ideally this patch should be squashed with the one mentioned above to avoid a bisect regression point.

drivers/pci/p2pdma.c | 10 ++--------

 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/pci/p2pdma.c +2 -9
index ae3c5b25dcc7..a2eb25271c96 100644
@@ -82,10 +82,8 @@ static void pci_p2pdma_percpu_release(struct percpu_ref *ref)
 	complete_all(&p2p->devmap_ref_done);
 }
 
-static void pci_p2pdma_percpu_kill(void *data)
+static void pci_p2pdma_percpu_kill(struct percpu_ref *ref)
 {
-	struct percpu_ref *ref = data;
-
 	/*
 	 * pci_p2pdma_add_resource() may be called multiple times
 	 * by a driver and may register the percpu_kill devm action multiple
@@ -198,6 +196,7 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
 	pgmap->type = MEMORY_DEVICE_PCI_P2PDMA;
 	pgmap->pci_p2pdma_bus_offset = pci_bus_address(pdev, bar) -
 		pci_resource_start(pdev, bar);
+	pgmap->kill = pci_p2pdma_percpu_kill;
 
 	addr = devm_memremap_pages(&pdev->dev, pgmap);
 	if (IS_ERR(addr)) {
@@ -211,11 +210,6 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
 	if (error)
 		goto pgmap_free;
 
-	error = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_percpu_kill,
-					  &pdev->p2pdma->devmap_ref);
-	if (error)
-		goto pgmap_free;
-
 	pci_info(pdev, "added peer-to-peer DMA memory %pR\n",
 		 &pgmap->res);
 
--

2.19.0

author maintainer
#3 lore raw
Quoted 15 lines · Bjorn Helgaas
Quoted 5 lines · Logan Gunthorpe

"mm-hmm-mark-hmm_devmem_add-add_resource-export_symbol_gpl.patch" in the mm tree breaks p2pdma. The patch was written and reviewed before p2pdma was merged so the necessary changes were not done to the call site in that code.

Quoted 8 lines

Without this patch, all drivers will fail to register P2P resources because devm_memremap_pages() will return -EINVAL due to the 'kill' member of the pagemap structure not yet being set.

Cc Andrew Morton <[email protected]>
Cc Dan Williams <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Quoted 4 lines

Applied with Dan's reviewed-by to pci/peer-to-peer for v4.21, thanks!

If the mm patch you mention gets merged for v4.20, let me know and I can promote this to for-linus so v4.20 doesn't end up broken.

Thanks Bjorn, but I think Andrew has also picked it up in the mm tree with the patch in question. My hope is that he squashes the two but I'm not sure what his intentions are. I mostly copied you for information purposes as this patch shouldn't even compile without Dan's patch.

Though, I guess we'll find out what goes in after the merge window -- you may not need to do anything.

Logan

#4 lore raw
Quoted 23 lines · Logan Gunthorpe
Quoted 15 lines · Bjorn Helgaas
Quoted 5 lines · Logan Gunthorpe

"mm-hmm-mark-hmm_devmem_add-add_resource-export_symbol_gpl.patch" in the mm tree breaks p2pdma. The patch was written and reviewed before p2pdma was merged so the necessary changes were not done to the call site in that code.

Quoted 8 lines

Without this patch, all drivers will fail to register P2P resources because devm_memremap_pages() will return -EINVAL due to the 'kill' member of the pagemap structure not yet being set.

Cc Andrew Morton <[email protected]>
Cc Dan Williams <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Quoted 4 lines

Applied with Dan's reviewed-by to pci/peer-to-peer for v4.21, thanks!

If the mm patch you mention gets merged for v4.20, let me know and I can promote this to for-linus so v4.20 doesn't end up broken.

Quoted 7 lines

Thanks Bjorn, but I think Andrew has also picked it up in the mm tree with the patch in question. My hope is that he squashes the two but I'm not sure what his intentions are. I mostly copied you for information purposes as this patch shouldn't even compile without Dan's patch.

Though, I guess we'll find out what goes in after the merge window -- you may not need to do anything.

OK, I dropped this because I don't have the matching mm patch. I agree, it should be squashed so we avoid a bisection hole. Let me know if I need to do anything else with this.

Bjorn