Messages in this thread | | | Date | Tue, 15 Apr 2014 17:02:14 +0800 | From | "Li, ZhenHua" <> | Subject | Re: [PATCH 1/1] driver/net: add missing rtnl lock/unlock for benet |
| |
Because netif_running() is called in netif_device_detach and netif_device_attach. To avoid dev status changed while netif_device_detach/attach is not finished, I think a rtnl_lock and unlock should be called to avoid this.
Thanks Zhenhua
On 04/15/2014 04:07 PM, Sathya Perla wrote: >> -----Original Message----- >> From: Li, Zhen-Hua [mailto:zhen-hual@hp.com] >> >> In benet driver, netif_device_detach and netif_device_attach should be >> called between rtnl_lock and rtnl_unlock. > > Zhen, it's not clear to me why rtnl_lock is needed around netif_device_attach(). > Can you pls explain what exact data-structure you are protecting with the lock? > > Are you see warning stack trace? I don't see ASSERT_RTNL() called anywhere from netif_device_attach/detach()? > > thanks! > >> >> Signed-off-by: Li, Zhen-Hua <zhen-hual@hp.com> >> --- >> drivers/net/ethernet/emulex/benet/be_main.c | 17 ++++++++++++++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c >> b/drivers/net/ethernet/emulex/benet/be_main.c >> index 3e6df47..9c44b3f 100644 >> --- a/drivers/net/ethernet/emulex/benet/be_main.c >> +++ b/drivers/net/ethernet/emulex/benet/be_main.c >> @@ -4555,8 +4555,11 @@ static void be_func_recovery_task(struct work_struct *work) >> rtnl_unlock(); >> >> status = lancer_recover_func(adapter); >> - if (!status) >> + if (!status) { >> + rtnl_lock(); >> netif_device_attach(adapter->netdev); >> + rtnl_unlock(); >> + } >> } >> >> /* In Lancer, for all errors other than provisioning error (-EAGAIN), >> @@ -4784,12 +4787,12 @@ static int be_suspend(struct pci_dev *pdev, pm_message_t >> state) >> be_intr_set(adapter, false); >> cancel_delayed_work_sync(&adapter->func_recovery_work); >> >> + rtnl_lock(); >> netif_device_detach(netdev); >> if (netif_running(netdev)) { >> - rtnl_lock(); >> be_close(netdev); >> - rtnl_unlock(); >> } >> + rtnl_unlock(); >> be_clear(adapter); >> >> pci_save_state(pdev); >> @@ -4804,7 +4807,9 @@ static int be_resume(struct pci_dev *pdev) >> struct be_adapter *adapter = pci_get_drvdata(pdev); >> struct net_device *netdev = adapter->netdev; >> >> + rtnl_lock(); >> netif_device_detach(netdev); >> + rtnl_unlock(); >> >> status = pci_enable_device(pdev); >> if (status) >> @@ -4832,7 +4837,9 @@ static int be_resume(struct pci_dev *pdev) >> >> schedule_delayed_work(&adapter->func_recovery_work, >> msecs_to_jiffies(1000)); >> + rtnl_lock(); >> netif_device_attach(netdev); >> + rtnl_unlock(); >> >> if (adapter->wol_en) >> be_setup_wol(adapter, false); >> @@ -4853,7 +4860,9 @@ static void be_shutdown(struct pci_dev *pdev) >> cancel_delayed_work_sync(&adapter->work); >> cancel_delayed_work_sync(&adapter->func_recovery_work); >> >> + rtnl_lock(); >> netif_device_detach(adapter->netdev); >> + rtnl_unlock(); >> >> be_cmd_reset_function(adapter); >> >> @@ -4957,7 +4966,9 @@ static void be_eeh_resume(struct pci_dev *pdev) >> >> schedule_delayed_work(&adapter->func_recovery_work, >> msecs_to_jiffies(1000)); >> + rtnl_lock(); >> netif_device_attach(netdev); >> + rtnl_unlock(); >> return; >> err: >> dev_err(&adapter->pdev->dev, "EEH resume failed\n"); >> -- >> 1.7.10.4 >
| |