From: John Crispin <blogic@openwrt.org>
To: "David S. Miller" <davem@davemloft.net>
Cc: "Felix Fietkau" <nbd@openwrt.org>,
"Matthias Brugger" <matthias.bgg@gmail.com>,
"Sean Wang (王志亘)" <sean.wang@mediatek.com>,
netdev@vger.kernel.org, linux-mediatek@lists.infradead.org,
linux-kernel@vger.kernel.org, "John Crispin" <blogic@openwrt.org>
Subject: [PATCH 7/9] net: mediatek: fix TX locking
Date: Thu, 7 Apr 2016 19:57:54 +0200 [thread overview]
Message-ID: <1460051876-53135-7-git-send-email-blogic@openwrt.org> (raw)
In-Reply-To: <1460051876-53135-1-git-send-email-blogic@openwrt.org>
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>
---
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 60b66ab..8434355 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(ð->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(ð->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(ð->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 = ð->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 ont he same ring so we need to lock
+ * the ring access
+ */
+ spin_lock_irqsave(ð->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(ð->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(ð->page_lock, flags);
return NETDEV_TX_OK;
drop:
+ spin_unlock_irqrestore(ð->page_lock, flags);
stats->tx_dropped++;
dev_kfree_skb(skb);
return NETDEV_TX_OK;
--
1.7.10.4
next prev parent reply other threads:[~2016-04-07 17:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-07 17:57 [PATCH 1/9] net: mediatek: update the IRQ part of the binding document John Crispin
[not found] ` <1460051876-53135-1-git-send-email-blogic-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
2016-04-07 17:57 ` [PATCH 2/9] net: mediatek: watchdog_timeo was not set John Crispin
2016-04-07 17:57 ` [PATCH 3/9] net: mediatek: mtk_cal_txd_req() returns bad value John Crispin
2016-04-07 17:57 ` [PATCH 4/9] net: mediatek: remove superfluous reset call John Crispin
2016-04-07 17:57 ` [PATCH 5/9] net: mediatek: fix stop and wakeup of queue John Crispin
2016-04-07 17:57 ` [PATCH 8/9] net: mediatek: move the pending_work struct to the device generic struct John Crispin
2016-04-07 17:57 ` [PATCH 9/9] net: mediatek: do not set the QID field in the TX DMA descriptors John Crispin
2016-04-07 18:50 ` [PATCH 1/9] net: mediatek: update the IRQ part of the binding document David Miller
2016-04-07 17:57 ` [PATCH 6/9] net: mediatek: fix mtk_pending_work John Crispin
2016-04-07 17:57 ` John Crispin [this message]
2016-04-11 15:24 ` [PATCH 1/9] net: mediatek: update the IRQ part of the binding document Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1460051876-53135-7-git-send-email-blogic@openwrt.org \
--to=blogic@openwrt.org \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=nbd@openwrt.org \
--cc=netdev@vger.kernel.org \
--cc=sean.wang@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).