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

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

[Jeff Kirsher]

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(-)

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);
 

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

[Jeff Garzik]

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

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

[Waskiewicz Jr, Peter P]

> 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