linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Oliver O'Halloran <oohall@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: sbobroff@linux.ibm.com, Oliver O'Halloran <oohall@gmail.com>
Subject: [PATCH 02/14] powerpc/eeh: Fix race when freeing PDNs
Date: Tue,  3 Sep 2019 20:15:53 +1000	[thread overview]
Message-ID: <20190903101605.2890-3-oohall@gmail.com> (raw)
In-Reply-To: <20190903101605.2890-1-oohall@gmail.com>

When hot-adding devices we rely on the hotplug driver to create pci_dn's
for the devices under the hotplug slot. Converse, when hot-removing the
driver will remove the pci_dn's that it created. This is a problem because
the pci_dev is still live until it's refcount drops to zero. This can
happen if the driver is slow to tear down it's internal state. Ideally, the
driver would not attempt to perform any config accesses to the device once
it's been marked as removed, but sometimes it happens. As a result, we
might attempt to access the pci_dn for a device that has been torn down and
the kernel may crash as a result.

To fix this, don't free the pci_dn unless the corresponding pci_dev has
been released.  If the pci_dev is still live, then we mark the pci_dn with
a flag that indicates the pci_dev's release function should free it.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/include/asm/pci-bridge.h |  1 +
 arch/powerpc/kernel/pci-hotplug.c     |  7 +++++++
 arch/powerpc/kernel/pci_dn.c          | 21 +++++++++++++++++++--
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 54a9de01c2a3..69f4cb3b7c56 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -183,6 +183,7 @@ struct iommu_table;
 struct pci_dn {
 	int     flags;
 #define PCI_DN_FLAG_IOV_VF	0x01
+#define PCI_DN_FLAG_DEAD	0x02    /* Device has been hot-removed */
 
 	int	busno;			/* pci bus number */
 	int	devfn;			/* pci device and function number */
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index 0b0cf8168b47..fc62c4bc47b1 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -55,11 +55,18 @@ EXPORT_SYMBOL_GPL(pci_find_bus_by_node);
 void pcibios_release_device(struct pci_dev *dev)
 {
 	struct pci_controller *phb = pci_bus_to_host(dev->bus);
+	struct pci_dn *pdn = pci_get_pdn(dev);
 
 	eeh_remove_device(dev);
 
 	if (phb->controller_ops.release_device)
 		phb->controller_ops.release_device(dev);
+
+	/* free()ing the pci_dn has been deferred to us, do it now */
+	if (pdn && (pdn->flags & PCI_DN_FLAG_DEAD)) {
+		pci_dbg(dev, "freeing dead pdn\n");
+		kfree(pdn);
+	}
 }
 
 /**
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 5614ca0940ca..4e654df55969 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -320,6 +320,7 @@ void pci_remove_device_node_info(struct device_node *dn)
 {
 	struct pci_dn *pdn = dn ? PCI_DN(dn) : NULL;
 	struct device_node *parent;
+	struct pci_dev *pdev;
 #ifdef CONFIG_EEH
 	struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
 
@@ -333,12 +334,28 @@ void pci_remove_device_node_info(struct device_node *dn)
 	WARN_ON(!list_empty(&pdn->child_list));
 	list_del(&pdn->list);
 
+	/* Drop the parent pci_dn's ref to our backing dt node */
 	parent = of_get_parent(dn);
 	if (parent)
 		of_node_put(parent);
 
-	dn->data = NULL;
-	kfree(pdn);
+	/*
+	 * At this point we *might* still have a pci_dev that was
+	 * instantiated from this pci_dn. So defer free()ing it until
+	 * the pci_dev's release function is called.
+	 */
+	pdev = pci_get_domain_bus_and_slot(pdn->phb->global_number,
+			pdn->busno, pdn->devfn);
+	if (pdev) {
+		/* NB: pdev has a ref to dn */
+		pci_dbg(pdev, "marked pdn (from %pOF) as dead\n", dn);
+		pdn->flags |= PCI_DN_FLAG_DEAD;
+	} else {
+		dn->data = NULL;
+		kfree(pdn);
+	}
+
+	pci_dev_put(pdev);
 }
 EXPORT_SYMBOL_GPL(pci_remove_device_node_info);
 
-- 
2.21.0


  parent reply	other threads:[~2019-09-03 10:23 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-03 10:15 EEH + hotplug fixes Oliver O'Halloran
2019-09-03 10:15 ` [PATCH 01/14] powerpc/eeh: Clean up EEH PEs after recovery finishes Oliver O'Halloran
2019-09-17  0:43   ` Sam Bobroff
2019-09-19 10:25   ` Michael Ellerman
2019-09-03 10:15 ` Oliver O'Halloran [this message]
2019-09-17  0:50   ` [PATCH 02/14] powerpc/eeh: Fix race when freeing PDNs Sam Bobroff
2019-09-03 10:15 ` [PATCH 03/14] powerpc/eeh: Make permanently failed devices non-actionable Oliver O'Halloran
2019-09-17  0:51   ` Sam Bobroff
2019-09-03 10:15 ` [PATCH 04/14] powerpc/eeh: Check slot presence state in eeh_handle_normal_event() Oliver O'Halloran
2019-09-17  1:00   ` Sam Bobroff
2019-09-17  4:20     ` Oliver O'Halloran
2019-09-03 10:15 ` [PATCH 05/14] powerpc/eeh: Defer printing stack trace Oliver O'Halloran
2019-09-17  1:04   ` Sam Bobroff
2019-09-17  1:45     ` Oliver O'Halloran
2019-09-17  3:35       ` Sam Bobroff
2019-09-17  3:38         ` Oliver O'Halloran
2019-09-03 10:15 ` [PATCH 06/14] powerpc/eeh: Remove stale CAPI comment Oliver O'Halloran
2019-09-03 14:09   ` Andrew Donnellan
2019-09-17  1:04   ` Sam Bobroff
2019-09-03 10:15 ` [PATCH 07/14] powernv/eeh: Use generic code to handle hot resets Oliver O'Halloran
2019-09-17  1:15   ` Sam Bobroff
2019-09-17  7:30     ` Oliver O'Halloran
2019-09-03 10:15 ` [PATCH 08/14] pci-hotplug/pnv_php: Add a reset_slot() callback Oliver O'Halloran
2019-09-03 10:16 ` [PATCH 09/14] pci-hotplug/pnv_php: Add support for IODA3 Power9 PHBs Oliver O'Halloran
2019-09-03 10:16 ` [PATCH 10/14] pci-hotplug/pnv_php: Add attention indicator support Oliver O'Halloran
2019-09-03 10:16 ` [PATCH 11/14] powerpc/eeh: Set attention indicator while recovering Oliver O'Halloran
2019-09-17  1:23   ` Sam Bobroff
2019-09-03 10:16 ` [PATCH 12/14] powerpc/eeh: Add debugfs interface to run an EEH check Oliver O'Halloran
2019-09-17  3:15   ` Sam Bobroff
2019-09-17  3:36     ` Oliver O'Halloran
2019-09-17  4:23       ` Oliver O'Halloran
2019-09-03 10:16 ` [PATCH 13/14] powerpc/eeh: Add a eeh_dev_break debugfs interface Oliver O'Halloran
2019-09-17  3:19   ` Sam Bobroff
2019-09-03 10:16 ` [PATCH 14/14] selftests/powerpc: Add basic EEH selftest Oliver O'Halloran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190903101605.2890-3-oohall@gmail.com \
    --to=oohall@gmail.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=sbobroff@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).