Netdev List

* Re: [PATCH net-next 1/2] net: stmmac: Rework coalesce timer and fix multi-queue races
From: Jose Abreu @ 2018-09-04  9:57 UTC (permalink / raw)
  To: Jerome Brunet, Jose Abreu, netdev
  Cc: Martin Blumenstingl, David S. Miller, Joao Pinto,
	Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <fff3f4ea8ca262bf8f5e6e2239f4776cded626a5.camel@baylibre.com>

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Hi Jerome,

On 03-09-2018 17:22, Jerome Brunet wrote:
>
> Situation is even worse with this.
> I'm using an NFS root filesystem. With your fixup, I'm not reaching the prompt
> anymore. Looks like a the same kind of network breakdown we had previously
>

I was able to reproduce your problem and the attached fixup patch
fixed it up for me. Can you please try?

Thanks and Best Regards,
Jose Miguel Abreu

[-- Attachment #2: fixup2.patch --]
[-- Type: text/x-patch, Size: 2485 bytes --]

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 14f890f2a970..2cf927ccb409 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1861,14 +1861,14 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int limit, u32 queue)
 {
 	struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
 	unsigned int bytes_compl = 0, pkts_compl = 0;
-	unsigned int entry, count = 0;
+	unsigned int entry;
 
 	netif_tx_lock(priv->dev);
 
 	priv->xstats.tx_clean++;
 
 	entry = tx_q->dirty_tx;
-	while ((entry != tx_q->cur_tx) && (count < limit)) {
+	while ((entry != tx_q->cur_tx) && (pkts_compl < limit)) {
 		struct sk_buff *skb = tx_q->tx_skbuff[entry];
 		struct dma_desc *p;
 		int status;
@@ -1884,8 +1884,6 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int limit, u32 queue)
 		if (unlikely(status & tx_dma_own))
 			break;
 
-		count++;
-
 		/* Make sure descriptor fields are read after reading
 		 * the own bit.
 		 */
@@ -1955,7 +1953,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int limit, u32 queue)
 	}
 	netif_tx_unlock(priv->dev);
 
-	return count;
+	return pkts_compl;
 }
 
 /**
@@ -2072,8 +2070,11 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
 		if (likely(status[chan] & handle_rx)) {
 			struct stmmac_rx_queue *rx_q = &priv->rx_queue[chan];
 
-			if (likely(napi_schedule_prep(&rx_q->napi)))
+			if (likely(napi_schedule_prep(&rx_q->napi))) {
+				stmmac_disable_dma_irq(priv, priv->ioaddr,
+						       rx_q->queue_index);
 				__napi_schedule(&rx_q->napi);
+			}
 		}
 	}
 
@@ -2085,8 +2086,11 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
 		if (status[chan] & handle_tx) {
 			struct stmmac_tx_queue *tx_q = &priv->tx_queue[chan];
 
-			if (likely(napi_schedule_prep(&tx_q->napi)))
+			if (likely(napi_schedule_prep(&tx_q->napi))) {
+				stmmac_disable_dma_irq(priv, priv->ioaddr,
+						       tx_q->queue_index);
 				__napi_schedule(&tx_q->napi);
+			}
 		}
 	}
 
@@ -2247,11 +2251,7 @@ static void stmmac_tx_timer(struct timer_list *t)
 	struct stmmac_tx_queue *tx_q = from_timer(tx_q, t, txtimer);
 	struct stmmac_priv *priv = tx_q->priv_data;
 
-	if (napi_schedule_prep(&tx_q->napi)) {
-		stmmac_disable_dma_irq(priv, priv->ioaddr, tx_q->queue_index);
-		__napi_schedule(&tx_q->napi);
-	}
-
+	stmmac_tx_clean(priv, ~0, tx_q->queue_index);
 	tx_q->tx_timer_active = 0;
 }
 

^ permalink raw reply related