[PATCH] ixgbe/ixgbevf: Free IRQ when PCI error recovery removes the device

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] ixgbe/ixgbevf: Free IRQ when PCI error recovery removes the device

Mauro S. M. Rodrigues
From: Mauro S M Rodrigues <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1776389

Since commit f7f37e7ff2b9 ("ixgbe: handle close/suspend race with
netif_device_detach/present") ixgbe_close_suspend is called, from
ixgbe_close, only if the device is present, i.e. if it isn't detached.
That exposed a situation where IRQs weren't freed if a PCI error
recovery system opts to remove the device. For such case the pci channel
state is set to pci_channel_io_perm_failure and ixgbe_io_error_detected
was returning PCI_ERS_RESULT_DISCONNECT before calling
ixgbe_close_suspend consequentially not freeing IRQ and crashing when
the remove handler calls pci_disable_device, hitting a BUG_ON at
free_msi_irqs, which asserts that there is no non-free IRQ associated
with the device to be removed:

BUG_ON(irq_has_action(entry->irq + i));

The issue is fixed by calling the ixgbe_close_suspend before evaluate
the pci channel state.

Reported-by: Naresh Bannoth <[hidden email]>
Reported-by: Abdul Haleem <[hidden email]>
Signed-off-by: Mauro S M Rodrigues <[hidden email]>
Reviewed-by: Alexander Duyck <[hidden email]>
Tested-by: Andrew Bowers <[hidden email]>
Signed-off-by: Jeff Kirsher <[hidden email]>
(cherry picked from commit b212d815e77c72be921979119c715166cc8987b1)
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     | 6 +++---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a549c4870882..e1326c0603af 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10910,14 +10910,14 @@ static pci_ers_result_t ixgbe_io_error_detected(struct pci_dev *pdev,
  rtnl_lock();
  netif_device_detach(netdev);
 
+ if (netif_running(netdev))
+ ixgbe_close_suspend(adapter);
+
  if (state == pci_channel_io_perm_failure) {
  rtnl_unlock();
  return PCI_ERS_RESULT_DISCONNECT;
  }
 
- if (netif_running(netdev))
- ixgbe_close_suspend(adapter);
-
  if (!test_and_set_bit(__IXGBE_DISABLED, &adapter->state))
  pci_disable_device(pdev);
  rtnl_unlock();
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 1f4a69134ade..64859f8318c8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -4241,14 +4241,14 @@ static pci_ers_result_t ixgbevf_io_error_detected(struct pci_dev *pdev,
  rtnl_lock();
  netif_device_detach(netdev);
 
+ if (netif_running(netdev))
+ ixgbevf_close_suspend(adapter);
+
  if (state == pci_channel_io_perm_failure) {
  rtnl_unlock();
  return PCI_ERS_RESULT_DISCONNECT;
  }
 
- if (netif_running(netdev))
- ixgbevf_close_suspend(adapter);
-
  if (!test_and_set_bit(__IXGBEVF_DISABLED, &adapter->state))
  pci_disable_device(pdev);
  rtnl_unlock();
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NACK: [PATCH] ixgbe/ixgbevf: Free IRQ when PCI error recovery removes the device

Kleber Souza
On 06/12/18 14:57, Mauro S. M. Rodrigues wrote:

> From: Mauro S M Rodrigues <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1776389
>
> Since commit f7f37e7ff2b9 ("ixgbe: handle close/suspend race with
> netif_device_detach/present") ixgbe_close_suspend is called, from
> ixgbe_close, only if the device is present, i.e. if it isn't detached.
> That exposed a situation where IRQs weren't freed if a PCI error
> recovery system opts to remove the device. For such case the pci channel
> state is set to pci_channel_io_perm_failure and ixgbe_io_error_detected
> was returning PCI_ERS_RESULT_DISCONNECT before calling
> ixgbe_close_suspend consequentially not freeing IRQ and crashing when
> the remove handler calls pci_disable_device, hitting a BUG_ON at
> free_msi_irqs, which asserts that there is no non-free IRQ associated
> with the device to be removed:
>
> BUG_ON(irq_has_action(entry->irq + i));
>
> The issue is fixed by calling the ixgbe_close_suspend before evaluate
> the pci channel state.
>
> Reported-by: Naresh Bannoth <[hidden email]>
> Reported-by: Abdul Haleem <[hidden email]>
> Signed-off-by: Mauro S M Rodrigues <[hidden email]>
> Reviewed-by: Alexander Duyck <[hidden email]>
> Tested-by: Andrew Bowers <[hidden email]>
> Signed-off-by: Jeff Kirsher <[hidden email]>
> (cherry picked from commit b212d815e77c72be921979119c715166cc8987b1)

Hi Mauro,

Please add your "Signed-off-by" as the last line right below the
"(cherry picked from...)" as you are the person sending the patch, this
is required even if you are the original author and the s-o-b would
appear more than once.

Please also identify on the email subject which series this patch is
targeted for.


Thank you,
Kleber

> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     | 6 +++---
>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index a549c4870882..e1326c0603af 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -10910,14 +10910,14 @@ static pci_ers_result_t ixgbe_io_error_detected(struct pci_dev *pdev,
>   rtnl_lock();
>   netif_device_detach(netdev);
>  
> + if (netif_running(netdev))
> + ixgbe_close_suspend(adapter);
> +
>   if (state == pci_channel_io_perm_failure) {
>   rtnl_unlock();
>   return PCI_ERS_RESULT_DISCONNECT;
>   }
>  
> - if (netif_running(netdev))
> - ixgbe_close_suspend(adapter);
> -
>   if (!test_and_set_bit(__IXGBE_DISABLED, &adapter->state))
>   pci_disable_device(pdev);
>   rtnl_unlock();
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 1f4a69134ade..64859f8318c8 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -4241,14 +4241,14 @@ static pci_ers_result_t ixgbevf_io_error_detected(struct pci_dev *pdev,
>   rtnl_lock();
>   netif_device_detach(netdev);
>  
> + if (netif_running(netdev))
> + ixgbevf_close_suspend(adapter);
> +
>   if (state == pci_channel_io_perm_failure) {
>   rtnl_unlock();
>   return PCI_ERS_RESULT_DISCONNECT;
>   }
>  
> - if (netif_running(netdev))
> - ixgbevf_close_suspend(adapter);
> -
>   if (!test_and_set_bit(__IXGBEVF_DISABLED, &adapter->state))
>   pci_disable_device(pdev);
>   rtnl_unlock();
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team