Add locking around the new holdingbf code.

Since this is being done during buffer free, it's a crap shoot whether
the TX path lock is held or not.  I tried putting the ath_freebuf() code
inside the TX lock and I got all kinds of locking issues - it turns out
that the buffer free path sometimes is called with the lock held and
sometimes isn't. So I'll go and fix that soon.

Hence for now the holdingbf buffers are protected by the TXBUF lock.
This commit is contained in:
adrian 2013-03-15 02:52:37 +00:00
parent da9ca4325a
commit 7be3120d83
2 changed files with 16 additions and 4 deletions

View File

@ -4156,14 +4156,13 @@ ath_returnbuf_head(struct ath_softc *sc, struct ath_buf *bf)
static void
ath_txq_freeholdingbuf(struct ath_softc *sc, struct ath_txq *txq)
{
ATH_TXBUF_LOCK_ASSERT(sc);
if (txq->axq_holdingbf == NULL)
return;
txq->axq_holdingbf->bf_flags &= ~ATH_BUF_BUSY;
ATH_TXBUF_LOCK(sc);
ath_returnbuf_tail(sc, txq->axq_holdingbf);
ATH_TXBUF_UNLOCK(sc);
txq->axq_holdingbf = NULL;
}
@ -4176,6 +4175,8 @@ ath_txq_addholdingbuf(struct ath_softc *sc, struct ath_buf *bf)
{
struct ath_txq *txq;
ATH_TXBUF_LOCK_ASSERT(sc);
/* XXX assert ATH_BUF_BUSY is set */
/* XXX assert the tx queue is under the max number */
@ -4188,10 +4189,8 @@ ath_txq_addholdingbuf(struct ath_softc *sc, struct ath_buf *bf)
ath_returnbuf_tail(sc, bf);
return;
}
txq = &sc->sc_txq[bf->bf_state.bfs_tx_queue];
ath_txq_freeholdingbuf(sc, txq);
txq->axq_holdingbf = bf;
}
@ -4219,7 +4218,9 @@ ath_freebuf(struct ath_softc *sc, struct ath_buf *bf)
* If this buffer is busy, push it onto the holding queue
*/
if (bf->bf_flags & ATH_BUF_BUSY) {
ATH_TXBUF_LOCK(sc);
ath_txq_addholdingbuf(sc, bf);
ATH_TXBUF_UNLOCK(sc);
return;
}
@ -4342,7 +4343,9 @@ ath_tx_draintxq(struct ath_softc *sc, struct ath_txq *txq)
/*
* Free the holding buffer if it exists
*/
ATH_TXBUF_LOCK(sc);
ath_txq_freeholdingbuf(sc, txq);
ATH_TXBUF_UNLOCK(sc);
/*
* Drain software queued frames which are on

View File

@ -329,6 +329,15 @@ struct ath_txq {
u_int axq_intrcnt; /* interrupt count */
u_int32_t *axq_link; /* link ptr in last TX desc */
TAILQ_HEAD(axq_q_s, ath_buf) axq_q; /* transmit queue */
/*
* XXX the holdingbf field is protected by the TXBUF lock
* for now, NOT the TX lock.
*
* Architecturally, it would likely be better to move
* the holdingbf field to a separate array in ath_softc
* just to highlight that it's not protected by the normal
* TX path lock.
*/
struct ath_buf *axq_holdingbf; /* holding TX buffer */
char axq_name[12]; /* e.g. "ath0_txq4" */