diff mbox

[PATCHv3,2/5] pci: Add is_removed state

Message ID 1475007815-28354-3-git-send-email-keith.busch@intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Keith Busch Sept. 27, 2016, 8:23 p.m. UTC
This adds a new state for devices that were once in the system, but
unexpectedly removed. This is so device tear down functions can observe
the device is not accessible so it may skip attempting to initialize
the hardware.

The pciehp and pcie-dpc drivers are aware of when the link is down,
so these explicitly set this flag when its handlers detect the device
is gone.

Signed-off-by: Keith Busch <keith.busch@intel.com>
Cc: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/hotplug/pciehp_pci.c | 2 ++
 drivers/pci/pcie/pcie-dpc.c      | 1 +
 include/linux/pci.h              | 1 +
 3 files changed, 4 insertions(+)

Comments

Lukas Wunner Oct. 21, 2016, 4:58 p.m. UTC | #1
On Tue, Sep 27, 2016 at 04:23:32PM -0400, Keith Busch wrote:
> This adds a new state for devices that were once in the system, but
> unexpectedly removed. This is so device tear down functions can observe
> the device is not accessible so it may skip attempting to initialize
> the hardware.
> 
> The pciehp and pcie-dpc drivers are aware of when the link is down,
> so these explicitly set this flag when its handlers detect the device
> is gone.

I note that you've removed the change to pci_device_is_present()
completely from v3 of this patch, however only a portion of the
change was problematic.  This portion would have been okay:


+	if (pdev->is_removed)
+		return false;
 
	return pci_bus_read_dev_vendor_id(pdev->bus, pdev->devfn, &v, 0);


Up to you if you want to include this in the next iteration or not.

Thanks,

Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keith Busch Oct. 21, 2016, 5:30 p.m. UTC | #2
On Fri, Oct 21, 2016 at 06:58:10PM +0200, Lukas Wunner wrote:
> On Tue, Sep 27, 2016 at 04:23:32PM -0400, Keith Busch wrote:
> I note that you've removed the change to pci_device_is_present()
> completely from v3 of this patch, however only a portion of the
> change was problematic.  This portion would have been okay:
> 
> 
> +	if (pdev->is_removed)
> +		return false;
>  
> 	return pci_bus_read_dev_vendor_id(pdev->bus, pdev->devfn, &v, 0);
> 
> 
> Up to you if you want to include this in the next iteration or not.

I was thinking it didn't matter since patch 3/5 has config access return
-ENODEV on a removed pci_dev, but pci_bus_read_dev_vendor_id doesn't use
the pci_dev. I'll add it back in.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index 9e69403..299ea5e 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -109,6 +109,8 @@  int pciehp_unconfigure_device(struct slot *p_slot)
 				break;
 			}
 		}
+		if (!presence)
+			dev->is_removed = 1;
 		pci_stop_and_remove_bus_device(dev);
 		/*
 		 * Ensure that no new Requests will be generated from
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 250f878..756eae4 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -44,6 +44,7 @@  static void interrupt_event_handler(struct work_struct *work)
 	list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
 					 bus_list) {
 		pci_dev_get(dev);
+		dev->is_removed = 1;
 		pci_stop_and_remove_bus_device(dev);
 		pci_dev_put(dev);
 	}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c81fbf7..36b759c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -337,6 +337,7 @@  struct pci_dev {
 	unsigned int	multifunction:1;/* Part of multi-function device */
 	/* keep track of device state */
 	unsigned int	is_added:1;
+	unsigned int	is_removed:1;	/* device was surprise removed */
 	unsigned int	is_busmaster:1; /* device is busmaster */
 	unsigned int	no_msi:1;	/* device may not use msi */
 	unsigned int	no_64bit_msi:1; /* device may only use 32-bit MSIs */