Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 8 Apr 2014 07:00:44 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r264252 - head/sys/dev/ath
Message-ID:  <201404080700.s3870iMA066612@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Tue Apr  8 07:00:43 2014
New Revision: 264252
URL: http://svnweb.freebsd.org/changeset/base/264252

Log:
  Don't resume a TID on each filtered frame completion - only do it if
  we did suspend it.
  
  The whole suspend/resume TID queue thing is supposed to be a matched
  reference count - a subsystem (eg addba negotiation, BAR transmission,
  filtered frames, etc) is supposed to call pause() once and then resume()
  once.
  
  ath_tx_tid_filt_comp_complete() is called upon the completion of any
  filtered frame, regardless of whether the driver had aleady seen
  a filtered frame and called pause().
  
  So only call resume() if tid->isfiltered = 1, which indicates that
  we had called pause() once.
  
  This fixes a seemingly whacked and different problem - traffic hangs.
  
  What was actually going on:
  
  * There'd be some marginal link with crappy behaviour, causing filtered
    frames and BAR TXing to occur;
  * A BAR TX would occur, setting the new BAW (block-ack window) to seqno n;
  * .. and pause() would be called, blocking further transmission;
  * A filtered frame completion would occur from the hardware, but with
    tid->isfiltered = 0 which indiciates we haven't actually marked
    the queue yet as filtered;
  * ath_tx_tid_filt_comp_complete() would call resume(), continuing
    transmission;
  * Some frames would be queued to the hardware, since the TID is now no
    longer paused;
  * .. and if some make it out and ACked successfully, the new BAW
    may be seqno n+1 or more;
  * .. then the BAR TX completes and sets the new seqno back to n.
  
  At this point the BAW tracking would be loopy because the BAW
  start was modified but the BAW ring buffer wasn't updated in lock
  step.
  
  Tested:
  
  * Routerstation Pro + AR9220 AP

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	Tue Apr  8 04:05:04 2014	(r264251)
+++ head/sys/dev/ath/if_ath_tx.c	Tue Apr  8 07:00:43 2014	(r264252)
@@ -3357,6 +3357,7 @@ static void
 ath_tx_tid_filt_comp_complete(struct ath_softc *sc, struct ath_tid *tid)
 {
 	struct ath_buf *bf;
+	int do_resume = 0;
 
 	ATH_TX_LOCK_ASSERT(sc);
 
@@ -3365,7 +3366,11 @@ ath_tx_tid_filt_comp_complete(struct ath
 
 	DPRINTF(sc, ATH_DEBUG_SW_TX_FILT, "%s: hwq=0, transition back\n",
 	    __func__);
-	tid->isfiltered = 0;
+	if (tid->isfiltered == 1) {
+		tid->isfiltered = 0;
+		do_resume = 1;
+	}
+
 	/* XXX ath_tx_tid_resume() also calls ath_tx_set_clrdmask()! */
 	ath_tx_set_clrdmask(sc, tid->an);
 
@@ -3375,7 +3380,8 @@ ath_tx_tid_filt_comp_complete(struct ath
 		ATH_TID_INSERT_HEAD(tid, bf, bf_list);
 	}
 
-	ath_tx_tid_resume(sc, tid);
+	if (do_resume)
+		ath_tx_tid_resume(sc, tid);
 }
 
 /*



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201404080700.s3870iMA066612>