From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755639AbbFONte (ORCPT ); Mon, 15 Jun 2015 09:49:34 -0400 Received: from nbfkord-smmo01.seg.att.com ([209.65.160.76]:50462 "EHLO nbfkord-smmo01.seg.att.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752139AbbFONtY (ORCPT ); Mon, 15 Jun 2015 09:49:24 -0400 X-MXL-Hash: 557ed7e431f49c2d-c65e5a8bf5d6fd5a5f530848eaafafef832c0eab Message-ID: <557ED7DE.7080407@solarflare.com> Date: Mon, 15 Jun 2015 14:49:18 +0100 From: Edward Cree User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Jarod Wilson CC: , Solarflare linux maintainers , Subject: Re: [PATCH] ethernet/sfc: mark state UNINIT after unregister References: <1434135061-42521-1-git-send-email-jarod@redhat.com> In-Reply-To: <1434135061-42521-1-git-send-email-jarod@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.17.20.45] X-TM-AS-Product-Ver: SMEX-10.0.0.1412-7.000.1014-21614.005 X-TM-AS-Result: No--12.560800-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-AnalysisOut: [v=2.0 cv=HJl04PRv c=1 sm=1 a=MkjXnYnS3dyNWGSWLXxFFQ==:17 a] X-AnalysisOut: [=mypmul4OXYAA:10 a=fVG4DLb5TBsA:10 a=BLceEmwcHowA:10 a=Ikc] X-AnalysisOut: [TkHD0fZMA:10 a=zRKbQ67AAAAA:8 a=XAFQembCKUMA:10 a=VwQbUJbx] X-AnalysisOut: [AAAA:8 a=20KFwNOVAAAA:8 a=5UERYwf5tyCHEAziIUgA:9 a=QEXdDO2] X-AnalysisOut: [ut3YA:10] X-Spam: [F=0.2000000000; CM=0.500; S=0.200(2014051901)] X-MAIL-FROM: X-SOURCE-IP: [12.187.104.25] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/06/15 19:51, Jarod Wilson wrote: > Without this change, modprobe -r sfc hits the BUG_ON() in > efx_pci_remove_main(). Best as I can tell, this was just an oversight, > efx->state gets set to STATE_UNINIT in the error path of > efx_register_netdev() just after unregister_netdevice(), and the same > should happen in efx_unregister_netdev() after its unregister_netdevice() > call. Now I can load and unload no problem. > > CC: Solarflare linux maintainers > CC: netdev@vger.kernel.org > Signed-off-by: Jarod Wilson > --- > drivers/net/ethernet/sfc/efx.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c > index 0c42ed9..f3eaade 100644 > --- a/drivers/net/ethernet/sfc/efx.c > +++ b/drivers/net/ethernet/sfc/efx.c > @@ -2448,6 +2448,7 @@ static void efx_unregister_netdev(struct efx_nic *efx) > #endif > device_remove_file(&efx->pci_dev->dev, &dev_attr_phy_type); > unregister_netdev(efx->net_dev); > + efx->state = STATE_UNINIT; > } > } > This isn't quite the right place, efx->state changes are supposed to be serialised by the RTNL lock. Our out-of-tree driver has this in efx_pci_remove, just after the efx_disable_interrupts(efx) call and before rtnl_unlock() (see patch below). I'd suggest that's the change we should make, but I haven't tested it yet. For reference, the "oversight" was in my e7fef9b45ae188066bb6eb3dde8310d33c2f7d5e "sfc: add sysfs entry to control MCDI tracing" which accidentally took our out-of-tree version of efx_unregister_netdev(). Before that the code was as in Jarod's patch. -Ed ----8<---- sfc: mark state UNINIT after unregister Without this change, modprobe -r sfc hits the BUG_ON() in efx_pci_remove_main(). Reported-by: Jarod Wilson Fixes: e7fef9b45ae188066bb6eb3dde8310d33c2f7d5e --- drivers/net/ethernet/sfc/efx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c index 0c42ed9..67bdaf3 100644 --- a/drivers/net/ethernet/sfc/efx.c +++ b/drivers/net/ethernet/sfc/efx.c @@ -2920,6 +2920,7 @@ static void efx_pci_remove(struct pci_dev *pci_dev) efx_dissociate(efx); dev_close(efx->net_dev); efx_disable_interrupts(efx); + efx->state = STATE_UNINIT; rtnl_unlock(); if (efx->type->sriov_fini)