Author: adrian
Date: Mon Apr 1 20:57:13 2013
New Revision: 248988
URL: http://svnweb.freebsd.org/changeset/base/248988
Log:
Ensure that we only call the busdma unmap/flush routines once, when
the buffer is being freed.
* When buffers are cloned, the original mapping isn't copied but it
wasn't freeing the mapping until later. To be safe, free the
mapping when the buffer is cloned.
* ath_freebuf() now no longer calls the busdma sync/unmap routines.
* ath_tx_freebuf() now calls sync/unmap.
* Call sync first, before calling unmap.
Tested:
* AR5416, STA mode
Modified:
head/sys/dev/ath/if_ath.c
head/sys/dev/ath/if_ath_misc.h
head/sys/dev/ath/if_ath_tx.c
Modified: head/sys/dev/ath/if_ath.c
==============================================================================
--- head/sys/dev/ath/if_ath.c Mon Apr 1 20:50:07 2013 (r248987)
+++ head/sys/dev/ath/if_ath.c Mon Apr 1 20:57:13 2013 (r248988)
@@ -2512,7 +2512,7 @@ _ath_getbuf_locked(struct ath_softc *sc,
* The caller must free the buffer using ath_freebuf().
*/
struct ath_buf *
-ath_buf_clone(struct ath_softc *sc, const struct ath_buf *bf)
+ath_buf_clone(struct ath_softc *sc, struct ath_buf *bf)
{
struct ath_buf *tbf;
@@ -2528,14 +2528,6 @@ ath_buf_clone(struct ath_softc *sc, cons
tbf->bf_flags = bf->bf_flags & ATH_BUF_FLAGS_CLONE;
tbf->bf_status = bf->bf_status;
tbf->bf_m = bf->bf_m;
- /*
- * XXX Copy the node reference, the caller is responsible
- * for deleting the node reference before it frees its
- * buffer.
- *
- * XXX It's done like this so we don't call the net80211
- * code whilst having active TX queue locks held.
- */
tbf->bf_node = bf->bf_node;
/* will be setup by the chain/setup function */
tbf->bf_lastds = NULL;
@@ -2547,6 +2539,23 @@ ath_buf_clone(struct ath_softc *sc, cons
/* The caller has to re-init the descriptor + links */
+ /*
+ * Free the DMA mapping here, before we NULL the mbuf.
+ * We must only call bus_dmamap_unload() once per mbuf chain
+ * or behaviour is undefined.
+ */
+ if (bf->bf_m != NULL) {
+ /*
+ * XXX is this POSTWRITE call required?
+ */
+ bus_dmamap_sync(sc->sc_dmat, bf->bf_dmamap,
+ BUS_DMASYNC_POSTWRITE);
+ bus_dmamap_unload(sc->sc_dmat, bf->bf_dmamap);
+ }
+
+ bf->bf_m = NULL;
+ bf->bf_node = NULL;
+
/* Copy state */
memcpy(&tbf->bf_state, &bf->bf_state, sizeof(bf->bf_state));
@@ -4220,9 +4229,6 @@ ath_txq_addholdingbuf(struct ath_softc *
void
ath_freebuf(struct ath_softc *sc, struct ath_buf *bf)
{
- bus_dmamap_unload(sc->sc_dmat, bf->bf_dmamap);
- bus_dmamap_sync(sc->sc_dmat, bf->bf_dmamap, BUS_DMASYNC_POSTWRITE);
-
KASSERT((bf->bf_node == NULL), ("%s: bf->bf_node != NULL\n", __func__));
KASSERT((bf->bf_m == NULL), ("%s: bf->bf_m != NULL\n", __func__));
@@ -4256,6 +4262,17 @@ ath_tx_freebuf(struct ath_softc *sc, str
struct ieee80211_node *ni = bf->bf_node;
struct mbuf *m0 = bf->bf_m;
+ /*
+ * Make sure that we only sync/unload if there's an mbuf.
+ * If not (eg we cloned a buffer), the unload will have already
+ * occured.
+ */
+ if (bf->bf_m != NULL) {
+ bus_dmamap_sync(sc->sc_dmat, bf->bf_dmamap,
+ BUS_DMASYNC_POSTWRITE);
+ bus_dmamap_unload(sc->sc_dmat, bf->bf_dmamap);
+ }
+
bf->bf_node = NULL;
bf->bf_m = NULL;
@@ -4270,13 +4287,9 @@ ath_tx_freebuf(struct ath_softc *sc, str
ieee80211_process_callback(ni, m0, status);
ieee80211_free_node(ni);
}
- m_freem(m0);
- /*
- * XXX the buffer used to be freed -after-, but the DMA map was
- * freed where ath_freebuf() now is. I've no idea what this
- * will do.
- */
+ /* Finally, we don't need this mbuf any longer */
+ m_freem(m0);
}
static struct ath_buf *
Modified: head/sys/dev/ath/if_ath_misc.h
==============================================================================
--- head/sys/dev/ath/if_ath_misc.h Mon Apr 1 20:50:07 2013
(r248987)
+++ head/sys/dev/ath/if_ath_misc.h Mon Apr 1 20:57:13 2013
(r248988)
@@ -59,7 +59,7 @@ extern struct ath_buf * ath_getbuf(struc
extern struct ath_buf * _ath_getbuf_locked(struct ath_softc *sc,
ath_buf_type_t btype);
extern struct ath_buf * ath_buf_clone(struct ath_softc *sc,
- const struct ath_buf *bf);
+ struct ath_buf *bf);
/* XXX change this to NULL the buffer pointer? */
extern void ath_freebuf(struct ath_softc *sc, struct ath_buf *bf);
extern void ath_returnbuf_head(struct ath_softc *sc, struct ath_buf *bf);
Modified: head/sys/dev/ath/if_ath_tx.c
==============================================================================
--- head/sys/dev/ath/if_ath_tx.c Mon Apr 1 20:50:07 2013
(r248987)
+++ head/sys/dev/ath/if_ath_tx.c Mon Apr 1 20:57:13 2013
(r248988)
@@ -3842,6 +3842,12 @@ ath_tx_retry_clone(struct ath_softc *sc,
struct ath_buf *nbf;
int error;
+ /*
+ * Clone the buffer. This will handle the dma unmap and
+ * copy the node reference to the new buffer. If this
+ * works out, 'bf' will have no DMA mapping, no mbuf
+ * pointer and no node reference.
+ */
nbf = ath_buf_clone(sc, bf);
#if 0
@@ -3879,9 +3885,7 @@ ath_tx_retry_clone(struct ath_softc *sc,
if (bf->bf_state.bfs_dobaw)
ath_tx_switch_baw_buf(sc, an, tid, bf, nbf);
- /* Free current buffer; return the older buffer */
- bf->bf_m = NULL;
- bf->bf_node = NULL;
+ /* Free original buffer; return new buffer */
ath_freebuf(sc, bf);
return nbf;
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
