Skip to contentSkip to conversation

vxge: Use fifo based trans_start time

PATCH In review Jon Mason 2 participants 2 messages Jul 8, 2010 – Jul 9, 2010

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

Jon Mason

author
#0 lore raw

PATCH 2/6 vxge: Use fifo based trans_start time

Use the fifo based netdev_queue->trans_start time in vxge_xmit, instead of the device based netdev->trans_start time.

Signed-off-by Jon Mason <[email protected]>
Signed-off-by Sreenivasa Honnur <[email protected]>
Signed-off-by Ramkrishna Vepa <[email protected]>
---
 drivers/net/vxge/vxge-main.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/net/vxge/vxge-main.c +4 -1
index e78703d..b8eed71 100644
@@ -802,6 +802,7 @@ vxge_xmit(struct sk_buff *skb, struct net_device *dev)
 	unsigned long flags = 0;
 	int vpath_no = 0;
 	int do_spin_tx_lock = 1;
+	struct netdev_queue *txq;
 
 	vxge_debug_entryexit(VXGE_TRACE, "%s: %s:%d",
 			dev->name, __func__, __LINE__);

@@ -968,8 +969,10 @@ vxge_xmit(struct sk_buff *skb, struct net_device *dev)
 					VXGE_HW_FIFO_TXD_TX_CKO_UDP_EN);
 
 	vxge_hw_fifo_txdl_post(fifo_hw, dtr);
+
 #ifdef NETIF_F_LLTX
-	dev->trans_start = jiffies; /* NETIF_F_LLTX driver :( */
+	txq = netdev_get_tx_queue(dev, vpath_no);
+	txq->trans_start = jiffies;
 #endif
 	spin_unlock_irqrestore(&fifo->tx_lock, flags);
maintainer
#1 lore raw

From: Jon Mason <[email protected]>
Date: Thu, 8 Jul 2010 14:21:26 -0500

drivers/net/vxge/vxge-main.c vxge_xmit(struct sk_buff *skb, struct net_device *dev) · line 976

Quoted 10 lines
drivers/net/vxge/vxge-main.c +3 -1
@@ -968,8 +969,10 @@ vxge_xmit(struct sk_buff *skb, struct net_device *dev)
 					VXGE_HW_FIFO_TXD_TX_CKO_UDP_EN);
 
 	vxge_hw_fifo_txdl_post(fifo_hw, dtr);
+
 #ifdef NETIF_F_LLTX
-	dev->trans_start = jiffies; /* NETIF_F_LLTX driver :( */
+	txq = netdev_get_tx_queue(dev, vpath_no);
+	txq->trans_start = jiffies;
 #endif

This comment was placed there not just for it's artistic value, you should heed what it's saying when making changes like this.

NETIF_F_LLTX drivers cannot use the per-txq trans_start mechanism, because doing so is racy.

The dev_watchdog() timer, which checks these ->trans_start values, can only synchornize with the driver by the traditional means, which is by taking the spinlock on the TX queue. This is bypassed by NETIF_F_LLTX drivers, so the driver can be in it's TX handler while the watchdog timer is trying to evaluate the trans_start state.

This is one of a many reasons why NETIF_F_LLTX is a bad idea and you should convert your driver away from it.