Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Aug 2011 03:41:48 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   svn commit: r224874 - user/adrian/if_ath_tx/sys/dev/ath
Message-ID:  <201108150341.p7F3fm1j091861@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Mon Aug 15 03:41:48 2011
New Revision: 224874
URL: http://svn.freebsd.org/changeset/base/224874

Log:
  Do some relatively major changes to my software TX scheduling code.
  
  In the first cut, and inspired by some other code I have here which
  implements this, I decided to hide the per-TID locking behind
  the hardware TXQ lock. That worked, as the mapping for TID->TXQ
  is constant and predictable, but it meant the hardware lock is
  held a lot longer than it needs to be. This has caused all kinds
  of problems.
  
  A 'better' way (for values of 'better') would be to implement
  fine-grained locking of the per-node and per-TID state. Then those
  locks can be held as long (short) as they need to be.
  
  But for now, I'm going with another temporary solution designed
  to give me more breathing room whilst I port over the code.
  I've separated out the TX code into a sort of split setup,
  with the upper half being net80211/network code, and the lower
  half being the TX scheduling and completion.
  
  * Create a "TX sched" task, which will run the TX scheduling
    code as a task
  * Remove a lot of the hardware TXQ locking and asserting
  * Re-introduce the per-TID software TXQ lock
  * Since the entry pathways into this code now aren't locked behind
    the hardware TXQ locks, re-lock the TID software queue access
  * Re-introduce the short-held hardware TXQ locks, so top and bottom
    level fiddling of the hardware TXQs don't trample on each other.
  
  Now, the "top" level code (ie, anything which wishes to queue packets)
  will enter via ath_start / ath_raw_xmit / ath_tx_start / ath_tx_raw_start
  and will either queue directly to the hardware (and that's protected
  by the hardware TXQ locks) or be queued to the software queue via a call
  to ath_tx_swq(). ath_tx_swq() simply queues a packet to the software
  queue (protected by a software TXQ lock) and then the actual TX packet
  scheduling code is invoked in a task call to ath_tx_sched_proc().
  
  ath_tx_sched_proc() handles scheduling; ath_tx_proc() handles TX
  completion and further scheduling; since neither of them run
  simultaneously I can avoid a lot of the complicated locking.
  
  This likely won't be the final solution (as I may end up introducing
  fine-grained locks anyway) but it does push (almost) all of the
  per-TID state and general aggregation state handling into the ath
  task, rather than trying to handle concurrent accesses from TX
  processes, RX/TX tasks and interrupt tasks.
  
  Note: net80211 node cleanup and node flush may still need some further
  locking. I'll look into that shortly. (as node_flush can occur during
  a scan, for example, and I haven't checked to see whether that runs
  within or separate to the ath taskqueue.)

Modified:
  user/adrian/if_ath_tx/sys/dev/ath/if_ath.c
  user/adrian/if_ath_tx/sys/dev/ath/if_ath_misc.h
  user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c
  user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath.c	Mon Aug 15 00:38:14 2011	(r224873)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath.c	Mon Aug 15 03:41:48 2011	(r224874)
@@ -175,6 +175,7 @@ static void	ath_tx_cleanup(struct ath_so
 static void	ath_tx_proc_q0(void *, int);
 static void	ath_tx_proc_q0123(void *, int);
 static void	ath_tx_proc(void *, int);
+static void	ath_tx_sched_proc(void *, int);
 static int	ath_chan_set(struct ath_softc *, struct ieee80211_channel *);
 static void	ath_draintxq(struct ath_softc *);
 static void	ath_stoprecv(struct ath_softc *);
@@ -398,6 +399,7 @@ ath_attach(u_int16_t devid, struct ath_s
 	TASK_INIT(&sc->sc_rxtask, 0, ath_rx_proc, sc);
 	TASK_INIT(&sc->sc_bmisstask, 0, ath_bmiss_proc, sc);
 	TASK_INIT(&sc->sc_bstucktask,0, ath_bstuck_proc, sc);
+	TASK_INIT(&sc->sc_txschedtask, 0, ath_tx_sched_proc, sc);
 
 	/*
 	 * Allocate hardware transmit queues: one queue for
@@ -4140,6 +4142,8 @@ ath_tx_default_comp(struct ath_softc *sc
 
 /*
  * Process completed xmit descriptors from the specified queue.
+ * Kick the packet scheduler if needed. This can occur from this
+ * particular task.
  */
 static int
 ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
@@ -4153,7 +4157,6 @@ ath_tx_processq(struct ath_softc *sc, st
 	int nacked;
 	HAL_STATUS status;
 
-	ATH_TXQ_LOCK(txq);
 	DPRINTF(sc, ATH_DEBUG_TX_PROC, "%s: tx queue %u head %p link %p\n",
 		__func__, txq->axq_qnum,
 		(caddr_t)(uintptr_t) ath_hal_gettxbuf(sc->sc_ah, txq->axq_qnum),
@@ -4161,8 +4164,10 @@ ath_tx_processq(struct ath_softc *sc, st
 	nacked = 0;
 	for (;;) {
 		txq->axq_intrcnt = 0;	/* reset periodic desc intr count */
+		ATH_TXQ_LOCK(txq);
 		bf = STAILQ_FIRST(&txq->axq_q);
 		if (bf == NULL) {
+			ATH_TXQ_UNLOCK(txq);
 			break;
 		}
 		ds0 = &bf->bf_desc[0];
@@ -4175,6 +4180,7 @@ ath_tx_processq(struct ath_softc *sc, st
 			    status == HAL_OK);
 #endif
 		if (status == HAL_EINPROGRESS) {
+			ATH_TXQ_UNLOCK(txq);
 			break;
 		}
 		ATH_TXQ_REMOVE_HEAD(txq, bf_list);
@@ -4191,6 +4197,7 @@ ath_tx_processq(struct ath_softc *sc, st
 		if (txq->axq_depth == 0)
 #endif
 			txq->axq_link = NULL;
+		ATH_TXQ_UNLOCK(txq);
 
 		ni = bf->bf_node;
 		/*
@@ -4238,7 +4245,6 @@ ath_tx_processq(struct ath_softc *sc, st
 
 	/* Kick the TXQ scheduler */
 	ath_txq_sched(sc, txq);
-	ATH_TXQ_UNLOCK(txq);
 
 	return nacked;
 }
@@ -4341,6 +4347,39 @@ ath_tx_proc(void *arg, int npending)
 }
 
 /*
+ * TX scheduling
+ *
+ * This calls the per-TXQ TX queue packet scheduling code.
+ *
+ * Note: there's no need to handle the mcastq; it doesn't have
+ * a software TID queue attached (as it's a software queue in
+ * itself.)
+ */
+static void
+ath_tx_sched_proc(void *arg, int npending)
+{
+	struct ath_softc *sc = arg;
+	struct ath_txq *txq;
+	int i;
+
+	for (i = 0; i < HAL_NUM_TX_QUEUES; i++) {
+		txq = &sc->sc_txq[i];
+		if (ATH_TXQ_SETUP(sc, i)) {
+			ath_txq_sched(sc, txq);
+		}
+	}
+}
+
+/*
+ * Schedule a TXQ scheduling task to occur.
+ */
+void
+ath_tx_sched_proc_sched(struct ath_softc *sc)
+{
+	taskqueue_enqueue(sc->sc_tq, &sc->sc_txschedtask);
+}
+
+/*
  * This is currently used by ath_tx_draintxq() and
  * ath_tx_tid_free_pkts().
  *
@@ -4389,23 +4428,17 @@ ath_tx_draintxq(struct ath_softc *sc, st
 	if (bf != NULL)
 		bf->bf_flags &= ~ATH_BUF_BUSY;
 	ATH_TXBUF_UNLOCK(sc);
-	/*
-	 * The TXQ lock is held here for the entire trip through the
-	 * list (rather than just being held whilst acquiring frames
-	 * off of the list) because of the (current) assumption that
-	 * the per-TID software queue stuff is locked by the hardware
-	 * queue it maps to, thus ensuring proper ordering of things.
-	 *
-	 * The chance for LOR's however, is now that much bigger. Sigh.
-	 */
-	ATH_TXQ_LOCK(txq);
+
 	for (ix = 0;; ix++) {
+		ATH_TXQ_LOCK(txq);
 		bf = STAILQ_FIRST(&txq->axq_q);
 		if (bf == NULL) {
+			ATH_TXQ_UNLOCK(txq);
 			txq->axq_link = NULL;
 			break;
 		}
 		ATH_TXQ_REMOVE_HEAD(txq, bf_list);
+		ATH_TXQ_UNLOCK(txq);
 #ifdef ATH_DEBUG
 		if (sc->sc_debug & ATH_DEBUG_RESET) {
 			struct ieee80211com *ic = sc->sc_ifp->if_l2com;
@@ -4428,7 +4461,6 @@ ath_tx_draintxq(struct ath_softc *sc, st
 		else
 			ath_tx_default_comp(sc, bf, 1);
 	}
-	ATH_TXQ_UNLOCK(txq);
 }
 
 static void

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_misc.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath_misc.h	Mon Aug 15 00:38:14 2011	(r224873)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_misc.h	Mon Aug 15 03:41:48 2011	(r224874)
@@ -59,5 +59,6 @@ extern void ath_tx_default_comp(struct a
 	    int fail);
 extern void ath_tx_freebuf(struct ath_softc *sc, struct ath_buf *bf,
     int status);
+extern void ath_tx_sched_proc_sched(struct ath_softc *sc);
 
 #endif

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Mon Aug 15 00:38:14 2011	(r224873)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Mon Aug 15 03:41:48 2011	(r224874)
@@ -470,6 +470,7 @@ ath_tx_handoff_hw(struct ath_softc *sc, 
 
 /*
  * Hand off a packet to the hardware (or mcast queue.)
+ *
  * The relevant hardware txq should be locked.
  */
 static void
@@ -995,6 +996,11 @@ ath_tx_normal_setup(struct ath_softc *sc
 
 /*
  * Direct-dispatch the current frame to the hardware.
+ *
+ * This can be called by the net80211 code.
+ *
+ * XXX what about locking? Or, push the seqno assign into the
+ * XXX aggregate scheduler so its serialised?
  */
 int
 ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
@@ -1130,12 +1136,11 @@ ath_tx_start(struct ath_softc *sc, struc
 		ath_tx_handoff(sc, txq, bf);
 		ATH_TXQ_UNLOCK(txq);
 	} else {
-		ATH_TXQ_LOCK(txq);
 		/* add to software queue */
 		ath_tx_swq(sc, ni, txq, bf);
-		/* Kick txq */
-		ath_txq_sched(sc, txq);
-		ATH_TXQ_UNLOCK(txq);
+
+		/* Schedule a TX scheduler task call to occur */
+		ath_tx_sched_proc_sched(sc);
 	}
 #else
 	/*
@@ -1143,10 +1148,7 @@ ath_tx_start(struct ath_softc *sc, struc
 	 * direct-dispatch to the hardware.
 	 */
 	ATH_TXQ_LOCK(txq);
-	if (ismcast)
-		ath_tx_handoff_mcast(sc, txq, bf);
-	else
-		ath_tx_handoff_hw(sc, txq, bf);
+	ath_tx_handoff(sc, txq, bf);
 	ATH_TXQ_UNLOCK(txq);
 #endif
 
@@ -1371,21 +1373,24 @@ ath_tx_raw_start(struct ath_softc *sc, s
 	 * frames to that node are.
 	 */
 
-	ATH_TXQ_LOCK(sc->sc_ac2q[pri]);
-	if (do_override)
+	if (do_override) {
+		ATH_TXQ_LOCK(sc->sc_ac2q[pri]);
 		ath_tx_handoff(sc, sc->sc_ac2q[pri], bf);
+		ATH_TXQ_UNLOCK(sc->sc_ac2q[pri]);
+	}
 	else {
 		/* Queue to software queue */
 		ath_tx_swq(sc, ni, sc->sc_ac2q[pri], bf);
-
-		/* Kick txq */
-		ath_txq_sched(sc, sc->sc_ac2q[pri]);
 	}
-	ATH_TXQ_UNLOCK(sc->sc_ac2q[pri]);
 
 	return 0;
 }
 
+/*
+ * Send a raw frame.
+ *
+ * This can be called by net80211.
+ */
 int
 ath_raw_xmit(struct ieee80211_node *ni, struct mbuf *m,
 	const struct ieee80211_bpf_params *params)
@@ -1438,6 +1443,14 @@ ath_raw_xmit(struct ieee80211_node *ni, 
 	ifp->if_opackets++;
 	sc->sc_stats.ast_tx_raw++;
 
+	/*
+	 * This kicks off a TX packet scheduler task,
+	 * pushing packet scheduling out of this thread
+	 * and into a separate context. This will (hopefully)
+	 * simplify locking/contention in the long run.
+	 */
+	ath_tx_sched_proc_sched(sc);
+
 	return 0;
 bad2:
 	ATH_TXBUF_LOCK(sc);
@@ -1557,8 +1570,6 @@ ath_tx_addto_baw(struct ath_softc *sc, s
 	if (bf->bf_state.bfs_isretried)
 		return;
 
-	ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]);
-
 	tap = ath_tx_get_tx_tid(an, tid->tid);
 	DPRINTF(sc, ATH_DEBUG_SW_TX_BAW, "%s: tid=%d, seqno %d; window %d:%d\n",
 		    __func__, tid->tid, SEQNO(bf->bf_state.bfs_seqno),
@@ -1604,8 +1615,6 @@ ath_tx_update_baw(struct ath_softc *sc, 
 	int index, cindex;
 	struct ieee80211_tx_ampdu *tap;
 
-	ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]);
-
 	tap = ath_tx_get_tx_tid(an, tid->tid);
 	DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: tid=%d, baw=%d:%d, seqno=%d\n",
 	    __func__, tid->tid, tap->txa_start, tap->txa_wnd, seqno);
@@ -1631,7 +1640,7 @@ ath_tx_update_baw(struct ath_softc *sc, 
  * This is done to make it easy for the software scheduler to
  * find which nodes have data to send.
  *
- * The relevant hw txq lock should be held.
+ * The TXQ lock must be held.
  */
 static void
 ath_tx_tid_sched(struct ath_softc *sc, struct ath_node *an, int tid)
@@ -1653,7 +1662,7 @@ ath_tx_tid_sched(struct ath_softc *sc, s
  * Mark the current node as no longer needing to be polled for
  * TX packets.
  *
- * The relevant hw txq lock should be held.
+ * The TXQ lock must NOT be held, it'll be grabbed as needed.
  */
 static void
 ath_tx_tid_unsched(struct ath_softc *sc, struct ath_node *an, int tid)
@@ -1661,13 +1670,13 @@ ath_tx_tid_unsched(struct ath_softc *sc,
 	struct ath_tid *atid = &an->an_tid[tid];
 	struct ath_txq *txq = sc->sc_ac2q[atid->ac];
 
-	ATH_TXQ_LOCK_ASSERT(txq);
-
 	if (atid->sched == 0)
 		return;
 
 	atid->sched = 0;
+	ATH_TXQ_LOCK(txq);
 	STAILQ_REMOVE(&txq->axq_tidq, atid, ath_tid, axq_qelem);
+	ATH_TXQ_UNLOCK(txq);
 }
 
 /*
@@ -1738,8 +1747,6 @@ ath_tx_swq(struct ath_softc *sc, struct 
 	int pri, tid;
 	struct mbuf *m0 = bf->bf_m;
 
-	ATH_TXQ_LOCK_ASSERT(txq);
-
 	/* Fetch the TID - non-QoS frames get assigned to TID 16 */
 	wh = mtod(m0, struct ieee80211_frame *);
 	pri = ath_tx_getac(sc, m0);
@@ -1756,10 +1763,14 @@ ath_tx_swq(struct ath_softc *sc, struct 
 	bf->bf_state.bfs_dobaw = 0;
 
 	/* Queue frame to the tail of the software queue */
+	ATH_TXQ_LOCK(atid);
 	ATH_TXQ_INSERT_TAIL(atid, bf, bf_list);
+	ATH_TXQ_UNLOCK(atid);
 
 	/* Mark the given tid as having packets to dequeue */
+	ATH_TXQ_LOCK(txq);
 	ath_tx_tid_sched(sc, an, tid);
+	ATH_TXQ_UNLOCK(txq);
 }
 
 /*
@@ -1805,6 +1816,9 @@ ath_tx_tid_init(struct ath_softc *sc, st
 			atid->ac = WME_AC_BE;
 		else
 			atid->ac = TID_TO_WME_AC(i);
+		snprintf(atid->axq_name, 48, "%p %s %d",
+		    an, device_get_nameunit(sc->sc_dev), i);
+		mtx_init(&atid->axq_lock, atid->axq_name, NULL, MTX_DEF);
 	}
 }
 
@@ -1815,9 +1829,6 @@ ath_tx_tid_init(struct ath_softc *sc, st
 static void
 ath_tx_tid_pause(struct ath_softc *sc, struct ath_tid *tid)
 {
-	struct ath_txq *txq = sc->sc_ac2q[tid->ac];
-
-	ATH_TXQ_LOCK_ASSERT(txq);
 	tid->paused++;
 	DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: paused = %d\n",
 	    __func__, tid->paused);
@@ -1828,7 +1839,6 @@ ath_tx_tid_resume(struct ath_softc *sc, 
 {
 	struct ath_txq *txq = sc->sc_ac2q[tid->ac];
 
-	ATH_TXQ_LOCK_ASSERT(txq);
 	tid->paused--;
 
 	DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: unpaused = %d\n",
@@ -1838,8 +1848,11 @@ ath_tx_tid_resume(struct ath_softc *sc, 
 	if (tid->axq_depth == 0)
 		return;
 
+	ATH_TXQ_LOCK(txq);
 	ath_tx_tid_sched(sc, tid->an, tid->tid);
-	ath_txq_sched(sc, txq);
+	ATH_TXQ_UNLOCK(txq);
+
+	ath_tx_sched_proc_sched(sc);
 }
 
 /*
@@ -1876,8 +1889,10 @@ ath_tx_tid_free_pkts(struct ath_softc *s
 
 	/* Walk the queue, free frames */
 	for (;;) {
+		ATH_TXQ_LOCK(atid);
 		bf = STAILQ_FIRST(&atid->axq_q);
 		if (bf == NULL) {
+			ATH_TXQ_UNLOCK(atid);
 			break;
 		}
 
@@ -1890,6 +1905,7 @@ ath_tx_tid_free_pkts(struct ath_softc *s
 			ath_tx_update_baw(sc, an, atid,
 			    SEQNO(bf->bf_state.bfs_seqno));
 		ATH_TXQ_REMOVE_HEAD(atid, bf_list);
+		ATH_TXQ_UNLOCK(atid);
 		ath_tx_freebuf(sc, bf, -1);
 	}
 }
@@ -1951,6 +1967,7 @@ ath_tx_tid_cleanup(struct ath_softc *sc,
 		atid = &an->an_tid[tid];
 		/* Mark hw-queued packets as having no parent now */
 		ath_tx_tid_txq_unmark(sc, an, tid);
+		mtx_destroy(&atid->axq_lock);
 	}
 }
 
@@ -1987,13 +2004,10 @@ ath_tx_comp_cleanup(struct ath_softc *sc
 	struct ath_node *an = ATH_NODE(ni);
 	int tid = bf->bf_state.bfs_tid;
 	struct ath_tid *atid = &an->an_tid[tid];
-	struct ath_txq *txq = sc->sc_ac2q[atid->ac];
 
 	device_printf(sc->sc_dev, "%s: TID %d: incomp=%d\n",
 	    __func__, tid, atid->incomp);
 
-	ATH_TXQ_LOCK_ASSERT(txq);
-
 	atid->incomp--;
 	if (atid->incomp == 0) {
 		device_printf(sc->sc_dev, "%s: TID %d: cleaned up! resume!\n",
@@ -2015,26 +2029,23 @@ ath_tx_comp_cleanup(struct ath_softc *sc
  *   handle it later.
  *
  * The caller is responsible for pausing the TID.
- * The caller is responsible for locking TXQ.
  */
 static void
 ath_tx_cleanup(struct ath_softc *sc, struct ath_node *an, int tid)
 {
 	struct ath_tid *atid = &an->an_tid[tid];
-	struct ath_txq *txq = sc->sc_ac2q[atid->ac];
 	struct ieee80211_tx_ampdu *tap;
 	struct ath_buf *bf, *bf_next;
 
 	device_printf(sc->sc_dev, "%s: TID %d: called\n", __func__, tid);
 
-	ATH_TXQ_LOCK_ASSERT(txq);
-
 	/*
 	 * Update the frames in the software TX queue:
 	 *
 	 * + Discard retry frames in the queue
 	 * + Fix the completion function to be non-aggregate
 	 */
+	ATH_TXQ_LOCK(atid);
 	bf = STAILQ_FIRST(&atid->axq_q);
 	while (bf) {
 		if (bf->bf_state.bfs_isretried) {
@@ -2056,6 +2067,7 @@ ath_tx_cleanup(struct ath_softc *sc, str
 		bf->bf_comp = NULL;
 		bf = STAILQ_NEXT(bf, bf_list);
 	}
+	ATH_TXQ_UNLOCK(atid);
 
 	/* The caller is required to pause the TID */
 #if 0
@@ -2123,11 +2135,8 @@ ath_tx_aggr_retry_unaggr(struct ath_soft
 	struct ath_node *an = ATH_NODE(ni);
 	int tid = bf->bf_state.bfs_tid;
 	struct ath_tid *atid = &an->an_tid[tid];
-	struct ath_txq *txq = sc->sc_ac2q[atid->ac];
 	struct ieee80211_tx_ampdu *tap;
 
-	ATH_TXQ_LOCK_ASSERT(txq);
-
 	tap = ath_tx_get_tx_tid(an, tid);
 
 	if (bf->bf_state.bfs_retries >= SWMAX_RETRIES) {
@@ -2170,8 +2179,13 @@ ath_tx_aggr_retry_unaggr(struct ath_soft
 	 * Insert this at the head of the queue, so it's
 	 * retried before any current/subsequent frames.
 	 */
+	ATH_TXQ_LOCK(atid);
 	ATH_TXQ_INSERT_HEAD(atid, bf, bf_list);
+	ATH_TXQ_UNLOCK(atid);
+
+	ATH_TXQ_LOCK(bf->bf_state.bfs_txq);
 	ath_tx_tid_sched(sc, an, atid->tid);
+	ATH_TXQ_UNLOCK(bf->bf_state.bfs_txq);
 }
 
 /*
@@ -2248,8 +2262,10 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft
 		    __func__);
 
 	for (;;) {
+		ATH_TXQ_LOCK(atid);
                 bf = STAILQ_FIRST(&atid->axq_q);
 		if (bf == NULL) {
+			ATH_TXQ_UNLOCK(atid);
 			break;
 		}
 		DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: bf=%p: tid=%d\n",
@@ -2272,10 +2288,6 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft
 			break;
 		}
 
-		/* Don't add packets to the BAW that don't contribute to it */
-		if (bf->bf_state.bfs_dobaw)
-			ath_tx_addto_baw(sc, an, atid, bf);
-
 		/*
 		 * XXX If the seqno is out of BAW, then we should pause this TID
 		 * XXX until a completion for this TID allows the BAW to be advanced.
@@ -2283,10 +2295,16 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft
 		 * XXX for this node/TID until some TX completion has occured
 		 * XXX and progress can be made.
 		 */
+
+		/* We've committed to sending it, so remove it from the list */
 		ATH_TXQ_REMOVE_HEAD(atid, bf_list);
+		ATH_TXQ_UNLOCK(atid);
+
+		/* Don't add packets to the BAW that don't contribute to it */
+		if (bf->bf_state.bfs_dobaw)
+			ath_tx_addto_baw(sc, an, atid, bf);
 
 		txq = bf->bf_state.bfs_txq;
-		ATH_TXQ_LOCK_ASSERT(txq);
 
 		/* Sanity check! */
 		if (tid != bf->bf_state.bfs_tid) {
@@ -2301,7 +2319,9 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft
 			device_printf(sc->sc_dev, "%s: TID=16?\n", __func__);
 
 		/* Punt to hardware or software txq */
+		ATH_TXQ_LOCK(txq);
 		ath_tx_handoff(sc, txq, bf);
+		ATH_TXQ_UNLOCK(txq);
 	}
 }
 
@@ -2324,14 +2344,17 @@ ath_tx_tid_hw_queue_norm(struct ath_soft
 		    __func__, tid);
 
 	for (;;) {
-                bf = STAILQ_FIRST(&atid->axq_q);
+		ATH_TXQ_LOCK(atid);
+		bf = STAILQ_FIRST(&atid->axq_q);
 		if (bf == NULL) {
+			ATH_TXQ_UNLOCK(atid);
 			break;
 		}
 		ATH_TXQ_REMOVE_HEAD(atid, bf_list);
+		ATH_TXQ_UNLOCK(atid);
 
 		txq = bf->bf_state.bfs_txq;
-		ATH_TXQ_LOCK_ASSERT(txq);
+
 		/* Sanity check! */
 		if (tid != bf->bf_state.bfs_tid) {
 			device_printf(sc->sc_dev, "%s: bfs_tid %d !="
@@ -2341,7 +2364,9 @@ ath_tx_tid_hw_queue_norm(struct ath_soft
 		bf->bf_comp = NULL;	/* XXX default handler */
 
 		/* Punt to hardware or software txq */
+		ATH_TXQ_LOCK(txq);
 		ath_tx_handoff(sc, txq, bf);
+		ATH_TXQ_UNLOCK(txq);
 	}
 }
 
@@ -2351,13 +2376,10 @@ ath_tx_tid_hw_queue_norm(struct ath_soft
  * This function walks the list of TIDs (ie, ath_node TIDs
  * with queued traffic) and attempts to schedule traffic
  * from them.
- *
- * This must be called with the HW TXQ lock held.
  */
 void
 ath_txq_sched(struct ath_softc *sc, struct ath_txq *txq)
 {
-	ATH_TXQ_LOCK_ASSERT(txq);
 	struct ath_tid *atid, *next;
 
 	/*
@@ -2468,10 +2490,8 @@ ath_addba_request(struct ieee80211_node 
 	struct ath_node *an = ATH_NODE(ni);
 	struct ath_tid *atid = &an->an_tid[tid];
 
-	ATH_TXQ_LOCK(sc->sc_ac2q[tap->txa_ac]);
 	DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: called\n", __func__);
 	ath_tx_tid_pause(sc, atid);
-	ATH_TXQ_UNLOCK(sc->sc_ac2q[tap->txa_ac]);
 
 	return sc->sc_addba_request(ni, tap, dialogtoken, baparamset,
 	    batimeout);
@@ -2503,10 +2523,8 @@ ath_addba_response(struct ieee80211_node
 	 */
 	r = sc->sc_addba_response(ni, tap, status, code, batimeout);
 
-	ATH_TXQ_LOCK(sc->sc_ac2q[tap->txa_ac]);
 	DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: called\n", __func__);
 	ath_tx_tid_resume(sc, atid);
-	ATH_TXQ_UNLOCK(sc->sc_ac2q[tap->txa_ac]);
 	return r;
 }
 
@@ -2521,21 +2539,16 @@ ath_addba_stop(struct ieee80211_node *ni
 	int tid = WME_AC_TO_TID(tap->txa_ac);
 	struct ath_node *an = ATH_NODE(ni);
 	struct ath_tid *atid = &an->an_tid[tid];
-	struct ath_txq *txq = sc->sc_ac2q[atid->ac];
 
 	DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: called\n", __func__);
 
 	/* Pause TID traffic early, so there aren't any races */
-	ATH_TXQ_LOCK(txq);
 	ath_tx_tid_pause(sc, atid);
-	ATH_TXQ_UNLOCK(txq);
 
 	/* There's no need to hold the TXQ lock here */
 	sc->sc_addba_stop(ni, tap);
 
-	ATH_TXQ_LOCK(txq);
 	ath_tx_cleanup(sc, an, tid);
-	ATH_TXQ_UNLOCK(txq);
 }
 
 /*

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Mon Aug 15 00:38:14 2011	(r224873)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Mon Aug 15 03:41:48 2011	(r224874)
@@ -91,8 +91,10 @@ struct ath_buf;
  * Note that TID 16 (WME_NUM_TID+1) is for handling non-QoS frames.
  */
 struct ath_tid {
-	STAILQ_HEAD(,ath_buf) axq_q;		/* pending buffers        */
+	STAILQ_HEAD(,ath_buf) axq_q;		/* pending buffers */
+	struct mtx		axq_lock;	/* lock on q and link */
 	u_int			axq_depth;	/* SW queue depth */
+	char			axq_name[48];	/* lock name */
 	struct ath_node		*an;		/* pointer to parent */
 	int			tid;		/* tid */
 	int			ac;		/* which AC gets this trafic */
@@ -417,6 +419,7 @@ struct ath_softc {
 	struct ath_txq		sc_txq[HAL_NUM_TX_QUEUES];
 	struct ath_txq		*sc_ac2q[5];	/* WME AC -> h/w q map */ 
 	struct task		sc_txtask;	/* tx int processing */
+	struct task		sc_txschedtask;	/* tx processing task */
 	int			sc_wd_timer;	/* count down for wd timer */
 	struct callout		sc_wd_ch;	/* tx watchdog timer */
 	struct ath_tx_radiotap_header sc_tx_th;



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