forcedeth: add/modify tx done with limit

PATCH Applied Ayaz Abdulla 2 participants 3 messages Mar 5, 2009 – Mar 10, 2009

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Ayaz Abdulla

author

2009-03-05 21:01 UTC #0 lore raw

PATCH 6/13 forcedeth: add/modify tx done with limit

There are two tx_done routines to handle tx completion processing. Both these functions now take in a limit value and return the amount of tx completions. This will be used by a future patch to determine the total amount of work done.

Signed-off-by Ayaz Abdulla <[email protected]>

drivers/net/forcedeth.c +14 -6

@@ -2397,14 +2397,16 @@
  *
  * Caller must own np->lock.
  */
-static void nv_tx_done(struct net_device *dev)
+static int nv_tx_done(struct net_device *dev, int limit)
 {
 	struct fe_priv *np = netdev_priv(dev);
 	u32 flags;
+	int tx_work = 0;
 	struct ring_desc* orig_get_tx = np->get_tx.orig;
 
 	while ((np->get_tx.orig != np->put_tx.orig) &&
-	       !((flags = le32_to_cpu(np->get_tx.orig->flaglen)) & NV_TX_VALID)) {
+	       !((flags = le32_to_cpu(np->get_tx.orig->flaglen)) & NV_TX_VALID) &&
+	       (tx_work < limit)) {
 
 		dprintk(KERN_DEBUG "%s: nv_tx_done: flags 0x%x.\n",
 					dev->name, flags);

@@ -2430,6 +2432,7 @@
 				}
 				dev_kfree_skb_any(np->get_tx_ctx->skb);
 				np->get_tx_ctx->skb = NULL;
+				tx_work++;
 			}
 		} else {
 			if (flags & NV_TX2_LASTPACKET) {

@@ -2447,6 +2450,7 @@
 				}
 				dev_kfree_skb_any(np->get_tx_ctx->skb);
 				np->get_tx_ctx->skb = NULL;
+				tx_work++;
 			}
 		}
 		if (unlikely(np->get_tx.orig++ == np->last_tx.orig))

@@ -2458,17 +2462,19 @@
 		np->tx_stop = 0;
 		netif_wake_queue(dev);
 	}
+	return tx_work;
 }
 
-static void nv_tx_done_optimized(struct net_device *dev, int limit)
+static int nv_tx_done_optimized(struct net_device *dev, int limit)
 {
 	struct fe_priv *np = netdev_priv(dev);
 	u32 flags;
+	int tx_work = 0;
 	struct ring_desc_ex* orig_get_tx = np->get_tx.ex;
 
 	while ((np->get_tx.ex != np->put_tx.ex) &&
 	       !((flags = le32_to_cpu(np->get_tx.ex->flaglen)) & NV_TX_VALID) &&
-	       (limit-- > 0)) {
+	       (tx_work < limit)) {
 
 		dprintk(KERN_DEBUG "%s: nv_tx_done_optimized: flags 0x%x.\n",
 					dev->name, flags);

@@ -2492,6 +2498,7 @@
 
 			dev_kfree_skb_any(np->get_tx_ctx->skb);
 			np->get_tx_ctx->skb = NULL;
+			tx_work++;
 
 			if (np->tx_limit) {
 				nv_tx_flip_ownership(dev);

@@ -2506,6 +2513,7 @@
 		np->tx_stop = 0;
 		netif_wake_queue(dev);
 	}
+	return tx_work;
 }
 
 /*

@@ -2578,7 +2586,7 @@
 
 	/* 2) check that the packets were not sent already: */
 	if (!nv_optimized(np))
-		nv_tx_done(dev);
+		nv_tx_done(dev, np->tx_ring_size);
 	else
 		nv_tx_done_optimized(dev, np->tx_ring_size);

@@ -3433,7 +3441,7 @@
 		nv_msi_workaround(np);
 
 		spin_lock(&np->lock);
-		nv_tx_done(dev);
+		nv_tx_done(dev, np->tx_ring_size);
 		spin_unlock(&np->lock);
 
 #ifdef CONFIG_FORCEDETH_NAPI

David Miller

maintainer Applied

2009-03-10 12:32 UTC #1 lore raw

From: Ayaz Abdulla <[email protected]>
Date: Thu, 05 Mar 2009 13:02:10 -0500

Quoted 3 lines

There are two tx_done routines to handle tx completion processing. Both these functions now take in a limit value and return the amount of tx completions. This will be used by a future patch to determine the total amount of work done.
Signed-off-by Ayaz Abdulla <[email protected]>

Applied.

But I would absolutely not count TX completion processing in the NAPI work limit as you do in one of the subsequent patches.

TX completely is just buffer freeing, very cheap.

Whereas RX processing involves actually network stack processing which is real work and is what NAPI should be limiting.

Also, please get rid of the NAPI config option for this driver and make NAPI unconditionally enabled.

Thanks.

Ayaz Abdulla

author

2009-03-10 17:55 UTC #2 lore raw

Quoted 6 lines · David Miller

From: Ayaz Abdulla <[email protected]>
Date: Thu, 05 Mar 2009 13:02:10 -0500
Quoted 2 lines
There are two tx_done routines to handle tx completion processing. Both these functions now take in a limit value and return the amount of tx completions. This will be used by a future patch to determine the total amount of work done.

Quoted 8 lines

Quoted 1 line
Signed-off-by Ayaz Abdulla <[email protected]>
Applied.
But I would absolutely not count TX completion processing in the NAPI work limit as you do in one of the subsequent patches.

I agree. However, I am counting tx work only for the purpose of interrupt moderation. The tx work count is not included in the NAPI limit check. ie.

+ nv_change_interrupt_mode(dev, tx_work + rx_work);

- if (pkts < budget) {
+ if (rx_work < budget) {

Quoted 8 lines

TX completely is just buffer freeing, very cheap.
Whereas RX processing involves actually network stack processing which is real work and is what NAPI should be limiting.
Also, please get rid of the NAPI config option for this driver and make NAPI unconditionally enabled.

Sure, I will submit a new patch for removing the config option.

Quoted 1 line

Thanks.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------