| From 20d52f91ba958d221c17a8ff175ea1e3e922a5ee Mon Sep 17 00:00:00 2001 |
| From: Alexander Duyck <alexander.h.duyck@linux.intel.com> |
| Date: Fri, 11 Oct 2019 08:34:52 -0700 |
| Subject: [PATCH] e1000e: Use rtnl_lock to prevent race conditions between net |
| and pci/pm |
| |
| commit a7023819404ac9bd2bb311a4fafd38515cfa71ec upstream. |
| |
| This patch is meant to address possible race conditions that can exist |
| between network configuration and power management. A similar issue was |
| fixed for igb in commit 9474933caf21 ("igb: close/suspend race in |
| netif_device_detach"). |
| |
| In addition it consolidates the code so that the PCI error handling code |
| will essentially perform the power management freeze on the device prior to |
| attempting a reset, and will thaw the device afterwards if that is what it |
| is planning to do. Otherwise when we call close on the interface it should |
| see it is detached and not attempt to call the logic to down the interface |
| and free the IRQs again. |
| |
| From what I can tell the check that was adding the check for __E1000_DOWN |
| in e1000e_close was added when runtime power management was added. However |
| it should not be relevant for us as we perform a call to |
| pm_runtime_get_sync before we call e1000_down/free_irq so it should always |
| be back up before we call into this anyway. |
| |
| Reported-by: Morumuri Srivalli <smorumu1@in.ibm.com> |
| Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com> |
| Tested-by: David Dai <zdai@linux.vnet.ibm.com> |
| Tested-by: Aaron Brown <aaron.f.brown@intel.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c |
| index 2a61c79cf467..91580fb2f198 100644 |
| --- a/drivers/net/ethernet/intel/e1000e/netdev.c |
| +++ b/drivers/net/ethernet/intel/e1000e/netdev.c |
| @@ -4713,12 +4713,12 @@ int e1000e_close(struct net_device *netdev) |
| |
| pm_runtime_get_sync(&pdev->dev); |
| |
| - if (!test_bit(__E1000_DOWN, &adapter->state)) { |
| + if (netif_device_present(netdev)) { |
| e1000e_down(adapter, true); |
| e1000_free_irq(adapter); |
| |
| /* Link status message must follow this format */ |
| - pr_info("%s NIC Link is Down\n", adapter->netdev->name); |
| + pr_info("%s NIC Link is Down\n", netdev->name); |
| } |
| |
| napi_disable(&adapter->napi); |
| @@ -6294,10 +6294,14 @@ static int e1000e_pm_freeze(struct device *dev) |
| { |
| struct net_device *netdev = dev_get_drvdata(dev); |
| struct e1000_adapter *adapter = netdev_priv(netdev); |
| + bool present; |
| |
| + rtnl_lock(); |
| + |
| + present = netif_device_present(netdev); |
| netif_device_detach(netdev); |
| |
| - if (netif_running(netdev)) { |
| + if (present && netif_running(netdev)) { |
| int count = E1000_CHECK_RESET_COUNT; |
| |
| while (test_bit(__E1000_RESETTING, &adapter->state) && count--) |
| @@ -6309,6 +6313,8 @@ static int e1000e_pm_freeze(struct device *dev) |
| e1000e_down(adapter, false); |
| e1000_free_irq(adapter); |
| } |
| + rtnl_unlock(); |
| + |
| e1000e_reset_interrupt_capability(adapter); |
| |
| /* Allow time for pending master requests to run */ |
| @@ -6556,6 +6562,30 @@ static void e1000e_disable_aspm_locked(struct pci_dev *pdev, u16 state) |
| __e1000e_disable_aspm(pdev, state, 1); |
| } |
| |
| +static int e1000e_pm_thaw(struct device *dev) |
| +{ |
| + struct net_device *netdev = dev_get_drvdata(dev); |
| + struct e1000_adapter *adapter = netdev_priv(netdev); |
| + int rc = 0; |
| + |
| + e1000e_set_interrupt_capability(adapter); |
| + |
| + rtnl_lock(); |
| + if (netif_running(netdev)) { |
| + rc = e1000_request_irq(adapter); |
| + if (rc) |
| + goto err_irq; |
| + |
| + e1000e_up(adapter); |
| + } |
| + |
| + netif_device_attach(netdev); |
| +err_irq: |
| + rtnl_unlock(); |
| + |
| + return rc; |
| +} |
| + |
| #ifdef CONFIG_PM |
| static int __e1000_resume(struct pci_dev *pdev) |
| { |
| @@ -6623,26 +6653,6 @@ static int __e1000_resume(struct pci_dev *pdev) |
| } |
| |
| #ifdef CONFIG_PM_SLEEP |
| -static int e1000e_pm_thaw(struct device *dev) |
| -{ |
| - struct net_device *netdev = dev_get_drvdata(dev); |
| - struct e1000_adapter *adapter = netdev_priv(netdev); |
| - |
| - e1000e_set_interrupt_capability(adapter); |
| - if (netif_running(netdev)) { |
| - u32 err = e1000_request_irq(adapter); |
| - |
| - if (err) |
| - return err; |
| - |
| - e1000e_up(adapter); |
| - } |
| - |
| - netif_device_attach(netdev); |
| - |
| - return 0; |
| -} |
| - |
| static int e1000e_pm_suspend(struct device *dev) |
| { |
| struct pci_dev *pdev = to_pci_dev(dev); |
| @@ -6814,16 +6824,11 @@ static void e1000_netpoll(struct net_device *netdev) |
| static pci_ers_result_t e1000_io_error_detected(struct pci_dev *pdev, |
| pci_channel_state_t state) |
| { |
| - struct net_device *netdev = pci_get_drvdata(pdev); |
| - struct e1000_adapter *adapter = netdev_priv(netdev); |
| - |
| - netif_device_detach(netdev); |
| + e1000e_pm_freeze(&pdev->dev); |
| |
| if (state == pci_channel_io_perm_failure) |
| return PCI_ERS_RESULT_DISCONNECT; |
| |
| - if (netif_running(netdev)) |
| - e1000e_down(adapter, true); |
| pci_disable_device(pdev); |
| |
| /* Request a slot slot reset. */ |
| @@ -6889,10 +6894,7 @@ static void e1000_io_resume(struct pci_dev *pdev) |
| |
| e1000_init_manageability_pt(adapter); |
| |
| - if (netif_running(netdev)) |
| - e1000e_up(adapter); |
| - |
| - netif_device_attach(netdev); |
| + e1000e_pm_thaw(&pdev->dev); |
| |
| /* If the controller has AMT, do not set DRV_LOAD until the interface |
| * is up. For all other cases, let the f/w know that the h/w is now |
| -- |
| 2.7.4 |
| |