On Fri, Apr 03, 2020 at 05:08:32PM +1100, Oliver O'Halloran wrote:

On Mon, 2020-03-30 at 15:56 +1100, Sam Bobroff wrote:

quoted

When EEH device state was released asynchronously by the device
release handler, it was possible for an outstanding reference to
prevent it's release and it was necessary to work around that if a
device was re-discovered at the same PCI location.

I think this is a bit misleading. The main situation where you'll hit
this hack is when recovering a device with a driver that doesn't
implement the error handling callbacks. In that case the device is
removed, reset, then re-probed by the PCI core, but we assume it's the
same physical device so the eeh_device state remains active.

If you actually changed the underlying device I suspect something bad
would happen.

I'm not sure I understand. Isn't the case you're talking about caught by
the earlier check (just above the patch)?

	if (edev->pdev == dev) {
		eeh_edev_dbg(edev, "Device already referenced!\n");
		return;
	}

quoted

Now that the state is released synchronously that is no longer
possible and the workaround is no longer necessary.

You could probably fold this into the previous patch, but eh. You could
probably fold this into the previous patch, but eh.

True.

quoted

Signed-off-by: Sam Bobroff <redacted>
---
 arch/powerpc/kernel/eeh.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index c36c5a7db5ca..12c248a16527 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c

@@ -1206,28 +1206,7 @@ void eeh_add_device_late(struct pci_dev *dev)
 		eeh_edev_dbg(edev, "Device already referenced!\n");
 		return;
 	}
-
-	/*
-	 * The EEH cache might not be removed correctly because of
-	 * unbalanced kref to the device during unplug time, which
-	 * relies on pcibios_release_device(). So we have to remove
-	 * that here explicitly.
-	 */
-	if (edev->pdev) {
-		eeh_rmv_from_parent_pe(edev);
-		eeh_addr_cache_rmv_dev(edev->pdev);
-		eeh_sysfs_remove_device(edev->pdev);
-
-		/*
-		 * We definitely should have the PCI device removed
-		 * though it wasn't correctly. So we needn't call
-		 * into error handler afterwards.
-		 */
-		edev->mode |= EEH_DEV_NO_HANDLER;
-
-		edev->pdev = NULL;
-		dev->dev.archdata.edev = NULL;
-	}
+	WARN_ON_ONCE(edev->pdev);
 
 	if (eeh_has_flag(EEH_PROBE_MODE_DEV))
 		eeh_ops->probe(pdn, NULL);