svn commit: r225012 - user/adrian/if_ath_tx/sys/dev/ath
Adrian Chadd
adrian at FreeBSD.org
Fri Aug 19 15:14:14 UTC 2011
Author: adrian
Date: Fri Aug 19 15:14:13 2011
New Revision: 225012
URL: http://svn.freebsd.org/changeset/base/225012
Log:
Add BAW state tracking to ensure I'm updating the BAW before freeing
the ath_buf.
Add locking around the BAW related code. Since the BAW can be modified
by both net80211/ifnet context (ie interface TX) as well as the ath
task (frame scheduling & completion), it needs to be locked.
A node purge during active traffic could leave the BAW inconsistent.
This has fixed some immediate issues with the TX hanging, but it hasn't
fully fixed things.
Modified:
user/adrian/if_ath_tx/sys/dev/ath/if_ath.c
user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c
user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx_ht.c
Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Fri Aug 19 13:41:00 2011 (r225011)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Fri Aug 19 15:14:13 2011 (r225012)
@@ -4156,6 +4156,10 @@ ath_tx_default_comp(struct ath_softc *sc
st = ((bf->bf_txflags & HAL_TXDESC_NOACK) == 0) ?
ts->ts_status : HAL_TXERR_XRETRY;
+ if (bf->bf_state.bfs_dobaw)
+ device_printf(sc->sc_dev,
+ "%s: dobaw should've been cleared!\n", __func__);
+
/*
* Do any tx complete callback. Note this must
* be done before releasing the node reference.
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 Fri Aug 19 13:41:00 2011 (r225011)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Fri Aug 19 15:14:13 2011 (r225012)
@@ -1721,6 +1721,9 @@ ath_tx_action_frame_override_queue(struc
*
* + fits inside the BAW;
* + already has had a sequence number allocated.
+ *
+ * Since the BAW status may be modified by both the ath task and
+ * the net80211/ifnet contexts, the TID must be locked.
*/
void
ath_tx_addto_baw(struct ath_softc *sc, struct ath_node *an,
@@ -1729,6 +1732,8 @@ ath_tx_addto_baw(struct ath_softc *sc, s
int index, cindex;
struct ieee80211_tx_ampdu *tap;
+ ATH_TXQ_LOCK_ASSERT(tid);
+
if (bf->bf_state.bfs_isretried)
return;
@@ -1765,6 +1770,9 @@ ath_tx_addto_baw(struct ath_softc *sc, s
/*
* seq_start - left edge of BAW
* seq_next - current/next sequence number to allocate
+ *
+ * Since the BAW status may be modified by both the ath task and
+ * the net80211/ifnet contexts, the TID must be locked.
*/
static void
ath_tx_update_baw(struct ath_softc *sc, struct ath_node *an,
@@ -1773,6 +1781,8 @@ ath_tx_update_baw(struct ath_softc *sc,
int index, cindex;
struct ieee80211_tx_ampdu *tap;
+ ATH_TXQ_LOCK_ASSERT(tid);
+
tap = ath_tx_get_tx_tid(an, tid->tid);
index = ATH_BA_INDEX(tap->txa_start, seqno);
cindex = (tid->baw_head + index) & (ATH_TID_MAX_BUFS - 1);
@@ -2089,9 +2099,11 @@ ath_tx_tid_free_pkts(struct ath_softc *s
* the BAW.
*/
if (ath_tx_ampdu_running(sc, an, tid) &&
- bf->bf_state.bfs_dobaw)
+ bf->bf_state.bfs_dobaw) {
ath_tx_update_baw(sc, an, atid,
SEQNO(bf->bf_state.bfs_seqno));
+ bf->bf_state.bfs_dobaw = 0;
+ }
ATH_TXQ_REMOVE(atid, bf, bf_list);
ATH_TXQ_UNLOCK(atid);
ath_tx_default_comp(sc, bf, -1);
@@ -2224,6 +2236,7 @@ ath_tx_cleanup(struct ath_softc *sc, str
if (bf->bf_state.bfs_dobaw)
ath_tx_update_baw(sc, an, atid,
SEQNO(bf->bf_state.bfs_seqno));
+ bf->bf_state.bfs_dobaw = 0;
/*
* Call the default completion handler with "fail" just
* so upper levels are suitably notified about this.
@@ -2251,6 +2264,8 @@ ath_tx_cleanup(struct ath_softc *sc, str
* not yet ACKed.
*/
tap = ath_tx_get_tx_tid(an, tid);
+ /* Need the lock - fiddling with BAW */
+ ATH_TXQ_LOCK(atid);
while (atid->baw_head != atid->baw_tail) {
if (atid->tx_buf[atid->baw_head]) {
atid->incomp++;
@@ -2260,6 +2275,7 @@ ath_tx_cleanup(struct ath_softc *sc, str
INCR(atid->baw_head, ATH_TID_MAX_BUFS);
INCR(tap->txa_start, IEEE80211_SEQ_RANGE);
}
+ ATH_TXQ_UNLOCK(atid);
/*
* If cleanup is required, defer TID scheduling
@@ -2314,9 +2330,13 @@ ath_tx_aggr_retry_unaggr(struct ath_soft
sc->sc_stats.ast_tx_swretrymax++;
/* Update BAW anyway */
- if (bf->bf_state.bfs_dobaw)
+ if (bf->bf_state.bfs_dobaw) {
+ ATH_TXQ_LOCK(atid);
ath_tx_update_baw(sc, an, atid,
SEQNO(bf->bf_state.bfs_seqno));
+ ATH_TXQ_UNLOCK(atid);
+ }
+ bf->bf_state.bfs_dobaw = 0;
/* Send BAR frame */
/*
@@ -2369,8 +2389,10 @@ ath_tx_aggr_retry_unaggr(struct ath_soft
*/
if (bf->bf_flags & ATH_BUF_BUSY) {
bf->bf_flags &= ~ ATH_BUF_BUSY;
+#if 0
DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL,
"%s: bf %p: ATH_BUF_BUSY\n", __func__, bf);
+#endif
}
/*
@@ -2409,7 +2431,10 @@ ath_tx_retry_subframe(struct ath_softc *
if (bf->bf_state.bfs_retries >= SWMAX_RETRIES) {
device_printf(sc->sc_dev, "%s: max retries: seqno %d\n",
__func__, SEQNO(bf->bf_state.bfs_seqno));
+ ATH_TXQ_LOCK(atid);
ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno));
+ bf->bf_state.bfs_dobaw = 0;
+ ATH_TXQ_UNLOCK(atid);
/* XXX subframe completion status? is that valid here? */
ath_tx_default_comp(sc, bf, 0);
return 1;
@@ -2417,8 +2442,10 @@ ath_tx_retry_subframe(struct ath_softc *
if (bf->bf_flags & ATH_BUF_BUSY) {
bf->bf_flags &= ~ ATH_BUF_BUSY;
+#if 0
DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL,
"%s: bf %p: ATH_BUF_BUSY\n", __func__, bf);
+#endif
}
ath_tx_set_retry(sc, bf);
@@ -2617,8 +2644,11 @@ ath_tx_aggr_comp_aggr(struct ath_softc *
ATH_BA_ISSET(ba, ba_index));
if (tx_ok && ATH_BA_ISSET(ba, ba_index)) {
+ ATH_TXQ_LOCK(atid);
ath_tx_update_baw(sc, an, atid,
SEQNO(bf->bf_state.bfs_seqno));
+ ATH_TXQ_UNLOCK(atid);
+ bf->bf_state.bfs_dobaw = 0;
ath_tx_default_comp(sc, bf, 0);
} else {
drops += ath_tx_retry_subframe(sc, bf, &bf_q);
@@ -2714,8 +2744,12 @@ ath_tx_aggr_comp_unaggr(struct ath_softc
/* Success? Complete */
DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: TID=%d, seqno %d\n",
__func__, tid, SEQNO(bf->bf_state.bfs_seqno));
- if (bf->bf_state.bfs_dobaw)
+ if (bf->bf_state.bfs_dobaw) {
+ ATH_TXQ_LOCK(atid);
ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno));
+ bf->bf_state.bfs_dobaw = 0;
+ ATH_TXQ_UNLOCK(atid);
+ }
ath_tx_default_comp(sc, bf, fail);
/* bf is freed at this point */
Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx_ht.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx_ht.c Fri Aug 19 13:41:00 2011 (r225011)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx_ht.c Fri Aug 19 15:14:13 2011 (r225012)
@@ -486,9 +486,16 @@ ath_tx_form_aggr(struct ath_softc *sc, s
* this packet is part of an aggregate.
*/
ATH_TXQ_REMOVE(tid, bf, bf_list);
- ATH_TXQ_UNLOCK(tid);
+ /* The TID lock is required for the BAW update */
ath_tx_addto_baw(sc, an, tid, bf);
+ ATH_TXQ_UNLOCK(tid);
+
+ /*
+ * Add the now owned buffer (which isn't
+ * on the software TXQ any longer) to our
+ * aggregate frame list.
+ */
TAILQ_INSERT_TAIL(bf_q, bf, bf_list);
nframes ++;
More information about the svn-src-user
mailing list