diff mbox

ixgbe: Fix usage of netif_*_all_queues() with netif_carrier_{off|on}()

Message ID 20081107221608.13396.10693.stgit@gitlost.lost
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Kirsher, Jeffrey T Nov. 7, 2008, 10:16 p.m. UTC 
From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>

netif_carrier_off() is sufficient to stop Tx into the driver.  Stopping the Tx
queues is redundant and unnecessary.  By the same token, netif_carrier_on()
will be sufficient to re-enable Tx, so waking the queues is unnecessary.

Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)


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

Comments

Jeff Garzik Nov. 11, 2008, 8:33 a.m. UTC | #1 
Jeff Kirsher wrote:
> From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
> 
> netif_carrier_off() is sufficient to stop Tx into the driver.  Stopping the Tx
> queues is redundant and unnecessary.  By the same token, netif_carrier_on()
> will be sufficient to re-enable Tx, so waking the queues is unnecessary.

Is this a fix as the subject says (2.6.28), or an optimization as the 
description implies (2.6.29)?

AFAICS from a quick glance, things are not actually _broken_, are they?

	Jeff



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Waskiewicz Jr, Peter P Nov. 11, 2008, 8:37 a.m. UTC | #2 
> Jeff Kirsher wrote:

>> From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>

>>

>> netif_carrier_off() is sufficient to stop Tx into the driver.  Stopping the Tx

>> queues is redundant and unnecessary.  By the same token, netif_carrier_on()

>> will be sufficient to re-enable Tx, so waking the queues is unnecessary.


> Is this a fix as the subject says (2.6.28), or an optimization as the

> description implies (2.6.29)?


> AFAICS from a quick glance, things are not actually _broken_, are they?


>        Jeff


This can be applied to 2.6.29; it doesn't need 2.6.28 inclusion.  Nothing is broken here, it's just poor usage.  DaveM pointed this out to us awhile ago, and I've finally gotten around to getting the simple patch out to correct the unneeded overhead.

Cheers,
-PJ Waskiewicz
diff mbox

Patch

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 4c23d78..0748526 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -1998,6 +1998,9 @@  static int ixgbe_up_complete(struct ixgbe_adapter *adapter)
 
 	ixgbe_irq_enable(adapter);
 
+	/* enable transmits */
+	netif_tx_start_all_queues(netdev);
+
 	/* bring the link up in the watchdog, this could race with our first
 	 * link up interrupt but shouldn't be a problem */
 	adapter->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
@@ -3248,7 +3251,6 @@  static void ixgbe_watchdog_task(struct work_struct *work)
 			         (FLOW_TX ? "TX" : "None"))));
 
 			netif_carrier_on(netdev);
-			netif_tx_wake_all_queues(netdev);
 		} else {
 			/* Force detection of hung controller */
 			adapter->detect_tx_hung = true;
@@ -3259,7 +3261,6 @@  static void ixgbe_watchdog_task(struct work_struct *work)
 		if (netif_carrier_ok(netdev)) {
 			DPRINTK(LINK, INFO, "NIC Link is Down\n");
 			netif_carrier_off(netdev);
-			netif_tx_stop_all_queues(netdev);
 		}
 	}
 
@@ -3946,7 +3947,6 @@  static int __devinit ixgbe_probe(struct pci_dev *pdev,
 	}
 
 	netif_carrier_off(netdev);
-	netif_tx_stop_all_queues(netdev);
 
 	ixgbe_napi_add_all(adapter);