Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753665AbaDOJDT (ORCPT ); Tue, 15 Apr 2014 05:03:19 -0400 Received: from g4t3426.houston.hp.com ([15.201.208.54]:1677 "EHLO g4t3426.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751094AbaDOJDR (ORCPT ); Tue, 15 Apr 2014 05:03:17 -0400 Message-ID: <534CF596.3010609@hp.com> Date: Tue, 15 Apr 2014 17:02:14 +0800 From: "Li, ZhenHua" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Sathya Perla CC: Subramanian Seetharaman , Ajit Khaparde , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "Li, ZhenHua" Subject: Re: [PATCH 1/1] driver/net: add missing rtnl lock/unlock for benet References: <1397544352-28841-1-git-send-email-zhen-hual@hp.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/