Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

e1000e: Use rtnl_lock to prevent race conditions between net and pci/pm

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>

authored by

Alexander Duyck and committed by
Jeff Kirsher
a7023819 914ee9c4

+35 -33
+35 -33
drivers/net/ethernet/intel/e1000e/netdev.c
··· 4715 4715 4716 4716 pm_runtime_get_sync(&pdev->dev); 4717 4717 4718 - if (!test_bit(__E1000_DOWN, &adapter->state)) { 4718 + if (netif_device_present(netdev)) { 4719 4719 e1000e_down(adapter, true); 4720 4720 e1000_free_irq(adapter); 4721 4721 4722 4722 /* Link status message must follow this format */ 4723 - pr_info("%s NIC Link is Down\n", adapter->netdev->name); 4723 + pr_info("%s NIC Link is Down\n", netdev->name); 4724 4724 } 4725 4725 4726 4726 napi_disable(&adapter->napi); ··· 6466 6466 { 6467 6467 struct net_device *netdev = dev_get_drvdata(dev); 6468 6468 struct e1000_adapter *adapter = netdev_priv(netdev); 6469 + bool present; 6469 6470 6471 + rtnl_lock(); 6472 + 6473 + present = netif_device_present(netdev); 6470 6474 netif_device_detach(netdev); 6471 6475 6472 - if (netif_running(netdev)) { 6476 + if (present && netif_running(netdev)) { 6473 6477 int count = E1000_CHECK_RESET_COUNT; 6474 6478 6475 6479 while (test_bit(__E1000_RESETTING, &adapter->state) && count--) ··· 6485 6481 e1000e_down(adapter, false); 6486 6482 e1000_free_irq(adapter); 6487 6483 } 6484 + rtnl_unlock(); 6485 + 6488 6486 e1000e_reset_interrupt_capability(adapter); 6489 6487 6490 6488 /* Allow time for pending master requests to run */ ··· 6734 6728 __e1000e_disable_aspm(pdev, state, 1); 6735 6729 } 6736 6730 6731 + static int e1000e_pm_thaw(struct device *dev) 6732 + { 6733 + struct net_device *netdev = dev_get_drvdata(dev); 6734 + struct e1000_adapter *adapter = netdev_priv(netdev); 6735 + int rc = 0; 6736 + 6737 + e1000e_set_interrupt_capability(adapter); 6738 + 6739 + rtnl_lock(); 6740 + if (netif_running(netdev)) { 6741 + rc = e1000_request_irq(adapter); 6742 + if (rc) 6743 + goto err_irq; 6744 + 6745 + e1000e_up(adapter); 6746 + } 6747 + 6748 + netif_device_attach(netdev); 6749 + err_irq: 6750 + rtnl_unlock(); 6751 + 6752 + return rc; 6753 + } 6754 + 6737 6755 #ifdef CONFIG_PM 6738 6756 static int __e1000_resume(struct pci_dev *pdev) 6739 6757 { ··· 6825 6795 } 6826 6796 6827 6797 #ifdef CONFIG_PM_SLEEP 6828 - static int e1000e_pm_thaw(struct device *dev) 6829 - { 6830 - struct net_device *netdev = dev_get_drvdata(dev); 6831 - struct e1000_adapter *adapter = netdev_priv(netdev); 6832 - 6833 - e1000e_set_interrupt_capability(adapter); 6834 - if (netif_running(netdev)) { 6835 - u32 err = e1000_request_irq(adapter); 6836 - 6837 - if (err) 6838 - return err; 6839 - 6840 - e1000e_up(adapter); 6841 - } 6842 - 6843 - netif_device_attach(netdev); 6844 - 6845 - return 0; 6846 - } 6847 - 6848 6798 static int e1000e_pm_suspend(struct device *dev) 6849 6799 { 6850 6800 struct net_device *netdev = pci_get_drvdata(to_pci_dev(dev)); ··· 7010 7000 static pci_ers_result_t e1000_io_error_detected(struct pci_dev *pdev, 7011 7001 pci_channel_state_t state) 7012 7002 { 7013 - struct net_device *netdev = pci_get_drvdata(pdev); 7014 - struct e1000_adapter *adapter = netdev_priv(netdev); 7015 - 7016 - netif_device_detach(netdev); 7003 + e1000e_pm_freeze(&pdev->dev); 7017 7004 7018 7005 if (state == pci_channel_io_perm_failure) 7019 7006 return PCI_ERS_RESULT_DISCONNECT; 7020 7007 7021 - if (netif_running(netdev)) 7022 - e1000e_down(adapter, true); 7023 7008 pci_disable_device(pdev); 7024 7009 7025 7010 /* Request a slot slot reset. */ ··· 7080 7075 7081 7076 e1000_init_manageability_pt(adapter); 7082 7077 7083 - if (netif_running(netdev)) 7084 - e1000e_up(adapter); 7085 - 7086 - netif_device_attach(netdev); 7078 + e1000e_pm_thaw(&pdev->dev); 7087 7079 7088 7080 /* If the controller has AMT, do not set DRV_LOAD until the interface 7089 7081 * is up. For all other cases, let the f/w know that the h/w is now