All of lore.kernel.org
 help / color / mirror / Atom feed
From: rmk+kernel@arm.linux.org.uk (Russell King)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH CFT 18/30] net: fec: remove inappropriate calls around fec_restart()
Date: Fri, 27 Jun 2014 16:20:25 +0100	[thread overview]
Message-ID: <E1X0Xwv-0001O2-8y@rmk-PC.arm.linux.org.uk> (raw)
In-Reply-To: <20140627151542.GL32514@n2100.arm.linux.org.uk>

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@arm.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();
-- 
1.8.3.1

WARNING: multiple messages have this Message-ID (diff)
From: Russell King <rmk+kernel@arm.linux.org.uk>
To: linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
Cc: Fugang Duan <B38611@freescale.com>
Subject: [PATCH CFT 18/30] net: fec: remove inappropriate calls around fec_restart()
Date: Fri, 27 Jun 2014 16:20:25 +0100	[thread overview]
Message-ID: <E1X0Xwv-0001O2-8y@rmk-PC.arm.linux.org.uk> (raw)
In-Reply-To: <20140627151542.GL32514@n2100.arm.linux.org.uk>

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@arm.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();
-- 
1.8.3.1

  parent reply	other threads:[~2014-06-27 15:20 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-27 15:15 [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches Russell King - ARM Linux
2014-06-27 15:15 ` Russell King - ARM Linux
2014-06-27 15:18 ` [PATCH CFT 01/30] net: fec: Don't clear IPV6 header checksum field when IP accelerator enable Russell King
2014-06-27 15:18   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 02/30] net: fec: iMX6 FEC does not support half-duplex gigabit Russell King
2014-06-27 15:19   ` Russell King
2014-07-08  6:46   ` Uwe Kleine-König
2014-07-08  6:46     ` Uwe Kleine-König
2014-07-08 11:47     ` Russell King - ARM Linux
2014-07-08 11:47       ` Russell King - ARM Linux
2014-06-27 15:19 ` [PATCH CFT 03/30] net: fec: fix ethtool set_pauseparam duplex bug Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 04/30] net: fec: fix interrupt handling races Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 05/30] net: fec: use netif_tx_disable() rather than netif_stop_queue() Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 06/30] net: fec: remove checking for NULL phy_dev in fec_enet_close() Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 07/30] net: fec: ensure that a disconnected phy isn't configured Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 08/30] net: fec: stop the phy before shutting down the MAC Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 09/30] net: fec: remove useless fep->opened Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 10/30] net: fec: make rx skb handling more robust Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 11/30] net: fec: clean up transmit descriptor setup Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:19 ` [PATCH CFT 12/30] net: fec: ensure fec_enet_free_buffers() properly cleans the rings Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 18:48   ` Sergei Shtylyov
2014-06-27 18:48     ` Sergei Shtylyov
2014-07-01 14:48     ` Russell King - ARM Linux
2014-07-01 14:48       ` Russell King - ARM Linux
2014-06-27 15:19 ` [PATCH CFT 13/30] net: fec: fix missing kmalloc() failure check in fec_enet_alloc_buffers() Russell King
2014-06-27 15:19   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 14/30] net: fec: improve safety of suspend/resume/transmit timeout paths Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 15/30] net: fec: ensure fec_enet_close() copes with resume failure Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 16/30] net: fec: only restart or stop the device if it is present and running Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 17/30] net: fec: move calls to quiesce/resume packet processing out of fec_restart() Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` Russell King [this message]
2014-06-27 15:20   ` [PATCH CFT 18/30] net: fec: remove inappropriate calls around fec_restart() Russell King
2014-06-27 15:20 ` [PATCH CFT 19/30] net: fec: quiesce packet processing before stopping device in fec_suspend() Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 20/30] net: fec: quiesce packet processing before stopping device in fec_set_features() Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 21/30] net: fec: quiesce packet processing before changing features Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 22/30] net: fec: quiesce packet processing when taking link down in fec_enet_adjust_link() Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 23/30] net: fec: clean up duplex mode handling Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:20 ` [PATCH CFT 24/30] net: fec: better implementation of iMX6 ERR006358 quirk Russell King
2014-06-27 15:20   ` Russell King
2014-06-27 15:21 ` [PATCH CFT 25/30] net: fec: replace delayed work with standard work Russell King
2014-06-27 15:21   ` Russell King
2014-06-27 15:21 ` [PATCH CFT 26/30] net: fec: clear receive interrupts before processing a packet Russell King
2014-06-27 15:21   ` Russell King
2014-06-27 15:40   ` Russell King - ARM Linux
2014-06-27 15:40     ` Russell King - ARM Linux
2014-06-27 15:21 ` [PATCH CFT 27/30] net: fec: reorder ethtool ops to match order in struct declaration Russell King
2014-06-27 15:21   ` Russell King
2014-06-27 15:21 ` [PATCH CFT 28/30] net: fec: add support for dumping transmit ring on timeout Russell King
2014-06-27 15:21   ` Russell King
2014-06-27 15:21 ` [PATCH CFT 29/30] net: fec: remove useless status check in tx reap path Russell King
2014-06-27 15:21   ` Russell King
2014-06-27 15:21 ` [PATCH CFT 30/30] net: fec: consolidate hwtstamp implementation Russell King
2014-06-27 15:21   ` Russell King
2014-07-01  3:23 ` [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches fugang.duan
2014-07-01 14:21 ` Nathan Lynch
2014-07-01 14:21   ` Nathan Lynch
2014-07-01 14:34   ` Russell King - ARM Linux
2014-07-01 14:34     ` Russell King - ARM Linux
2014-07-01 14:56     ` Nathan Lynch
2014-07-01 14:56       ` Nathan Lynch
2014-07-07 21:41 ` Russell King - ARM Linux
2014-07-07 21:41   ` Russell King - ARM Linux
2014-07-07 22:45   ` David Miller
2014-07-07 22:45     ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1X0Xwv-0001O2-8y@rmk-PC.arm.linux.org.uk \
    --to=rmk+kernel@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.