Since a05988bb... "ath5k: fix race condition in tx desc
processing" we have been keeping the last TX descriptor
around to avoid potential stalls by the hardware's DMA
unit and nebulous crashes.
Looking more closely, it seems the real problem was
handling of txq->link: the final descriptor could get
assigned to a different queue and then a subsequent packet
would get queued twice on two different queues, leading
to multiple or garbage transmissions and stalls.
In the case that hardware does reach the end of the
descriptor chain, we now always rewrite TXDP before
re-enabling DMA, so that device does not read the link
from the last descriptor.
Also set ds_link to null right after we accept a buffer
to ensure that, if there really is a race between interrupt
status flag and the reading of ds_link, we'll see it more
frequently in the form of queue stalls.
This also fixes accounting so that txq_len reflects reality,
and makes skb null check in the tx path unnecessary
(converted to WARN_ON).
Signed-off-by: Bob Copeland <m...@bobcopeland.com>
---
drivers/net/wireless/ath/ath5k/base.c | 62 ++++++++++++++++-----------------
1 files changed, 30 insertions(+), 32 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/base.c
b/drivers/net/wireless/ath/ath5k/base.c
index e9ea38d..513d925 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1647,42 +1647,40 @@ ath5k_tx_processq(struct ath5k_hw *ah, struct ath5k_txq
*txq)
txq->txq_poll_mark = false;
- /* skb might already have been processed last time. */
- if (bf->skb != NULL) {
- ds = bf->desc;
-
- ret = ah->ah_proc_tx_desc(ah, ds, &ts);
- if (unlikely(ret == -EINPROGRESS))
- break;
- else if (unlikely(ret)) {
- ATH5K_ERR(ah,
- "error %d while processing "
- "queue %u\n", ret, txq->qnum);
- break;
- }
+ if (WARN_ON(!bf->skb))
+ continue;
- skb = bf->skb;
- bf->skb = NULL;
+ ds = bf->desc;
- dma_unmap_single(ah->dev, bf->skbaddr, skb->len,
- DMA_TO_DEVICE);
- ath5k_tx_frame_completed(ah, skb, txq, &ts);
+ ret = ah->ah_proc_tx_desc(ah, ds, &ts);
+ if (unlikely(ret == -EINPROGRESS))
+ break;
+ else if (unlikely(ret)) {
+ ATH5K_ERR(ah,
+ "error %d while processing "
+ "queue %u\n", ret, txq->qnum);
+ break;
}
+ /* XXX next assignment is just to find races with HW */
+ ds->ds_link = 0;
- /*
- * It's possible that the hardware can say the buffer is
- * completed when it hasn't yet loaded the ds_link from
- * host memory and moved on.
- * Always keep the last descriptor to avoid HW races...
- */
- if (ath5k_hw_get_txdp(ah, txq->qnum) != bf->daddr) {
- spin_lock(&ah->txbuflock);
- list_move_tail(&bf->list, &ah->txbuf);
- ah->txbuf_len++;
- txq->txq_len--;
- spin_unlock(&ah->txbuflock);
- }
+ skb = bf->skb;
+ bf->skb = NULL;
+
+ dma_unmap_single(ah->dev, bf->skbaddr, skb->len,
+ DMA_TO_DEVICE);
+ ath5k_tx_frame_completed(ah, skb, txq, &ts);
+
+ spin_lock(&ah->txbuflock);
+ list_move_tail(&bf->list, &ah->txbuf);
+ ah->txbuf_len++;
+ txq->txq_len--;
+ spin_unlock(&ah->txbuflock);
}
+ /* if we processed every buffer, reprogram txdp next time */
+ if (list_empty(&txq->q))
+ txq->link = NULL;
+
spin_unlock(&txq->lock);
if (txq->txq_len < ATH5K_TXQ_LEN_LOW && txq->qnum < 4)
ieee80211_wake_queue(ah->hw, txq->qnum);
@@ -2308,7 +2306,7 @@ ath5k_tx_complete_poll_work(struct work_struct *work)
if (ah->txqs[i].setup) {
txq = &ah->txqs[i];
spin_lock_bh(&txq->lock);
- if (txq->txq_len > 1) {
+ if (txq->txq_len) {
if (txq->txq_poll_mark) {
ATH5K_DBG(ah, ATH5K_DEBUG_XMIT,
"TX queue stuck %d\n",
--
1.7.6
_______________________________________________
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel
