[PATCH net v6 0/4] tg3: tx_pending fixes

* [PATCH net v6 0/4] tg3: tx_pending fixes
@ 2014-09-05  1:30 Benjamin Poirier
  2014-09-05  1:30 ` [PATCH net v6 1/4] tg3: Limit minimum tx queue wakeup threshold Benjamin Poirier
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Benjamin Poirier @ 2014-09-05  1:30 UTC (permalink / raw)
  To: Prashant Sreedharan, Michael Chan; +Cc: netdev, linux-kernel


Extra info regarding patch 4:
This version of the series calls gso_segment() without NETIF_F_SG. This avoids
the need for desc_cnt_est in tg3_tso_bug() as in previous versions of this
patch series. Since Michael had previously raised concerns about gso_segment
without SG, I ran some netperf throughput tests. I used a small patch to force
tg3_tso_bug() to be called even when it is not needed [1].

root@linux-y64m:~# perf stat -r10 -ad netperf -H 192.168.9.30 -l60 -T 0,0 -t omni -- -d send

* original tg3_tso_bug() (ie. without patch 4/4)
  781±2 10^6bits/s
  6.60 cycle/bit
* gso_segment() without SG (current series)
  801.0±0.9 10^6bits/s
  5.79 cycle/bit
* gso_segment() with SG (alternate patch 4/4 [2])
  783±2 10^6bits/s
  7.25 cycle/bit

(For reference, with the original tg3_tso_bug() implementation but without
forcing it to be called, the throughput I get is 822±1 10^6bits/s @ 3.82
cycle/bit with 0 invocations of tg3_tso_bug)

[1] fault injection patch

---
 drivers/net/ethernet/broadcom/tg3.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index cb77ae9..f9144dc 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -47,6 +47,7 @@
 #include <linux/ssb/ssb_driver_gige.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
+#include <linux/debugfs.h>
 
 #include <net/checksum.h>
 #include <net/ip.h>
@@ -468,6 +469,27 @@ static const struct {
 #define TG3_NUM_TEST	ARRAY_SIZE(ethtool_test_keys)
 
 
+/* debugging stuff */
+static u32 tg3_do_mangle;
+static struct dentry *tg3_mangle_debugfs;
+
+static int __init tg3_mod_init(void)
+{
+	tg3_mangle_debugfs = debugfs_create_u32("tg3_do_mangle", S_IRUGO |
+						S_IWUSR, NULL,
+						&tg3_do_mangle);
+
+	return 0;
+}
+module_init(tg3_mod_init);
+
+static void __exit tg3_mod_exit(void)
+{
+	debugfs_remove(tg3_mangle_debugfs);
+}
+module_exit(tg3_mod_exit);
+/* --- */
+
 static void tg3_write32(struct tg3 *tp, u32 off, u32 val)
 {
 	writel(val, tp->regs + off);
@@ -8048,6 +8070,11 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 				would_hit_hwbug = 1;
 				break;
 			}
+
+			if (tg3_do_mangle > 0) {
+				would_hit_hwbug = 4;
+				break;
+			}
 		}
 	}
 
-- 

[2] alternate patch 4

call gso_segment with SG (without removing it, actually)

---
 drivers/net/ethernet/broadcom/tg3.c | 80 +++++++++++++++++++++++++++----------
 1 file changed, 59 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index ee93b51..1ecb393 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -205,6 +205,9 @@ static inline void _tg3_flag_clear(enum TG3_FLAGS flag, unsigned long *bits)
 /* minimum number of free TX descriptors required to wake up TX process */
 #define TG3_TX_WAKEUP_THRESH(tnapi)	max_t(u32, (tnapi)->tx_pending / 4, \
 					      MAX_SKB_FRAGS + 1)
+/* estimate a certain number of descriptors per gso segment */
+#define TG3_TX_DESC_PER_SEG(seg_nb)	((seg_nb) * 3)
+
 #define TG3_TX_BD_DMA_MAX_2K		2048
 #define TG3_TX_BD_DMA_MAX_4K		4096
 
@@ -7852,6 +7855,8 @@ static int tigon3_dma_hwbug_workaround(struct tg3_napi *tnapi,
 }
 
 static netdev_tx_t tg3_start_xmit(struct sk_buff *, struct net_device *);
+static netdev_tx_t __tg3_start_xmit(struct sk_buff *, struct net_device *,
+				    u32);
 
 /* Returns true if the queue has been stopped. Note that it may have been
  * restarted since.
@@ -7888,27 +7893,56 @@ static inline bool tg3_maybe_stop_txq(struct tg3_napi *tnapi,
 static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
 		       struct netdev_queue *txq, struct sk_buff *skb)
 {
-	struct sk_buff *segs, *nskb;
-	u32 frag_cnt_est = skb_shinfo(skb)->gso_segs * 3;
+	unsigned int segs_remaining = skb_shinfo(skb)->gso_segs;
+	u32 desc_cnt_est = TG3_TX_DESC_PER_SEG(segs_remaining);
 
-	/* Estimate the number of fragments in the worst case */
-	tg3_maybe_stop_txq(tnapi, txq, frag_cnt_est, frag_cnt_est);
-	if (netif_tx_queue_stopped(txq))
-		return NETDEV_TX_BUSY;
+	if (unlikely(tg3_tx_avail(tnapi) <= desc_cnt_est)) {
+		if (!skb_is_nonlinear(skb) || skb_linearize(skb))
+			goto tg3_tso_bug_drop;
+		tg3_start_xmit(skb, tp->dev);
+	} else {
+		struct sk_buff *segs, *nskb;
 
-	segs = skb_gso_segment(skb, tp->dev->features &
-				    ~(NETIF_F_TSO | NETIF_F_TSO6));
-	if (IS_ERR(segs) || !segs)
-		goto tg3_tso_bug_end;
+		segs = skb_gso_segment(skb, tp->dev->features &
+				       ~(NETIF_F_TSO | NETIF_F_TSO6));
+		if (IS_ERR(segs) || !segs)
+			goto tg3_tso_bug_drop;
 
-	do {
-		nskb = segs;
-		segs = segs->next;
-		nskb->next = NULL;
-		tg3_start_xmit(nskb, tp->dev);
-	} while (segs);
+		do {
+			unsigned int desc_cnt = skb_shinfo(segs)->nr_frags + 1;
+
+			nskb = segs;
+			segs = segs->next;
+			nskb->next = NULL;
+
+			if (tg3_tx_avail(tnapi) <= segs_remaining - 1 +
+			    desc_cnt && skb_linearize(nskb)) {
+				nskb->next = segs;
+				segs = nskb;
+				do {
+					nskb = segs->next;
+
+					dev_kfree_skb_any(segs);
+					segs = nskb;
+				} while (segs);
+				tg3_maybe_stop_txq(tnapi, txq, MAX_SKB_FRAGS + 1,
+						   TG3_TX_WAKEUP_THRESH(tnapi));
+
+				goto tg3_tso_bug_drop;
+			}
+			if (--segs_remaining)
+				__tg3_start_xmit(nskb, tp->dev, segs_remaining);
+			else
+				tg3_start_xmit(nskb, tp->dev);
+		} while (segs);
 
-tg3_tso_bug_end:
+		dev_kfree_skb_any(skb);
+	}
+
+	return NETDEV_TX_OK;
+
+tg3_tso_bug_drop:
+	tp->tx_dropped++;
 	dev_kfree_skb_any(skb);
 
 	return NETDEV_TX_OK;
@@ -7917,6 +7951,12 @@ tg3_tso_bug_end:
 /* hard_start_xmit for all devices */
 static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
+	return __tg3_start_xmit(skb, dev, MAX_SKB_FRAGS + 1);
+}
+
+static netdev_tx_t __tg3_start_xmit(struct sk_buff *skb,
+				    struct net_device *dev, u32 stop_thresh)
+{
 	struct tg3 *tp = netdev_priv(dev);
 	u32 len, entry, base_flags, mss, vlan = 0;
 	u32 budget;
@@ -8129,7 +8169,7 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	tw32_tx_mbox(tnapi->prodmbox, entry);
 
 	tnapi->tx_prod = entry;
-	tg3_maybe_stop_txq(tnapi, txq, MAX_SKB_FRAGS + 1,
+	tg3_maybe_stop_txq(tnapi, txq, stop_thresh,
 			   TG3_TX_WAKEUP_THRESH(tnapi));
 
 	mmiowb();
@@ -12363,9 +12403,7 @@ static int tg3_set_ringparam(struct net_device *dev, struct ethtool_ringparam *e
 	if ((ering->rx_pending > tp->rx_std_ring_mask) ||
 	    (ering->rx_jumbo_pending > tp->rx_jmb_ring_mask) ||
 	    (ering->tx_pending > TG3_TX_RING_SIZE - 1) ||
-	    (ering->tx_pending <= MAX_SKB_FRAGS + 1) ||
-	    (tg3_flag(tp, TSO_BUG) &&
-	     (ering->tx_pending <= (MAX_SKB_FRAGS * 3))))
+	    (ering->tx_pending <= MAX_SKB_FRAGS + 1))
 		return -EINVAL;
 
 	if (netif_running(dev)) {
-- 

^ permalink raw reply related	[flat|nested] 12+ messages in thread