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

[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 <hch@lst.de>
---
 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->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 <sathyanarayanan.kuppuswamy@linux.intel.com>


> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   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->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

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 <hch@lst.de>

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->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
>