From owner-svn-src-head@FreeBSD.ORG Mon Apr 1 20:57:14 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 36811797; Mon, 1 Apr 2013 20:57:14 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) by mx1.freebsd.org (Postfix) with ESMTP id 1A332C6; Mon, 1 Apr 2013 20:57:14 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.6/8.14.6) with ESMTP id r31KvDf0008596; Mon, 1 Apr 2013 20:57:13 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.6/8.14.5/Submit) id r31KvDbq008593; Mon, 1 Apr 2013 20:57:13 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201304012057.r31KvDbq008593@svn.freebsd.org> From: Adrian Chadd Date: Mon, 1 Apr 2013 20:57:13 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r248988 - head/sys/dev/ath X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Apr 2013 20:57:14 -0000 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;