svn commit: r240677 - head/sys/dev/ath
Adrian Chadd
adrian at FreeBSD.org
Tue Sep 18 20:33:04 UTC 2012
Author: adrian
Date: Tue Sep 18 20:33:04 2012
New Revision: 240677
URL: http://svn.freebsd.org/changeset/base/240677
Log:
Oops - take a copy of ath_tx_status from the buffer before the TX processing
is done.
The aggregate path was definitely accessing 'ts' before it was actually
being assigned.
This had the side effect of over-filtering frames, since occasionally that
bit would be '1'.
Whilst here, do the same thing in the non-aggregate completion function -
as calling the filter function may also invalidate bf.
Pointy hat to: adrian, for not noticing this over many, many code reviews.
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 Sep 18 20:28:55 2012 (r240676)
+++ head/sys/dev/ath/if_ath_tx.c Tue Sep 18 20:33:04 2012 (r240677)
@@ -3955,6 +3955,12 @@ ath_tx_aggr_comp_aggr(struct ath_softc *
DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR, "%s: called; hwq_depth=%d\n",
__func__, atid->hwq_depth);
+ /*
+ * Take a copy; this may be needed -after- bf_first
+ * has been completed and freed.
+ */
+ ts = bf_first->bf_status.ds_txstat;
+
TAILQ_INIT(&bf_q);
TAILQ_INIT(&bf_cq);
@@ -3970,6 +3976,8 @@ ath_tx_aggr_comp_aggr(struct ath_softc *
* If the TID is filtered, handle completing the filter
* transition before potentially kicking it to the cleanup
* function.
+ *
+ * XXX this is duplicate work, ew.
*/
if (atid->isfiltered)
ath_tx_tid_filt_comp_complete(sc, atid);
@@ -4030,11 +4038,6 @@ ath_tx_aggr_comp_aggr(struct ath_softc *
}
/*
- * Take a copy; this may be needed -after- bf_first
- * has been completed and freed.
- */
- ts = bf_first->bf_status.ds_txstat;
- /*
* XXX for now, use the first frame in the aggregate for
* XXX rate control completion; it's at least consistent.
*/
@@ -4255,10 +4258,16 @@ ath_tx_aggr_comp_unaggr(struct ath_softc
struct ath_node *an = ATH_NODE(ni);
int tid = bf->bf_state.bfs_tid;
struct ath_tid *atid = &an->an_tid[tid];
- struct ath_tx_status *ts = &bf->bf_status.ds_txstat;
+ struct ath_tx_status ts;
int drops = 0;
/*
+ * Take a copy of this; filtering/cloning the frame may free the
+ * bf pointer.
+ */
+ ts = bf->bf_status.ds_txstat;
+
+ /*
* Update rate control status here, before we possibly
* punt to retry or cleanup.
*
@@ -4268,7 +4277,7 @@ ath_tx_aggr_comp_unaggr(struct ath_softc
ath_tx_update_ratectrl(sc, ni, bf->bf_state.bfs_rc,
&bf->bf_status.ds_txstat,
bf->bf_state.bfs_pktlen,
- 1, (ts->ts_status == 0) ? 0 : 1);
+ 1, (ts.ts_status == 0) ? 0 : 1);
/*
* This is called early so atid->hwq_depth can be tracked.
@@ -4328,8 +4337,8 @@ ath_tx_aggr_comp_unaggr(struct ath_softc
* list as it will end up being recycled without having
* been made available for the hardware.
*/
- if ((ts->ts_status & HAL_TXERR_FILT) ||
- (ts->ts_status != 0 && atid->isfiltered)) {
+ if ((ts.ts_status & HAL_TXERR_FILT) ||
+ (ts.ts_status != 0 && atid->isfiltered)) {
int freeframe;
if (fail != 0)
@@ -4383,7 +4392,7 @@ ath_tx_aggr_comp_unaggr(struct ath_softc
#if 0
if (fail == 0 && ts->ts_status & HAL_TXERR_XRETRY) {
#endif
- if (fail == 0 && ts->ts_status != 0) {
+ if (fail == 0 && ts.ts_status != 0) {
ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: retry_unaggr\n",
__func__);
More information about the svn-src-head
mailing list