Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1374060 > unrolled thread

[PATCH V3 0/8] net: mediatek: make the driver pass stress tests

Back to article view | Back to linux.kernel

Contents

  [PATCH V3 0/8] net: mediatek: make the driver pass stress tests John Crispin <blogic@openwrt.org> - 2016-04-08 10:00 +0200
    [PATCH V3 5/8] net: mediatek: fix TX locking John Crispin <blogic@openwrt.org> - 2016-04-08 10:00 +0200
    Re: [PATCH V3 0/8] net: mediatek: make the driver pass stress tests David Miller <davem@davemloft.net> - 2016-04-13 04:50 +0200

#1374060 — [PATCH V3 0/8] net: mediatek: make the driver pass stress tests

While testing the driver we managed to get the TX path to stall and fail
to recover. When dual MAC support was added to the driver, the whole queue
stop/wake code was not properly adapted. There was also a regression in the
locking of the xmit function. The fact that watchdog_timeo was not set and
that the tx_timeout code failed to properly reset the dma, irq and queue
just made the mess complete.

This series make the driver pass stress testing. With this series applied
the testbed has been running for several days and still has not locked up.
We have a second setup that has a small hack patch applied to randomly stop
irqs and/or one of the queues and successfully manages to recover from these
simulated tx stalls.

John Crispin (8):
  net: mediatek: watchdog_timeo was not set
  net: mediatek: mtk_cal_txd_req() returns bad value
  net: mediatek: remove superfluous reset call
  net: mediatek: fix stop and wakeup of queue
  net: mediatek: fix TX locking
  net: mediatek: fix mtk_pending_work
  net: mediatek: move the pending_work struct to the device generic
    struct
  net: mediatek: do not set the QID field in the TX DMA descriptors

 drivers/net/ethernet/mediatek/mtk_eth_soc.c |  106 ++++++++++++++++-----------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |    4 +-
 2 files changed, 66 insertions(+), 44 deletions(-)

-- 
1.7.10.4

[toc] | [next] | [standalone]

#1374061 — [PATCH V3 5/8] net: mediatek: fix TX locking

Inside the TX path there is a lock inside the tx_map function. This is
however too late. The patch moves the lock to the start of the xmit
function right before the free count check of the DMA ring happens.
If we do not do this, the code becomes racy leading to TX stalls and
dropped packets. This happens as there are 2 netdevs running on the
same physical DMA ring.

Signed-off-by: John Crispin <blogic@openwrt.org>

---
Changes in V3
* fix a typo in the comment

 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 4ebc42e..7b76075 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -536,7 +536,6 @@ static int mtk_tx_map(struct sk_buff *skb, struct net_device *dev,
 	struct mtk_eth *eth = mac->hw;
 	struct mtk_tx_dma *itxd, *txd;
 	struct mtk_tx_buf *tx_buf;
-	unsigned long flags;
 	dma_addr_t mapped_addr;
 	unsigned int nr_frags;
 	int i, n_desc = 1;
@@ -568,11 +567,6 @@ static int mtk_tx_map(struct sk_buff *skb, struct net_device *dev,
 	if (unlikely(dma_mapping_error(&dev->dev, mapped_addr)))
 		return -ENOMEM;
 
-	/* normally we can rely on the stack not calling this more than once,
-	 * however we have 2 queues running ont he same ring so we need to lock
-	 * the ring access
-	 */
-	spin_lock_irqsave(&eth->page_lock, flags);
 	WRITE_ONCE(itxd->txd1, mapped_addr);
 	tx_buf->flags |= MTK_TX_FLAGS_SINGLE0;
 	dma_unmap_addr_set(tx_buf, dma_addr0, mapped_addr);
@@ -632,8 +626,6 @@ static int mtk_tx_map(struct sk_buff *skb, struct net_device *dev,
 	WRITE_ONCE(itxd->txd3, (TX_DMA_SWC | TX_DMA_PLEN0(skb_headlen(skb)) |
 				(!nr_frags * TX_DMA_LS0)));
 
-	spin_unlock_irqrestore(&eth->page_lock, flags);
-
 	netdev_sent_queue(dev, skb->len);
 	skb_tx_timestamp(skb);
 
@@ -661,8 +653,6 @@ err_dma:
 		itxd = mtk_qdma_phys_to_virt(ring, itxd->txd2);
 	} while (itxd != txd);
 
-	spin_unlock_irqrestore(&eth->page_lock, flags);
-
 	return -ENOMEM;
 }
 
@@ -712,14 +702,22 @@ static int mtk_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct mtk_eth *eth = mac->hw;
 	struct mtk_tx_ring *ring = &eth->tx_ring;
 	struct net_device_stats *stats = &dev->stats;
+	unsigned long flags;
 	bool gso = false;
 	int tx_num;
 
+	/* normally we can rely on the stack not calling this more than once,
+	 * however we have 2 queues running on the same ring so we need to lock
+	 * the ring access
+	 */
+	spin_lock_irqsave(&eth->page_lock, flags);
+
 	tx_num = mtk_cal_txd_req(skb);
 	if (unlikely(atomic_read(&ring->free_count) <= tx_num)) {
 		mtk_stop_queue(eth);
 		netif_err(eth, tx_queued, dev,
 			  "Tx Ring full when queue awake!\n");
+		spin_unlock_irqrestore(&eth->page_lock, flags);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -747,10 +745,12 @@ static int mtk_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			     ring->thresh))
 			mtk_wake_queue(eth);
 	}
+	spin_unlock_irqrestore(&eth->page_lock, flags);
 
 	return NETDEV_TX_OK;
 
 drop:
+	spin_unlock_irqrestore(&eth->page_lock, flags);
 	stats->tx_dropped++;
 	dev_kfree_skb(skb);
 	return NETDEV_TX_OK;
-- 
1.7.10.4

[toc] | [prev] | [next] | [standalone]

#1377478

From: John Crispin <blogic@openwrt.org>
Date: Fri,  8 Apr 2016 00:54:03 +0200

> While testing the driver we managed to get the TX path to stall and fail
> to recover. When dual MAC support was added to the driver, the whole queue
> stop/wake code was not properly adapted. There was also a regression in the
> locking of the xmit function. The fact that watchdog_timeo was not set and
> that the tx_timeout code failed to properly reset the dma, irq and queue
> just made the mess complete.
> 
> This series make the driver pass stress testing. With this series applied
> the testbed has been running for several days and still has not locked up.
> We have a second setup that has a small hack patch applied to randomly stop
> irqs and/or one of the queues and successfully manages to recover from these
> simulated tx stalls.

Series applied, thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web