Re: BQL support in gianfar causes network hickup
flat view
From: Eric Dumazet <hidden>
Date: 2012-11-26 17:17:38
Subsystem:
freescale etsec ethernet driver (gianfar), networking drivers, the rest · Maintainers:
Claudiu Manoil, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
On Mon, 2012-11-26 at 18:08 +0100, Keitel, Tino (ALC NetworX GmbH) wrote:
On Mo, 2012-11-26 at 08:34 -0800, Eric Dumazet wrote:quoted
On Mon, 2012-11-26 at 11:01 +0100, Tino Keitel wrote:quoted
On Sat, Nov 24, 2012 at 15:43:36 -0800, Eric Dumazet wrote: [...]quoted
Hmm, I wonder if BQL makes a particular bug showing more often. I see gianfar uses a very small watchdog_timeo of 1 second, while many drivers use 5 seconds. What happens if you change this to 5 seconds ?I still got the trace and a failing ptp client.Thanks. Is this bug easy to trigger ? I suspect a core issue and a race, likely to happen on your (non x86) hardware Could you add the following debugging patch ?No visible difference:
OK it seems you trigger the problem fast ! Please try the following as well :
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 19ac096..77190bf 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c@@ -101,7 +101,7 @@ #include "gianfar.h" -#define TX_TIMEOUT (1*HZ) +#define TX_TIMEOUT (5*HZ) const char gfar_driver_version[] = "1.3";
@@ -2465,9 +2465,9 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) struct sk_buff *skb; int skb_dirtytx; int tx_ring_size = tx_queue->tx_ring_size; - int frags = 0, nr_txbds = 0; + int frags, nr_txbds; int i; - int howmany = 0; + int howmany = 0, total_txbds = 0; int tqi = tx_queue->qindex; unsigned int bytes_sent = 0; u32 lstatus;
@@ -2479,7 +2479,6 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) skb_dirtytx = tx_queue->skb_dirtytx; while ((skb = tx_queue->tx_skbuff[skb_dirtytx])) { - unsigned long flags; frags = skb_shinfo(skb)->nr_frags;
@@ -2541,20 +2540,24 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) TX_RING_MOD_MASK(tx_ring_size); howmany++; - spin_lock_irqsave(&tx_queue->txlock, flags); - tx_queue->num_txbdfree += nr_txbds; - spin_unlock_irqrestore(&tx_queue->txlock, flags); + total_txbds += nr_txbds; } - /* If we freed a buffer, we can restart transmission, if necessary */ - if (netif_tx_queue_stopped(txq) && tx_queue->num_txbdfree) - netif_wake_subqueue(dev, tqi); + if (howmany) { + unsigned long flags; + spin_lock_irqsave(&tx_queue->txlock, flags); + tx_queue->num_txbdfree += total_txbds; + /* If we freed a buffer, we can restart transmission, if necessary */ + if (netif_tx_queue_stopped(txq)) + netif_tx_wake_queue(txq); + netdev_tx_completed_queue(txq, howmany, bytes_sent); + spin_unlock_irqrestore(&tx_queue->txlock, flags); + } /* Update dirty indicators */ tx_queue->skb_dirtytx = skb_dirtytx; tx_queue->dirty_tx = bdp; - netdev_tx_completed_queue(txq, howmany, bytes_sent); return howmany; }