Re: [PATCH] PCI/ERR: handle disconnected devices in report_error_detected

[Bjorn Helgaas]

On Wed, Jun 01, 2022 at 09:40:24AM +0200, Christoph Hellwig wrote:
> When a device is already unplugged by pciehp by the time that the AER
> handler is invoked, the PCIe device will lready be in the
> pci_channel_io_perm_failure state.  In that case we should simply
> return PCI_ERS_RESULT_DISCONNECT instead of trying to do a state
> transition that will fail.
> 
> Also untangle the state transition failure from the lack of methods to
> improve the debugging output in case it will happen ever again.
> 
> Signed-off-by: Christoph Hellwig 

Applied with Sathy's reviewed-by to pci/err for v5.20, thanks!

> ---
>  drivers/pci/pcie/err.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index 0c5a143025af4..59c90d04a609a 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -55,10 +55,14 @@ static int report_error_detected(struct pci_dev *dev,
>  
>   device_lock(>dev);
>   pdrv = dev->driver;
> - if (!pci_dev_set_io_state(dev, state) ||
> - !pdrv ||
> - !pdrv->err_handler ||
> - !pdrv->err_handler->error_detected) {
> + if (pci_dev_is_disconnected(dev)) {
> + vote = PCI_ERS_RESULT_DISCONNECT;
> + } else if (!pci_dev_set_io_state(dev, state)) {
> + pci_info(dev, "can't recover (state transition %u -> %u 
> invalid)\n",
> + dev->error_state, state);
> + vote = PCI_ERS_RESULT_NONE;
> + } else if (!pdrv || !pdrv->err_handler ||
> +!pdrv->err_handler->error_detected) {
>   /*
>* If any device in the subtree does not have an error_detected
>* callback, PCI_ERS_RESULT_NO_AER_DRIVER prevents subsequent
> -- 
> 2.30.2
> 

Re: [PATCH] PCI/ERR: handle disconnected devices in report_error_detected

[Sathyanarayanan Kuppuswamy]

Hi,

On 6/1/22 12:40 AM, Christoph Hellwig wrote:

When a device is already unplugged by pciehp by the time that the AER
handler is invoked, the PCIe device will lready be in the


/s/lready/already


pci_channel_io_perm_failure state.  In that case we should simply
return PCI_ERS_RESULT_DISCONNECT instead of trying to do a state
transition that will fail.

Also untangle the state transition failure from the lack of methods to
improve the debugging output in case it will happen ever again.


Otherwise, it looks good to me.

Reviewed-by: Kuppuswamy Sathyanarayanan 





Signed-off-by: Christoph Hellwig 
---
  drivers/pci/pcie/err.c | 12 
  1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index 0c5a143025af4..59c90d04a609a 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -55,10 +55,14 @@ static int report_error_detected(struct pci_dev *dev,
  
  	device_lock(>dev);

pdrv = dev->driver;
-   if (!pci_dev_set_io_state(dev, state) ||
-   !pdrv ||
-   !pdrv->err_handler ||
-   !pdrv->err_handler->error_detected) {
+   if (pci_dev_is_disconnected(dev)) {
+   vote = PCI_ERS_RESULT_DISCONNECT;
+   } else if (!pci_dev_set_io_state(dev, state)) {
+   pci_info(dev, "can't recover (state transition %u -> %u 
invalid)\n",
+   dev->error_state, state);
+   vote = PCI_ERS_RESULT_NONE;
+   } else if (!pdrv || !pdrv->err_handler ||
+  !pdrv->err_handler->error_detected) {
/*
 * If any device in the subtree does not have an error_detected
 * callback, PCI_ERS_RESULT_NO_AER_DRIVER prevents subsequent


--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

[PATCH] PCI/ERR: handle disconnected devices in report_error_detected

[Christoph Hellwig]

When a device is already unplugged by pciehp by the time that the AER
handler is invoked, the PCIe device will lready be in the
pci_channel_io_perm_failure state.  In that case we should simply
return PCI_ERS_RESULT_DISCONNECT instead of trying to do a state
transition that will fail.

Also untangle the state transition failure from the lack of methods to
improve the debugging output in case it will happen ever again.

Signed-off-by: Christoph Hellwig 
---
 drivers/pci/pcie/err.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index 0c5a143025af4..59c90d04a609a 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -55,10 +55,14 @@ static int report_error_detected(struct pci_dev *dev,
 
device_lock(>dev);
pdrv = dev->driver;
-   if (!pci_dev_set_io_state(dev, state) ||
-   !pdrv ||
-   !pdrv->err_handler ||
-   !pdrv->err_handler->error_detected) {
+   if (pci_dev_is_disconnected(dev)) {
+   vote = PCI_ERS_RESULT_DISCONNECT;
+   } else if (!pci_dev_set_io_state(dev, state)) {
+   pci_info(dev, "can't recover (state transition %u -> %u 
invalid)\n",
+   dev->error_state, state);
+   vote = PCI_ERS_RESULT_NONE;
+   } else if (!pdrv || !pdrv->err_handler ||
+  !pdrv->err_handler->error_detected) {
/*
 * If any device in the subtree does not have an error_detected
 * callback, PCI_ERS_RESULT_NO_AER_DRIVER prevents subsequent
-- 
2.30.2