diff options
author | adrian <adrian@FreeBSD.org> | 2014-05-06 01:15:42 +0000 |
---|---|---|
committer | adrian <adrian@FreeBSD.org> | 2014-05-06 01:15:42 +0000 |
commit | 02d3dc06db747009e0c108b503d9d4c8efcf328b (patch) | |
tree | ac2c60be69d89c0f947a88c9039d7a4cef8f5aa1 | |
parent | 68ca190323e7bcd6d570e89b1cfd93450a605c07 (diff) | |
download | FreeBSD-src-02d3dc06db747009e0c108b503d9d4c8efcf328b.zip FreeBSD-src-02d3dc06db747009e0c108b503d9d4c8efcf328b.tar.gz |
Modify the RX path to keep the previous RX descriptor around once it's
used.
It turns out that the RX DMA engine does the same last-descriptor-link-
pointer-re-reading trick that the TX DMA engine. That is, the hardware
re-reads the link pointer before it moves onto the next descriptor.
Thus we can't free a descriptor before we move on; it's possible the
hardware will need to re-read the link pointer before we overwrite
it with a new one.
Tested:
* AR5416, STA mode
TODO:
* more thorough AP and STA mode testing!
* test on other pre-AR9380 NICs, just to be sure.
* Break out the RX descriptor grabbing bits from the RX completion
bits, like what is done in the RX EDMA code, so ..
* .. the RX lock can be held during ath_rx_proc(), but not across
packet input.
-rw-r--r-- | sys/dev/ath/if_ath_rx.c | 132 | ||||
-rw-r--r-- | sys/dev/ath/if_ath_sysctl.c | 6 | ||||
-rw-r--r-- | sys/dev/ath/if_athvar.h | 1 |
3 files changed, 122 insertions, 17 deletions
diff --git a/sys/dev/ath/if_ath_rx.c b/sys/dev/ath/if_ath_rx.c index 77c684a..d930fd8 100644 --- a/sys/dev/ath/if_ath_rx.c +++ b/sys/dev/ath/if_ath_rx.c @@ -245,6 +245,8 @@ ath_legacy_rxbuf_init(struct ath_softc *sc, struct ath_buf *bf) struct mbuf *m; struct ath_desc *ds; + /* XXX TODO: ATH_RX_LOCK_ASSERT(sc); */ + m = bf->bf_m; if (m == NULL) { /* @@ -974,6 +976,14 @@ rx_next: #define ATH_RX_MAX 128 +/* + * XXX TODO: break out the "get buffers" from "call ath_rx_pkt()" like + * the EDMA code does. + * + * XXX TODO: then, do all of the RX list management stuff inside + * ATH_RX_LOCK() so we don't end up potentially racing. The EDMA + * code is doing it right. + */ static void ath_rx_proc(struct ath_softc *sc, int resched) { @@ -995,6 +1005,7 @@ ath_rx_proc(struct ath_softc *sc, int resched) u_int64_t tsf; int npkts = 0; int kickpcu = 0; + int ret; /* XXX we must not hold the ATH_LOCK here */ ATH_UNLOCK_ASSERT(sc); @@ -1094,8 +1105,26 @@ ath_rx_proc(struct ath_softc *sc, int resched) if (ath_rx_pkt(sc, rs, status, tsf, nf, HAL_RX_QUEUE_HP, bf, m)) ngood++; rx_proc_next: - TAILQ_INSERT_TAIL(&sc->sc_rxbuf, bf, bf_list); - } while (ath_rxbuf_init(sc, bf) == 0); + /* + * If there's a holding buffer, insert that onto + * the RX list; the hardware is now definitely not pointing + * to it now. + */ + ret = 0; + if (sc->sc_rxedma[HAL_RX_QUEUE_HP].m_holdbf != NULL) { + TAILQ_INSERT_TAIL(&sc->sc_rxbuf, + sc->sc_rxedma[HAL_RX_QUEUE_HP].m_holdbf, + bf_list); + ret = ath_rxbuf_init(sc, + sc->sc_rxedma[HAL_RX_QUEUE_HP].m_holdbf); + } + /* + * Next, throw our buffer into the holding entry. The hardware + * may use the descriptor to read the link pointer before + * DMAing the next descriptor in to write out a packet. + */ + sc->sc_rxedma[HAL_RX_QUEUE_HP].m_holdbf = bf; + } while (ret == 0); /* rx signal state monitoring */ ath_hal_rxmonitor(ah, &sc->sc_halstats, sc->sc_curchan); @@ -1127,6 +1156,13 @@ rx_proc_next: * constantly write over the same frame, leading * the RX driver code here to get heavily confused. */ + /* + * XXX Has RX DMA stopped enough here to just call + * ath_startrecv()? + * XXX Do we need to use the holding buffer to restart + * RX DMA by appending entries to the final + * descriptor? Quite likely. + */ #if 1 ath_startrecv(sc); #else @@ -1217,6 +1253,58 @@ ath_legacy_flushrecv(struct ath_softc *sc) ath_rx_proc(sc, 0); } +static void +ath_legacy_flush_rxpending(struct ath_softc *sc) +{ + + /* XXX ATH_RX_LOCK_ASSERT(sc); */ + + if (sc->sc_rxedma[HAL_RX_QUEUE_LP].m_rxpending != NULL) { + m_freem(sc->sc_rxedma[HAL_RX_QUEUE_LP].m_rxpending); + sc->sc_rxedma[HAL_RX_QUEUE_LP].m_rxpending = NULL; + } + if (sc->sc_rxedma[HAL_RX_QUEUE_HP].m_rxpending != NULL) { + m_freem(sc->sc_rxedma[HAL_RX_QUEUE_HP].m_rxpending); + sc->sc_rxedma[HAL_RX_QUEUE_HP].m_rxpending = NULL; + } +} + +static int +ath_legacy_flush_rxholdbf(struct ath_softc *sc) +{ + struct ath_buf *bf; + + /* XXX ATH_RX_LOCK_ASSERT(sc); */ + /* + * If there are RX holding buffers, free them here and return + * them to the list. + * + * XXX should just verify that bf->bf_m is NULL, as it must + * be at this point! + */ + bf = sc->sc_rxedma[HAL_RX_QUEUE_HP].m_holdbf; + if (bf != NULL) { + if (bf->bf_m != NULL) + m_freem(bf->bf_m); + bf->bf_m = NULL; + TAILQ_INSERT_TAIL(&sc->sc_rxbuf, bf, bf_list); + (void) ath_rxbuf_init(sc, bf); + } + sc->sc_rxedma[HAL_RX_QUEUE_HP].m_holdbf = NULL; + + bf = sc->sc_rxedma[HAL_RX_QUEUE_LP].m_holdbf; + if (bf != NULL) { + if (bf->bf_m != NULL) + m_freem(bf->bf_m); + bf->bf_m = NULL; + TAILQ_INSERT_TAIL(&sc->sc_rxbuf, bf, bf_list); + (void) ath_rxbuf_init(sc, bf); + } + sc->sc_rxedma[HAL_RX_QUEUE_LP].m_holdbf = NULL; + + return (0); +} + /* * Disable the receive h/w in preparation for a reset. */ @@ -1228,6 +1316,8 @@ ath_legacy_stoprecv(struct ath_softc *sc, int dodelay) ((_pa) - (_sc)->sc_rxdma.dd_desc_paddr))) struct ath_hal *ah = sc->sc_ah; + ATH_RX_LOCK(sc); + ath_hal_stoppcurecv(ah); /* disable PCU */ ath_hal_setrxfilter(ah, 0); /* clear recv filter */ ath_hal_stopdmarecv(ah); /* disable DMA engine */ @@ -1261,22 +1351,23 @@ ath_legacy_stoprecv(struct ath_softc *sc, int dodelay) } } #endif - /* - * Free both high/low RX pending, just in case. - */ - if (sc->sc_rxedma[HAL_RX_QUEUE_LP].m_rxpending != NULL) { - m_freem(sc->sc_rxedma[HAL_RX_QUEUE_LP].m_rxpending); - sc->sc_rxedma[HAL_RX_QUEUE_LP].m_rxpending = NULL; - } - if (sc->sc_rxedma[HAL_RX_QUEUE_HP].m_rxpending != NULL) { - m_freem(sc->sc_rxedma[HAL_RX_QUEUE_HP].m_rxpending); - sc->sc_rxedma[HAL_RX_QUEUE_HP].m_rxpending = NULL; - } + + (void) ath_legacy_flush_rxpending(sc); + (void) ath_legacy_flush_rxholdbf(sc); + sc->sc_rxlink = NULL; /* just in case */ + + ATH_RX_UNLOCK(sc); #undef PA2DESC } /* + * XXX TODO: something was calling startrecv without calling + * stoprecv. Let's figure out what/why. It was showing up + * as a mbuf leak (rxpending) and ath_buf leak (holdbf.) + */ + +/* * Enable the receive h/w following a reset. */ static int @@ -1285,9 +1376,18 @@ ath_legacy_startrecv(struct ath_softc *sc) struct ath_hal *ah = sc->sc_ah; struct ath_buf *bf; + ATH_RX_LOCK(sc); + + /* + * XXX should verify these are already all NULL! + */ sc->sc_rxlink = NULL; - sc->sc_rxedma[HAL_RX_QUEUE_LP].m_rxpending = NULL; - sc->sc_rxedma[HAL_RX_QUEUE_HP].m_rxpending = NULL; + (void) ath_legacy_flush_rxpending(sc); + (void) ath_legacy_flush_rxholdbf(sc); + + /* + * Re-chain all of the buffers in the RX buffer list. + */ TAILQ_FOREACH(bf, &sc->sc_rxbuf, bf_list) { int error = ath_rxbuf_init(sc, bf); if (error != 0) { @@ -1303,6 +1403,8 @@ ath_legacy_startrecv(struct ath_softc *sc) ath_hal_rxena(ah); /* enable recv descriptors */ ath_mode_init(sc); /* set filters, etc. */ ath_hal_startpcurecv(ah); /* re-enable PCU/DMA engine */ + + ATH_RX_UNLOCK(sc); return 0; } diff --git a/sys/dev/ath/if_ath_sysctl.c b/sys/dev/ath/if_ath_sysctl.c index 9e27dbf..40a34d4 100644 --- a/sys/dev/ath/if_ath_sysctl.c +++ b/sys/dev/ath/if_ath_sysctl.c @@ -413,12 +413,14 @@ ath_sysctl_txagg(SYSCTL_HANDLER_ARGS) ATH_RX_LOCK(sc); for (i = 0; i < 2; i++) { - printf("%d: fifolen: %d/%d; head=%d; tail=%d\n", + printf("%d: fifolen: %d/%d; head=%d; tail=%d; m_pending=%p, m_holdbf=%p\n", i, sc->sc_rxedma[i].m_fifo_depth, sc->sc_rxedma[i].m_fifolen, sc->sc_rxedma[i].m_fifo_head, - sc->sc_rxedma[i].m_fifo_tail); + sc->sc_rxedma[i].m_fifo_tail, + sc->sc_rxedma[i].m_rxpending, + sc->sc_rxedma[i].m_holdbf); } i = 0; TAILQ_FOREACH(bf, &sc->sc_rxbuf, bf_list) { diff --git a/sys/dev/ath/if_athvar.h b/sys/dev/ath/if_athvar.h index f127543..d1ae089 100644 --- a/sys/dev/ath/if_athvar.h +++ b/sys/dev/ath/if_athvar.h @@ -510,6 +510,7 @@ struct ath_rx_edma { int m_fifo_tail; int m_fifo_depth; struct mbuf *m_rxpending; + struct ath_buf *m_holdbf; }; struct ath_tx_edma_fifo { |