netdev - RE: [PATCH RFC 18/30] net: fec: remove inappropriate calls around fec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <869ac1ad635844ee84adbecd2eb0d0dc@BLUPR03MB373.namprd03.prod.outlook.com>
Date:	Sun, 22 Jun 2014 06:54:28 +0000
From:	"fugang.duan@...escale.com" <fugang.duan@...escale.com>
To:	Russell King <rmk+kernel@....linux.org.uk>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH RFC 18/30] net: fec: remove inappropriate calls around
 fec_restart()

From: Russell King <rmk@....linux.org.uk> Data: Friday, June 20, 2014 8:13 PM
>To: linux-arm-kernel@...ts.infradead.org
>Cc: Duan Fugang-B38611; netdev@...r.kernel.org
>Subject: [PATCH RFC 18/30] net: fec: remove inappropriate calls around
>fec_restart()
>
>This is the second stage to "move calls to quiesce/resume packet
>processing out of fec_restart()", where we remove calls which are not
>appropriate to the call site.
>
>In the majority of cases, there is no need to detach and reattach the
>interface as we are holding the queue xmit lock across the reset.  The
>exception to that is in fec_resume(), where we are already detached by the
>suspend function.  Here, we can remove the call to detach the interface.
>
>We also do not need to stop the transmit queue.  Holding the xmit lock is
>enough to ensure that the transmit packet processing is not running while
>we perform our task.  However, since fec_restart() always cleans the rings,
>we call netif_wake_queue() (or netif_device_attach() in the case of resume)
>just before dropping the xmit lock.  This prevents the watchdog firing.
>
>Lastly, always call napi_enable() after the device has been reattached in
>the resume path so that we know that the transmit packet processing is
>already in an enabled state, so we don't call netif_wake_queue() while
>detached.
>
>Signed-off-by: Russell King <rmk+kernel@....linux.org.uk>
>---
> drivers/net/ethernet/freescale/fec_main.c | 26 ++++++--------------------
> 1 file changed, 6 insertions(+), 20 deletions(-)
>
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index 4a295b4bfb94..49c154af6da2 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -1058,15 +1058,12 @@ static void fec_enet_work(struct work_struct *work)
> 		fep->delay_work.timeout = false;
> 		rtnl_lock();
> 		if (netif_device_present(ndev) || netif_running(ndev)) {
>-			netif_device_detach(ndev);
> 			napi_disable(&fep->napi);
>-			netif_tx_disable(ndev);
> 			netif_tx_lock_bh(ndev);
> 			fec_restart(ndev, fep->full_duplex);
>-			netif_tx_unlock_bh(ndev);
> 			netif_wake_queue(ndev);
>+			netif_tx_unlock_bh(ndev);
> 			napi_enable(&fep->napi);
>-			netif_device_attach(ndev);
> 		}
> 		rtnl_unlock();
> 	}
>@@ -1524,15 +1521,12 @@ static void fec_enet_adjust_link(struct net_device
>*ndev)
>
> 		/* if any of the above changed restart the FEC */
> 		if (status_change) {
>-			netif_device_detach(ndev);
> 			napi_disable(&fep->napi);
>-			netif_tx_disable(ndev);
> 			netif_tx_lock_bh(ndev);
> 			fec_restart(ndev, phy_dev->duplex);
>-			netif_tx_unlock_bh(ndev);
> 			netif_wake_queue(ndev);
>+			netif_tx_unlock_bh(ndev);
> 			napi_enable(&fep->napi);
>-			netif_device_attach(ndev);
> 		}
> 	} else {
> 		if (fep->link) {
>@@ -1919,15 +1913,12 @@ static int fec_enet_set_pauseparam(struct
>net_device *ndev,
> 		phy_start_aneg(fep->phy_dev);
> 	}
> 	if (netif_running(ndev)) {
>-		netif_device_detach(ndev);
> 		napi_disable(&fep->napi);
>-		netif_tx_disable(ndev);
> 		netif_tx_lock_bh(ndev);
> 		fec_restart(ndev, fep->full_duplex);
>-		netif_tx_unlock_bh(ndev);
> 		netif_wake_queue(ndev);
>+		netif_tx_unlock_bh(ndev);
> 		napi_enable(&fep->napi);
>-		netif_device_attach(ndev);
> 	}
>
> 	return 0;
>@@ -2372,15 +2363,12 @@ static int fec_set_features(struct net_device
>*netdev,
>
> 		if (netif_running(netdev)) {
> 			fec_stop(netdev);
>-			netif_device_detach(netdev);
> 			napi_disable(&fep->napi);
>-			netif_tx_disable(netdev);
> 			netif_tx_lock_bh(netdev);
> 			fec_restart(netdev, fep->phy_dev->duplex);
>-			netif_tx_unlock_bh(netdev);
> 			netif_wake_queue(netdev);
>+			netif_tx_unlock_bh(netdev);
> 			napi_enable(&fep->napi);
>-			netif_device_attach(netdev);
> 		}
> 	}
>
>@@ -2748,15 +2736,13 @@ fec_resume(struct device *dev)
>
> 	rtnl_lock();
> 	if (netif_running(ndev)) {
>-		netif_device_detach(ndev);
> 		napi_disable(&fep->napi);
>-		netif_tx_disable(ndev);
> 		netif_tx_lock_bh(ndev);
> 		fec_restart(ndev, fep->full_duplex);
>+		netif_device_attach(ndev);
> 		netif_tx_unlock_bh(ndev);
>-		netif_wake_queue(ndev);
>-		napi_enable(&fep->napi);
> 		netif_device_attach(ndev);
>+		napi_enable(&fep->napi);
> 		phy_start(fep->phy_dev);
> 	}
> 	rtnl_unlock();
>--
Patch #17 anc #18 can merge to one patch.

Thanks,
Andy
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html