svn commit: r264708 - head/sys/dev/ath

[Adrian Chadd]

Author: adrian
Date: Mon Apr 21 01:02:49 2014
New Revision: 264708
URL: http://svnweb.freebsd.org/changeset/base/264708

Log:
  Fix a cleanup hang if cleanup gets called _during_ an active cleanup.
  
  During power save testing I noticed that the cleanup code is being
  called during a RUN->RUN state transition.  It's because the net80211
  stack is treating that (for reasons I don't quitey know yet) as a
  reassociation and this calls the node cleanup code.  The reason it's
  seeing a RUN->RUN transition is because during active power save
  stuff it's possible that the RUN->SLEEP and SLEEP->RUN transitions
  happen so quickly that the deferred net80211 vap state code
  "loses" a transition, namely the intermediary SLEEP transition.
  
  So, this was causing the node reassociation code to sometimes be called
  twice in quick succession and this would result in ath_tx_tid_cleanup()
  to be called again.  The code calling it would always call pause, and
  then only call resume if the TID didn't have "cleanup_inprogress" set.
  Unfortunately it didn't check if it was already set on entry, so it
  would pause but not call resume.  Thus, paused would be called more
  than once (once before each entry into ath-tx_tid_cleanup()) but resume
  would only be called once when the cleanup state was finished.
  
  This doesn't entirely fix all of the issues seen in the cleanup path
  but it's a necessary first step.
  
  Since this is a stability fix, it should be merged to stable/10 at some
  point.
  
  Tested:
  
  * AR5416, STA mode
  
  MFC after:	7 days

Modified:
  head/sys/dev/ath/if_ath_tx.c

Modified: head/sys/dev/ath/if_ath_tx.c
==============================================================================
--- head/sys/dev/ath/if_ath_tx.c	Sun Apr 20 23:01:56 2014	(r264707)
+++ head/sys/dev/ath/if_ath_tx.c	Mon Apr 21 01:02:49 2014	(r264708)
@@ -5789,12 +5789,26 @@ ath_addba_stop(struct ieee80211_node *ni
 	 */
 	TAILQ_INIT(&bf_cq);
 	ATH_TX_LOCK(sc);
-	ath_tx_tid_cleanup(sc, an, tid, &bf_cq);
+
 	/*
-	 * Unpause the TID if no cleanup is required.
+	 * In case there's a followup call to this, only call it
+	 * if we don't have a cleanup in progress.
+	 *
+	 * Since we've paused the queue above, we need to make
+	 * sure we unpause if there's already a cleanup in
+	 * progress - it means something else is also doing
+	 * this stuff, so we don't need to also keep it paused.
 	 */
-	if (! atid->cleanup_inprogress)
+	if (atid->cleanup_inprogress) {
 		ath_tx_tid_resume(sc, atid);
+	} else {
+		ath_tx_tid_cleanup(sc, an, tid, &bf_cq);
+		/*
+		 * Unpause the TID if no cleanup is required.
+		 */
+		if (! atid->cleanup_inprogress)
+			ath_tx_tid_resume(sc, atid);
+	}
 	ATH_TX_UNLOCK(sc);
 
 	/* Handle completing frames and fail them */
@@ -5828,19 +5842,25 @@ ath_tx_node_reassoc(struct ath_softc *sc
 		tid = &an->an_tid[i];
 		if (tid->hwq_depth == 0)
 			continue;
-		ath_tx_tid_pause(sc, tid);
 		DPRINTF(sc, ATH_DEBUG_NODE,
 		    "%s: %6D: TID %d: cleaning up TID\n",
 		    __func__,
 		    an->an_node.ni_macaddr,
 		    ":",
 		    i);
-		ath_tx_tid_cleanup(sc, an, i, &bf_cq);
 		/*
-		 * Unpause the TID if no cleanup is required.
+	 * In case there's a followup call to this, only call it
+		 * if we don't have a cleanup in progress.
 		 */
-		if (! tid->cleanup_inprogress)
-			ath_tx_tid_resume(sc, tid);
+		if (! tid->cleanup_inprogress) {
+			ath_tx_tid_pause(sc, tid);
+			ath_tx_tid_cleanup(sc, an, i, &bf_cq);
+			/*
+			 * Unpause the TID if no cleanup is required.
+			 */
+			if (! tid->cleanup_inprogress)
+				ath_tx_tid_resume(sc, tid);
+		}
 	}
 	ATH_TX_UNLOCK(sc);