* [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes
@ 2016-05-05 9:26 John Crispin
2016-05-05 9:26 ` [PATCH 01/12] net-next: mediatek: fix DQL support John Crispin
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw)
To: David S. Miller; +Cc: nbd, netdev, linux-mediatek, linux-kernel, John Crispin
This series is a collection of fixes for bugs that we stumble across while
doing more performance testing and code level review.
John Crispin (12):
net-next: mediatek: fix DQL support
net-next: mediatek: add missing return code check
net-next: mediatek: fix missing free of scratch memory
net-next: mediatek: invalid buffer lookup in mtk_tx_map()
net-next: mediatek: dropped rx packets are not being counted properly
net-next: mediatek: add next data pointer coherency protection
net-next: mediatek: disable all interrupts during probe
net-next: mediatek: fix threshold value
net-next: mediatek: increase watchdog_timeo
net-next: mediatek: fix off by one in the TX ring allocation
net-next: mediatek: only wake the queue if it is stopped
net-next: mediatek: remove superfluous queue wake up call
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 94 ++++++++++++++++++---------
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 3 +
2 files changed, 65 insertions(+), 32 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 01/12] net-next: mediatek: fix DQL support 2016-05-05 9:26 [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes John Crispin @ 2016-05-05 9:26 ` John Crispin [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> ` (5 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller; +Cc: nbd, netdev, linux-mediatek, linux-kernel, John Crispin The MTK ethernet core has 2 MACs both sitting on the same DMA ring. The current code will assign the TX traffic of each MAC to its own DQL. This results in the amount of data, that DQL says is in the queue incorrect. As the data from multiple devices is infact enqueued. This makes any decision based on these value non deterministic. Fix this by tracking all TX traffic, regardless of the MAC it belongs to in the DQL of all devices using the DMA. Signed-off-by: John Crispin <john@phrozen.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 33 ++++++++++++++++----------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 41cdc0d..07674e5 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -641,7 +641,16 @@ 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))); - netdev_sent_queue(dev, skb->len); + /* we have a single DMA ring so BQL needs to be updated for all devices + * sitting on this ring + */ + for (i = 0; i < MTK_MAC_COUNT; i++) { + if (!eth->netdev[i]) + continue; + + netdev_sent_queue(eth->netdev[i], skb->len); + } + skb_tx_timestamp(skb); ring->next_free = mtk_qdma_phys_to_virt(ring, txd->txd2); @@ -869,21 +878,18 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget, bool *tx_again) struct mtk_tx_dma *desc; struct sk_buff *skb; struct mtk_tx_buf *tx_buf; - int total = 0, done[MTK_MAX_DEVS]; - unsigned int bytes[MTK_MAX_DEVS]; + int total = 0, done = 0; + unsigned int bytes = 0; u32 cpu, dma; static int condition; int i; - memset(done, 0, sizeof(done)); - memset(bytes, 0, sizeof(bytes)); - cpu = mtk_r32(eth, MTK_QTX_CRX_PTR); dma = mtk_r32(eth, MTK_QTX_DRX_PTR); desc = mtk_qdma_phys_to_virt(ring, cpu); - while ((cpu != dma) && budget) { + while ((cpu != dma) && done < budget) { u32 next_cpu = desc->txd2; int mac; @@ -903,9 +909,8 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget, bool *tx_again) } if (skb != (struct sk_buff *)MTK_DMA_DUMMY_DESC) { - bytes[mac] += skb->len; - done[mac]++; - budget--; + bytes += skb->len; + done++; } mtk_tx_unmap(eth->dev, tx_buf); @@ -918,11 +923,13 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget, bool *tx_again) mtk_w32(eth, cpu, MTK_QTX_CRX_PTR); + /* we have a single DMA ring so BQL needs to be updated for all devices + * sitting on this ring + */ for (i = 0; i < MTK_MAC_COUNT; i++) { - if (!eth->netdev[i] || !done[i]) + if (!eth->netdev[i]) continue; - netdev_completed_queue(eth->netdev[i], done[i], bytes[i]); - total += done[i]; + netdev_completed_queue(eth->netdev[i], done, bytes); } /* read hw index again make sure no new tx packet */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
[parent not found: <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>]
* [PATCH 02/12] net-next: mediatek: add missing return code check [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 03/12] net-next: mediatek: fix missing free of scratch memory John Crispin ` (5 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, John Crispin, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, nbd-Vt+b4OUoWG0 The code fails to check if the scratch memory was properly allocated. Add this check and return with an error if the allocation failed. Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 07674e5..0ed49a4 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -483,6 +483,9 @@ static int mtk_init_fq_dma(struct mtk_eth *eth) eth->scratch_head = kcalloc(cnt, MTK_QDMA_PAGE_SIZE, GFP_KERNEL); + if (unlikely(!eth->scratch_head)) + return -ENOMEM; + dma_addr = dma_map_single(eth->dev, eth->scratch_head, cnt * MTK_QDMA_PAGE_SIZE, DMA_FROM_DEVICE); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 03/12] net-next: mediatek: fix missing free of scratch memory [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> 2016-05-05 9:26 ` [PATCH 02/12] net-next: mediatek: add missing return code check John Crispin @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 04/12] net-next: mediatek: invalid buffer lookup in mtk_tx_map() John Crispin ` (4 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, John Crispin, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, nbd-Vt+b4OUoWG0 Scratch memory gets allocated in mtk_init_fq_dma() but the corresponding code to free it is missing inside mtk_dma_free() causing a memory leak. With this patch applied, we can run ifconfig/up/down several thousand times without any problems. Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 18 +++++++++++++----- drivers/net/ethernet/mediatek/mtk_eth_soc.h | 2 ++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 0ed49a4..e415986 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -469,14 +469,14 @@ static inline void mtk_rx_get_desc(struct mtk_rx_dma *rxd, /* the qdma core needs scratch memory to be setup */ static int mtk_init_fq_dma(struct mtk_eth *eth) { - dma_addr_t phy_ring_head, phy_ring_tail; + dma_addr_t phy_ring_tail; int cnt = MTK_DMA_SIZE; dma_addr_t dma_addr; int i; eth->scratch_ring = dma_alloc_coherent(eth->dev, cnt * sizeof(struct mtk_tx_dma), - &phy_ring_head, + ð->phy_scratch_ring, GFP_ATOMIC | __GFP_ZERO); if (unlikely(!eth->scratch_ring)) return -ENOMEM; @@ -493,19 +493,19 @@ static int mtk_init_fq_dma(struct mtk_eth *eth) return -ENOMEM; memset(eth->scratch_ring, 0x0, sizeof(struct mtk_tx_dma) * cnt); - phy_ring_tail = phy_ring_head + + phy_ring_tail = eth->phy_scratch_ring + (sizeof(struct mtk_tx_dma) * (cnt - 1)); for (i = 0; i < cnt; i++) { eth->scratch_ring[i].txd1 = (dma_addr + (i * MTK_QDMA_PAGE_SIZE)); if (i < cnt - 1) - eth->scratch_ring[i].txd2 = (phy_ring_head + + eth->scratch_ring[i].txd2 = (eth->phy_scratch_ring + ((i + 1) * sizeof(struct mtk_tx_dma))); eth->scratch_ring[i].txd3 = TX_DMA_SDL(MTK_QDMA_PAGE_SIZE); } - mtk_w32(eth, phy_ring_head, MTK_QDMA_FQ_HEAD); + mtk_w32(eth, eth->phy_scratch_ring, MTK_QDMA_FQ_HEAD); mtk_w32(eth, phy_ring_tail, MTK_QDMA_FQ_TAIL); mtk_w32(eth, (cnt << 16) | cnt, MTK_QDMA_FQ_CNT); mtk_w32(eth, MTK_QDMA_PAGE_SIZE << 16, MTK_QDMA_FQ_BLEN); @@ -1205,6 +1205,14 @@ static void mtk_dma_free(struct mtk_eth *eth) for (i = 0; i < MTK_MAC_COUNT; i++) if (eth->netdev[i]) netdev_reset_queue(eth->netdev[i]); + if (eth->scratch_ring) { + dma_free_coherent(eth->dev, + MTK_DMA_SIZE * sizeof(struct mtk_tx_dma), + eth->scratch_ring, + eth->phy_scratch_ring); + eth->scratch_ring = NULL; + eth->phy_scratch_ring = 0; + } mtk_tx_clean(eth); mtk_rx_clean(eth); kfree(eth->scratch_head); diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h index eed626d..57f7e8a 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h @@ -357,6 +357,7 @@ struct mtk_rx_ring { * @rx_ring: Pointer to the memore holding info about the RX ring * @rx_napi: The NAPI struct * @scratch_ring: Newer SoCs need memory for a second HW managed TX ring + * @phy_scratch_ring: physical address of scratch_ring * @scratch_head: The scratch memory that scratch_ring points to. * @clk_ethif: The ethif clock * @clk_esw: The switch clock @@ -384,6 +385,7 @@ struct mtk_eth { struct mtk_rx_ring rx_ring; struct napi_struct rx_napi; struct mtk_tx_dma *scratch_ring; + dma_addr_t phy_scratch_ring; void *scratch_head; struct clk *clk_ethif; struct clk *clk_esw; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 04/12] net-next: mediatek: invalid buffer lookup in mtk_tx_map() [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> 2016-05-05 9:26 ` [PATCH 02/12] net-next: mediatek: add missing return code check John Crispin 2016-05-05 9:26 ` [PATCH 03/12] net-next: mediatek: fix missing free of scratch memory John Crispin @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 05/12] net-next: mediatek: dropped rx packets are not being counted properly John Crispin ` (3 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, John Crispin, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, nbd-Vt+b4OUoWG0 The lookup of the tx_buffer in the error path inside mtk_tx_map() uses the wrong descriptor pointer. This looks like a copy & paste error. Change the code to use the correct pointer. Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index e415986..7fdd1a3 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -671,7 +671,7 @@ static int mtk_tx_map(struct sk_buff *skb, struct net_device *dev, err_dma: do { - tx_buf = mtk_desc_to_tx_buf(ring, txd); + tx_buf = mtk_desc_to_tx_buf(ring, itxd); /* unmap dma */ mtk_tx_unmap(&dev->dev, tx_buf); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 05/12] net-next: mediatek: dropped rx packets are not being counted properly [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> ` (2 preceding siblings ...) 2016-05-05 9:26 ` [PATCH 04/12] net-next: mediatek: invalid buffer lookup in mtk_tx_map() John Crispin @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 06/12] net-next: mediatek: add next data pointer coherency protection John Crispin ` (2 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, John Crispin, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, nbd-Vt+b4OUoWG0 There are 2 places inside mtk_poll_rx where rx_dropped is not being incremented properly. Fix this by adding the missing code to increment the counter. Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 7fdd1a3..2e79af9 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -826,6 +826,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget, DMA_FROM_DEVICE); if (unlikely(dma_mapping_error(&netdev->dev, dma_addr))) { skb_free_frag(new_data); + netdev->stats.rx_dropped++; goto release_desc; } @@ -833,6 +834,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget, skb = build_skb(data, ring->frag_size); if (unlikely(!skb)) { put_page(virt_to_head_page(new_data)); + netdev->stats.rx_dropped++; goto release_desc; } skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 06/12] net-next: mediatek: add next data pointer coherency protection [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> ` (3 preceding siblings ...) 2016-05-05 9:26 ` [PATCH 05/12] net-next: mediatek: dropped rx packets are not being counted properly John Crispin @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 07/12] net-next: mediatek: disable all interrupts during probe John Crispin 2016-05-05 9:26 ` [PATCH 08/12] net-next: mediatek: fix threshold value John Crispin 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, John Crispin, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, nbd-Vt+b4OUoWG0 The QDMA engine can fail to update the register pointing to the next TX descriptor if this bit does not get set in the QDMA configuration register. Not setting this bit can result in invalid values inside the TX rings registers which will causes TX stalls. Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +- drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 2e79af9..60095be 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -1277,7 +1277,7 @@ static int mtk_start_dma(struct mtk_eth *eth) mtk_w32(eth, MTK_TX_WB_DDONE | MTK_RX_DMA_EN | MTK_TX_DMA_EN | MTK_RX_2B_OFFSET | MTK_DMA_SIZE_16DWORDS | - MTK_RX_BT_32DWORDS, + MTK_RX_BT_32DWORDS | MTK_NDP_CO_PRO, MTK_QDMA_GLO_CFG); return 0; diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h index 57f7e8a..a5eb7c6 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h @@ -91,6 +91,7 @@ #define MTK_QDMA_GLO_CFG 0x1A04 #define MTK_RX_2B_OFFSET BIT(31) #define MTK_RX_BT_32DWORDS (3 << 11) +#define MTK_NDP_CO_PRO BIT(10) #define MTK_TX_WB_DDONE BIT(6) #define MTK_DMA_SIZE_16DWORDS (2 << 4) #define MTK_RX_DMA_BUSY BIT(3) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 07/12] net-next: mediatek: disable all interrupts during probe [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> ` (4 preceding siblings ...) 2016-05-05 9:26 ` [PATCH 06/12] net-next: mediatek: add next data pointer coherency protection John Crispin @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 08/12] net-next: mediatek: fix threshold value John Crispin 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, John Crispin, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, nbd-Vt+b4OUoWG0 The current code only disables those IRQs that we will later use. To ensure that we have a predefined state, we really want to disable all IRQs. Change the code to disable all IRQs to achieve this. Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 60095be..08f3df4 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -1391,7 +1391,7 @@ static int __init mtk_hw_init(struct mtk_eth *eth) /* disable delay and normal interrupt */ mtk_w32(eth, 0, MTK_QDMA_DELAY_INT); - mtk_irq_disable(eth, MTK_TX_DONE_INT | MTK_RX_DONE_INT); + mtk_irq_disable(eth, ~0); mtk_w32(eth, RST_GL_PSE, MTK_RST_GL); mtk_w32(eth, 0, MTK_RST_GL); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 08/12] net-next: mediatek: fix threshold value [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> ` (5 preceding siblings ...) 2016-05-05 9:26 ` [PATCH 07/12] net-next: mediatek: disable all interrupts during probe John Crispin @ 2016-05-05 9:26 ` John Crispin 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller Cc: netdev-u79uwXL29TY76Z2rM5mHXA, John Crispin, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, nbd-Vt+b4OUoWG0 The logic to calculate the threshold value for stopping the TX queue is bad. Currently it will always use 1/2 of the rings size, which is way too much. Set the threshold to MAX_SKB_FRAGS. This makes sure that the queue is stopped when there is not enough room to accept an additional segment. Signed-off-by: John Crispin <john@phrozen.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 08f3df4..4439991 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -1028,8 +1028,7 @@ static int mtk_tx_alloc(struct mtk_eth *eth) atomic_set(&ring->free_count, MTK_DMA_SIZE - 2); ring->next_free = &ring->dma[0]; ring->last_free = &ring->dma[MTK_DMA_SIZE - 2]; - ring->thresh = max((unsigned long)MTK_DMA_SIZE >> 2, - MAX_SKB_FRAGS); + ring->thresh = MAX_SKB_FRAGS; /* make sure that all changes to the dma ring are flushed before we * continue -- 1.7.10.4 _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 09/12] net-next: mediatek: increase watchdog_timeo 2016-05-05 9:26 [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes John Crispin 2016-05-05 9:26 ` [PATCH 01/12] net-next: mediatek: fix DQL support John Crispin [not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org> @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 10/12] net-next: mediatek: fix off by one in the TX ring allocation John Crispin ` (3 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller; +Cc: nbd, netdev, linux-mediatek, linux-kernel, John Crispin During stress testing, after reducing the threshold value, we have seen TX timeouts that were caused by the watchdog_timeo value being too low. Increase the value to 5 * HZ which is a value commonly used by many other drivers. Signed-off-by: John Crispin <john@phrozen.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 4439991..12e9f1a 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -1704,7 +1704,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np) mac->hw_stats->reg_offset = id * MTK_STAT_OFFSET; SET_NETDEV_DEV(eth->netdev[id], eth->dev); - eth->netdev[id]->watchdog_timeo = HZ; + eth->netdev[id]->watchdog_timeo = 5 * HZ; eth->netdev[id]->netdev_ops = &mtk_netdev_ops; eth->netdev[id]->base_addr = (unsigned long)eth->base; eth->netdev[id]->vlan_features = MTK_HW_FEATURES & -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 10/12] net-next: mediatek: fix off by one in the TX ring allocation 2016-05-05 9:26 [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes John Crispin ` (2 preceding siblings ...) 2016-05-05 9:26 ` [PATCH 09/12] net-next: mediatek: increase watchdog_timeo John Crispin @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 11/12] net-next: mediatek: only wake the queue if it is stopped John Crispin ` (2 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller; +Cc: nbd, netdev, linux-mediatek, linux-kernel, John Crispin The TX ring setup has an off by one error causing it to not utilise all descriptors. This has the side effect that we need to reset the next pointer at runtime to make it work. Fix the off by one and remove the code fixing the ring at runtime. Signed-off-by: John Crispin <john@phrozen.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 12e9f1a..81155a1 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -919,7 +919,6 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget, bool *tx_again) } mtk_tx_unmap(eth->dev, tx_buf); - ring->last_free->txd2 = next_cpu; ring->last_free = desc; atomic_inc(&ring->free_count); @@ -1027,7 +1026,7 @@ static int mtk_tx_alloc(struct mtk_eth *eth) atomic_set(&ring->free_count, MTK_DMA_SIZE - 2); ring->next_free = &ring->dma[0]; - ring->last_free = &ring->dma[MTK_DMA_SIZE - 2]; + ring->last_free = &ring->dma[MTK_DMA_SIZE - 1]; ring->thresh = MAX_SKB_FRAGS; /* make sure that all changes to the dma ring are flushed before we -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 11/12] net-next: mediatek: only wake the queue if it is stopped 2016-05-05 9:26 [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes John Crispin ` (3 preceding siblings ...) 2016-05-05 9:26 ` [PATCH 10/12] net-next: mediatek: fix off by one in the TX ring allocation John Crispin @ 2016-05-05 9:26 ` John Crispin 2016-05-05 9:26 ` [PATCH 12/12] net-next: mediatek: remove superfluous queue wake up call John Crispin 2016-05-05 12:25 ` [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes Andrew Lunn 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller; +Cc: nbd, netdev, linux-mediatek, linux-kernel, John Crispin The current code unconditionally wakes up the queue at the end of each tx_poll action. Change the code to only wake up the queues if any of them have actually been stopped before. Signed-off-by: John Crispin <john@phrozen.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 81155a1..2d3c464 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -701,6 +701,20 @@ static inline int mtk_cal_txd_req(struct sk_buff *skb) return nfrags; } +static int mtk_queue_stopped(struct mtk_eth *eth) +{ + int i; + + for (i = 0; i < MTK_MAC_COUNT; i++) { + if (!eth->netdev[i]) + continue; + if (netif_queue_stopped(eth->netdev[i])) + return 1; + } + + return 0; +} + static void mtk_wake_queue(struct mtk_eth *eth) { int i; @@ -945,7 +959,8 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget, bool *tx_again) if (!total) return 0; - if (atomic_read(&ring->free_count) > ring->thresh) + if (mtk_queue_stopped(eth) && + (atomic_read(&ring->free_count) > ring->thresh)) mtk_wake_queue(eth); return total; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 12/12] net-next: mediatek: remove superfluous queue wake up call 2016-05-05 9:26 [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes John Crispin ` (4 preceding siblings ...) 2016-05-05 9:26 ` [PATCH 11/12] net-next: mediatek: only wake the queue if it is stopped John Crispin @ 2016-05-05 9:26 ` John Crispin 2016-05-05 12:25 ` [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes Andrew Lunn 6 siblings, 0 replies; 14+ messages in thread From: John Crispin @ 2016-05-05 9:26 UTC (permalink / raw) To: David S. Miller; +Cc: nbd, netdev, linux-mediatek, linux-kernel, John Crispin The code checks if the queue should be stopped because we are below the threshold of free descriptors only to check if it should be started again. If we do end up in a state where we are at the threshold limit, it makes more sense to just stop the queue and wait for the next IRQ to trigger the TX housekeeping again. There is no rush in enqueuing the next packet, it needs to wait for all the others in the queue to be dispatched first anyway. Signed-off-by: John Crispin <john@phrozen.org> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 2d3c464..4d0b179 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -780,12 +780,9 @@ static int mtk_start_xmit(struct sk_buff *skb, struct net_device *dev) if (mtk_tx_map(skb, dev, tx_num, ring, gso) < 0) goto drop; - if (unlikely(atomic_read(&ring->free_count) <= ring->thresh)) { + if (unlikely(atomic_read(&ring->free_count) <= ring->thresh)) mtk_stop_queue(eth); - if (unlikely(atomic_read(&ring->free_count) > - ring->thresh)) - mtk_wake_queue(eth); - } + spin_unlock_irqrestore(ð->page_lock, flags); return NETDEV_TX_OK; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes 2016-05-05 9:26 [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes John Crispin ` (5 preceding siblings ...) 2016-05-05 9:26 ` [PATCH 12/12] net-next: mediatek: remove superfluous queue wake up call John Crispin @ 2016-05-05 12:25 ` Andrew Lunn 6 siblings, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2016-05-05 12:25 UTC (permalink / raw) To: John Crispin; +Cc: David S. Miller, nbd, netdev, linux-mediatek, linux-kernel On Thu, May 05, 2016 at 11:26:13AM +0200, John Crispin wrote: > This series is a collection of fixes for bugs that we stumble across while > doing more performance testing and code level review. Hi John I just noticed the code uses kfree(mii_bus), rather than mdiobus_free(mii_bus). Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-05-05 12:25 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-05 9:26 [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes John Crispin
2016-05-05 9:26 ` [PATCH 01/12] net-next: mediatek: fix DQL support John Crispin
[not found] ` <1462440385-51939-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>
2016-05-05 9:26 ` [PATCH 02/12] net-next: mediatek: add missing return code check John Crispin
2016-05-05 9:26 ` [PATCH 03/12] net-next: mediatek: fix missing free of scratch memory John Crispin
2016-05-05 9:26 ` [PATCH 04/12] net-next: mediatek: invalid buffer lookup in mtk_tx_map() John Crispin
2016-05-05 9:26 ` [PATCH 05/12] net-next: mediatek: dropped rx packets are not being counted properly John Crispin
2016-05-05 9:26 ` [PATCH 06/12] net-next: mediatek: add next data pointer coherency protection John Crispin
2016-05-05 9:26 ` [PATCH 07/12] net-next: mediatek: disable all interrupts during probe John Crispin
2016-05-05 9:26 ` [PATCH 08/12] net-next: mediatek: fix threshold value John Crispin
2016-05-05 9:26 ` [PATCH 09/12] net-next: mediatek: increase watchdog_timeo John Crispin
2016-05-05 9:26 ` [PATCH 10/12] net-next: mediatek: fix off by one in the TX ring allocation John Crispin
2016-05-05 9:26 ` [PATCH 11/12] net-next: mediatek: only wake the queue if it is stopped John Crispin
2016-05-05 9:26 ` [PATCH 12/12] net-next: mediatek: remove superfluous queue wake up call John Crispin
2016-05-05 12:25 ` [PATCH 00/12] net-next: mediatek: bug fixes and tx stall fixes Andrew Lunn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).