linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bruno Randolf <br1@einfach.org>
To: linville@tuxdriver.com
Cc: ath5k-devel@lists.ath5k.org, linux-wireless@vger.kernel.org
Subject: [PATCH 7/9] ath5k: Keep last descriptor in queue
Date: Thu, 16 Sep 2010 18:51:49 +0900	[thread overview]
Message-ID: <20100916095149.17637.77736.stgit@tt-desk> (raw)
In-Reply-To: <20100916095112.17637.17207.stgit@tt-desk>

If we return a TX descriptor to the pool of available descriptors, while a
queues TXDP still points to it we could potentially run into all sorts of
troube.

It has been suggested that there is hardware which can set the descriptors
done bit before it reads ds_link and moves on to the next descriptor. While the
documentation says this is not true for newer chipsets (the descriptor contents
are copied to some internal memory), we don't know about older hardware.

To be safe, we always keep the last descriptor in the queue, and avoid dangling
TXDP pointers. Unfortunately this does not fully resolve the problem - queues
still get stuck!

This is similar to what ath9k does.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 drivers/net/wireless/ath/ath5k/base.c |   64 +++++++++++++++++----------------
 1 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index c4f0786..9b67cee 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1590,44 +1590,44 @@ ath5k_tx_processq(struct ath5k_softc *sc, struct ath5k_txq *txq)
 
 	spin_lock(&txq->lock);
 	list_for_each_entry_safe(bf, bf0, &txq->q, list) {
-		ds = bf->desc;
+
+		txq->txq_poll_mark = false;
+
+		/* skb might already have been processed last time. */
+		if (bf->skb != NULL) {
+			ds = bf->desc;
+
+			ret = sc->ah->ah_proc_tx_desc(sc->ah, ds, &ts);
+			if (unlikely(ret == -EINPROGRESS))
+				break;
+			else if (unlikely(ret)) {
+				ATH5K_ERR(sc,
+					"error %d while processing "
+					"queue %u\n", ret, txq->qnum);
+				break;
+			}
+
+			skb = bf->skb;
+			bf->skb = NULL;
+			pci_unmap_single(sc->pdev, bf->skbaddr, skb->len,
+					PCI_DMA_TODEVICE);
+			ath5k_tx_frame_completed(sc, skb, &ts);
+		}
 
 		/*
 		 * 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.  If there are more TX
-		 * descriptors in the queue, wait for TXDP to change
-		 * before processing this one.
+		 * host memory and moved on.
+		 * Always keep the last descriptor to avoid HW races...
 		 */
-		if (ath5k_hw_get_txdp(sc->ah, txq->qnum) == bf->daddr &&
-		    !list_is_last(&bf->list, &txq->q))
-			break;
-		ret = sc->ah->ah_proc_tx_desc(sc->ah, ds, &ts);
-		if (unlikely(ret == -EINPROGRESS))
-			break;
-		else if (unlikely(ret)) {
-			ATH5K_ERR(sc, "error %d while processing queue %u\n",
-				ret, txq->qnum);
-			break;
+		if (ath5k_hw_get_txdp(sc->ah, txq->qnum) != bf->daddr) {
+			spin_lock(&sc->txbuflock);
+			list_move_tail(&bf->list, &sc->txbuf);
+			sc->txbuf_len++;
+			txq->txq_len--;
+			spin_unlock(&sc->txbuflock);
 		}
-
-		skb = bf->skb;
-		bf->skb = NULL;
-		pci_unmap_single(sc->pdev, bf->skbaddr, skb->len,
-				PCI_DMA_TODEVICE);
-
-		ath5k_tx_frame_completed(sc, skb, &ts);
-
-		spin_lock(&sc->txbuflock);
-		list_move_tail(&bf->list, &sc->txbuf);
-		sc->txbuf_len++;
-		txq->txq_len--;
-		spin_unlock(&sc->txbuflock);
-
-		txq->txq_poll_mark = false;
 	}
-	if (likely(list_empty(&txq->q)))
-		txq->link = NULL;
 	spin_unlock(&txq->lock);
 	if (txq->txq_len < ATH5K_TXQ_LEN_LOW)
 		ieee80211_wake_queue(sc->hw, txq->qnum);
@@ -2192,7 +2192,7 @@ ath5k_tx_complete_poll_work(struct work_struct *work)
 		if (sc->txqs[i].setup) {
 			txq = &sc->txqs[i];
 			spin_lock_bh(&txq->lock);
-			if (txq->txq_len > 0) {
+			if (txq->txq_len > 1) {
 				if (txq->txq_poll_mark) {
 					ATH5K_DBG(sc, ATH5K_DEBUG_XMIT,
 						  "TX queue stuck %d\n",


  parent reply	other threads:[~2010-09-16  9:51 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-16  9:51 [PATCH 0/9] ath5: Add multiple queues / QoS support Bruno Randolf
2010-09-16  9:51 ` [PATCH 1/9] ath5k: Use four hardware queues Bruno Randolf
2010-09-16  9:51 ` [PATCH 2/9] ath5k: Fix queue debug file Bruno Randolf
2010-09-16  9:51 ` [PATCH 3/9] ath5k: Fix TX queues stopping Bruno Randolf
2010-09-16  9:51 ` [PATCH 4/9] ath5k: Move tx frame completion into separate function Bruno Randolf
2010-09-16  9:51 ` [PATCH 5/9] ath5k: Add watchdog for stuck TX queues Bruno Randolf
2010-09-16  9:51 ` [PATCH 6/9] ath5k: Count how many times a queue got stuck Bruno Randolf
2010-09-16  9:51 ` Bruno Randolf [this message]
2010-09-16  9:51 ` [PATCH 8/9] ath5k: Simplify cw_min/max and AIFS configuration Bruno Randolf
2010-09-16  9:52 ` [PATCH 9/9] ath5k: Add tx queue configuration function Bruno Randolf
2010-09-16 19:38 ` [PATCH 0/9] ath5: Add multiple queues / QoS support John W. Linville

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=20100916095149.17637.77736.stgit@tt-desk \
    --to=br1@einfach.org \
    --cc=ath5k-devel@lists.ath5k.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.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).