Skip to contentSkip to conversation

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

PATCH In review Jeff Kirsher 3 participants 3 messages Nov 7, 2008 – Nov 11, 2008

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Jeff Kirsher

author
#0 lore raw

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

From: PJ Waskiewicz <[email protected]>

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 <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

---

 drivers/net/ixgbe/ixgbe_main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/net/ixgbe/ixgbe_main.c +3 -3
index 4c23d78..0748526 100644
@@ -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);
#1 lore raw
Quoted 5 lines · Jeff Kirsher

From: PJ Waskiewicz <[email protected]>

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

#2 lore raw
Quoted 7 lines
Quoted 5 lines · Jeff Kirsher

From: PJ Waskiewicz <[email protected]>

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.

Quoted 6 lines

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