From owner-svn-src-head@FreeBSD.ORG Tue Mar 19 19:32:29 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 472328ED; Tue, 19 Mar 2013 19:32:29 +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 37A89F89; Tue, 19 Mar 2013 19:32:29 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.6/8.14.6) with ESMTP id r2JJWTU3013019; Tue, 19 Mar 2013 19:32:29 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.6/8.14.5/Submit) id r2JJWSUi013015; Tue, 19 Mar 2013 19:32:28 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201303191932.r2JJWSUi013015@svn.freebsd.org> From: Adrian Chadd Date: Tue, 19 Mar 2013 19:32:28 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r248529 - 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: Tue, 19 Mar 2013 19:32:29 -0000 Author: adrian Date: Tue Mar 19 19:32:28 2013 New Revision: 248529 URL: http://svnweb.freebsd.org/changeset/base/248529 Log: Break out the RX completion path into "FIFO check / refill" and "complete RX frames." The 128 entry RX FIFO is really easy to fill up and miss refilling when it's done in the ath taskq - as that gets blocked up doing RX completion, TX completion and other random things. So the 128 entry RX FIFO now gets emptied and refilled in the ath_intr() task (and it grabs / releases locks, so now ath_intr() can't just be a FAST handler yet!) but the locks aren't held for very long. The completion part is done in the ath taskqueue context. Details: * Create a new completed frame list - sc->sc_rx_rxlist; * Split the EDMA RX process queue into two halves - one that processes the RX FIFO and refills it with new frames; another that completes the completed frame list; * When tearing down the driver, flush whatever is in the deferred queue as well as what's in the FIFO; * Create two new RX methods - one that processes all RX queues, one that processes the given RX queue. When MSI is implemented, we get told which RX queue the interrupt came in on so we can specifically schedule that. (And I can do that with the non-MSI path too; I'll figure that out later.) * Convert the legacy code over to use these new RX methods; * Replace all the instances of the RX taskqueue enqueue with a call to a relevant RX method to enqueue one or all RX queues. Tested: * AR9380, STA * AR9580, STA * AR5413, STA Modified: head/sys/dev/ath/if_ath.c head/sys/dev/ath/if_ath_rx.c head/sys/dev/ath/if_ath_rx_edma.c head/sys/dev/ath/if_athvar.h Modified: head/sys/dev/ath/if_ath.c ============================================================================== --- head/sys/dev/ath/if_ath.c Tue Mar 19 17:55:36 2013 (r248528) +++ head/sys/dev/ath/if_ath.c Tue Mar 19 19:32:28 2013 (r248529) @@ -837,6 +837,11 @@ ath_attach(u_int16_t devid, struct ath_s } /* + * Initialise the deferred completed RX buffer list. + */ + TAILQ_INIT(&sc->sc_rx_rxlist); + + /* * Indicate we need the 802.11 header padded to a * 32-bit boundary for 4-address and QoS frames. */ @@ -1711,7 +1716,7 @@ ath_intr(void *arg) * traffic so any frames held on the staging * queue are aged and potentially flushed. */ - taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); + sc->sc_rx.recv_sched(sc, 1); #endif } } @@ -1751,13 +1756,13 @@ ath_intr(void *arg) if (! sc->sc_kickpcu) sc->sc_rxlink = NULL; sc->sc_kickpcu = 1; + ATH_PCU_UNLOCK(sc); /* * Enqueue an RX proc, to handled whatever * is in the RX queue. * This will then kick the PCU. */ - taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); - ATH_PCU_UNLOCK(sc); + sc->sc_rx.recv_sched(sc, 1); } if (status & HAL_INT_TXURN) { sc->sc_stats.ast_txurn++; @@ -1770,7 +1775,7 @@ ath_intr(void *arg) */ if (status & (HAL_INT_RX | HAL_INT_RXHP | HAL_INT_RXLP)) { sc->sc_stats.ast_rx_intr++; - taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); + sc->sc_rx.recv_sched(sc, 1); } if (status & HAL_INT_TX) { sc->sc_stats.ast_tx_intr++; Modified: head/sys/dev/ath/if_ath_rx.c ============================================================================== --- head/sys/dev/ath/if_ath_rx.c Tue Mar 19 17:55:36 2013 (r248528) +++ head/sys/dev/ath/if_ath_rx.c Tue Mar 19 19:32:28 2013 (r248529) @@ -1030,7 +1030,7 @@ rx_proc_next: * will reduce latency. */ if (npkts >= ATH_RX_MAX) - taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); + sc->sc_rx.recv_sched(sc, resched); ATH_PCU_LOCK(sc); sc->sc_rxproc_cnt--; @@ -1181,6 +1181,21 @@ ath_legacy_dma_rxteardown(struct ath_sof return (0); } +static void +ath_legacy_recv_sched(struct ath_softc *sc, int dosched) +{ + + taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); +} + +static void +ath_legacy_recv_sched_queue(struct ath_softc *sc, HAL_RX_QUEUE q, + int dosched) +{ + + taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); +} + void ath_recv_setup_legacy(struct ath_softc *sc) { @@ -1200,4 +1215,6 @@ ath_recv_setup_legacy(struct ath_softc * sc->sc_rx.recv_setup = ath_legacy_dma_rxsetup; sc->sc_rx.recv_teardown = ath_legacy_dma_rxteardown; + sc->sc_rx.recv_sched = ath_legacy_recv_sched; + sc->sc_rx.recv_sched_queue = ath_legacy_recv_sched_queue; } Modified: head/sys/dev/ath/if_ath_rx_edma.c ============================================================================== --- head/sys/dev/ath/if_ath_rx_edma.c Tue Mar 19 17:55:36 2013 (r248528) +++ head/sys/dev/ath/if_ath_rx_edma.c Tue Mar 19 19:32:28 2013 (r248529) @@ -132,12 +132,8 @@ MALLOC_DECLARE(M_ATHDEV); /* * XXX TODO: * - * + Add an RX lock, just to ensure we don't have things clash; * + Make sure the FIFO is correctly flushed and reinitialised * through a reset; - * + Handle the "kickpcu" state where the FIFO overflows. - * + Implement a "flush" routine, which doesn't push any - * new frames into the FIFO. * + Verify multi-descriptor frames work! * + There's a "memory use after free" which needs to be tracked down * and fixed ASAP. I've seen this in the legacy path too, so it @@ -152,7 +148,9 @@ static int ath_edma_rxfifo_alloc(struct int nbufs); static int ath_edma_rxfifo_flush(struct ath_softc *sc, HAL_RX_QUEUE qtype); static void ath_edma_rxbuf_free(struct ath_softc *sc, struct ath_buf *bf); -static int ath_edma_recv_proc_queue(struct ath_softc *sc, +static void ath_edma_recv_proc_queue(struct ath_softc *sc, + HAL_RX_QUEUE qtype, int dosched); +static int ath_edma_recv_proc_deferred_queue(struct ath_softc *sc, HAL_RX_QUEUE qtype, int dosched); static void @@ -283,6 +281,24 @@ ath_edma_startrecv(struct ath_softc *sc) } static void +ath_edma_recv_sched_queue(struct ath_softc *sc, HAL_RX_QUEUE qtype, + int dosched) +{ + + ath_edma_recv_proc_queue(sc, qtype, dosched); + taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); +} + +static void +ath_edma_recv_sched(struct ath_softc *sc, int dosched) +{ + + ath_edma_recv_proc_queue(sc, HAL_RX_QUEUE_HP, dosched); + ath_edma_recv_proc_queue(sc, HAL_RX_QUEUE_LP, dosched); + taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); +} + +static void ath_edma_recv_flush(struct ath_softc *sc) { @@ -292,27 +308,27 @@ ath_edma_recv_flush(struct ath_softc *sc sc->sc_rxproc_cnt++; ATH_PCU_UNLOCK(sc); + /* + * Flush any active frames from FIFO -> deferred list + */ ath_edma_recv_proc_queue(sc, HAL_RX_QUEUE_HP, 0); ath_edma_recv_proc_queue(sc, HAL_RX_QUEUE_LP, 0); + /* + * Process what's in the deferred queue + */ + ath_edma_recv_proc_deferred_queue(sc, HAL_RX_QUEUE_HP, 0); + ath_edma_recv_proc_deferred_queue(sc, HAL_RX_QUEUE_LP, 0); + ATH_PCU_LOCK(sc); sc->sc_rxproc_cnt--; ATH_PCU_UNLOCK(sc); } /* - * Process frames from the current queue. - * - * TODO: - * - * + Add a "dosched" flag, so we don't reschedule any FIFO frames - * to the hardware or re-kick the PCU after 'kickpcu' is set. - * - * + Perhaps split "check FIFO contents" and "handle frames", so - * we can run the "check FIFO contents" in ath_intr(), but - * "handle frames" in the RX tasklet. + * Process frames from the current queue into the deferred queue. */ -static int +static void ath_edma_recv_proc_queue(struct ath_softc *sc, HAL_RX_QUEUE qtype, int dosched) { @@ -323,12 +339,8 @@ ath_edma_recv_proc_queue(struct ath_soft struct mbuf *m; struct ath_hal *ah = sc->sc_ah; uint64_t tsf; - int16_t nf; - int ngood = 0, npkts = 0; - ath_bufhead rxlist; - struct ath_buf *next; - - TAILQ_INIT(&rxlist); + uint16_t nf; + int npkts = 0; tsf = ath_hal_gettsf64(ah); nf = ath_hal_getchannoise(ah, sc->sc_curchan); @@ -386,7 +398,7 @@ ath_edma_recv_proc_queue(struct ath_soft * queue. */ re->m_fifo[re->m_fifo_head] = NULL; - TAILQ_INSERT_TAIL(&rxlist, bf, bf_list); + TAILQ_INSERT_TAIL(&sc->sc_rx_rxlist, bf, bf_list); /* Bump the descriptor FIFO stats */ INCR(re->m_fifo_head, re->m_fifolen); @@ -400,6 +412,78 @@ ath_edma_recv_proc_queue(struct ath_soft ATH_RX_UNLOCK(sc); + /* rx signal state monitoring */ + ath_hal_rxmonitor(ah, &sc->sc_halstats, sc->sc_curchan); + + ATH_KTR(sc, ATH_KTR_INTERRUPTS, 1, + "ath edma rx proc: npkts=%d\n", + npkts); + + /* Handle resched and kickpcu appropriately */ + ATH_PCU_LOCK(sc); + if (dosched && sc->sc_kickpcu) { + ATH_KTR(sc, ATH_KTR_ERROR, 0, + "ath_edma_recv_proc_queue(): kickpcu"); + device_printf(sc->sc_dev, + "%s: handled npkts %d\n", + __func__, npkts); + + /* + * XXX TODO: what should occur here? Just re-poke and + * re-enable the RX FIFO? + */ + sc->sc_kickpcu = 0; + } + ATH_PCU_UNLOCK(sc); + + return; +} + +/* + * Flush the deferred queue. + * + * This destructively flushes the deferred queue - it doesn't + * call the wireless stack on each mbuf. + */ +static void +ath_edma_flush_deferred_queue(struct ath_softc *sc) +{ + struct ath_buf *bf, *next; + + ATH_RX_LOCK_ASSERT(sc); + /* Free in one set, inside the lock */ + TAILQ_FOREACH_SAFE(bf, &sc->sc_rx_rxlist, bf_list, next) { + /* Free the buffer/mbuf */ + ath_edma_rxbuf_free(sc, bf); + } +} + +static int +ath_edma_recv_proc_deferred_queue(struct ath_softc *sc, HAL_RX_QUEUE qtype, + int dosched) +{ + int ngood = 0; + uint64_t tsf; + struct ath_buf *bf, *next; + struct ath_rx_status *rs; + int16_t nf; + ath_bufhead rxlist; + + TAILQ_INIT(&rxlist); + + nf = ath_hal_getchannoise(sc->sc_ah, sc->sc_curchan); + /* + * XXX TODO: the NF/TSF should be stamped on the bufs themselves, + * otherwise we may end up adding in the wrong values if this + * is delayed too far.. + */ + tsf = ath_hal_gettsf64(sc->sc_ah); + + /* Copy the list over */ + ATH_RX_LOCK(sc); + TAILQ_CONCAT(&rxlist, &sc->sc_rx_rxlist, bf_list); + ATH_RX_UNLOCK(sc); + /* Handle the completed descriptors */ TAILQ_FOREACH_SAFE(bf, &rxlist, bf_list, next) { /* @@ -417,6 +501,14 @@ ath_edma_recv_proc_queue(struct ath_soft ngood++; } + if (ngood) { + sc->sc_lastrx = tsf; + } + + ATH_KTR(sc, ATH_KTR_INTERRUPTS, 1, + "ath edma rx deferred proc: ngood=%d\n", + ngood); + /* Free in one set, inside the lock */ ATH_RX_LOCK(sc); TAILQ_FOREACH_SAFE(bf, &rxlist, bf_list, next) { @@ -425,32 +517,6 @@ ath_edma_recv_proc_queue(struct ath_soft } ATH_RX_UNLOCK(sc); - /* rx signal state monitoring */ - ath_hal_rxmonitor(ah, &sc->sc_halstats, sc->sc_curchan); - if (ngood) - sc->sc_lastrx = tsf; - - ATH_KTR(sc, ATH_KTR_INTERRUPTS, 2, - "ath edma rx proc: npkts=%d, ngood=%d", - npkts, ngood); - - /* Handle resched and kickpcu appropriately */ - ATH_PCU_LOCK(sc); - if (dosched && sc->sc_kickpcu) { - ATH_KTR(sc, ATH_KTR_ERROR, 0, - "ath_edma_recv_proc_queue(): kickpcu"); - device_printf(sc->sc_dev, - "%s: handled npkts %d ngood %d\n", - __func__, npkts, ngood); - - /* - * XXX TODO: what should occur here? Just re-poke and - * re-enable the RX FIFO? - */ - sc->sc_kickpcu = 0; - } - ATH_PCU_UNLOCK(sc); - return (ngood); } @@ -480,6 +546,9 @@ ath_edma_recv_tasklet(void *arg, int npe ath_edma_recv_proc_queue(sc, HAL_RX_QUEUE_HP, 1); ath_edma_recv_proc_queue(sc, HAL_RX_QUEUE_LP, 1); + ath_edma_recv_proc_deferred_queue(sc, HAL_RX_QUEUE_HP, 1); + ath_edma_recv_proc_deferred_queue(sc, HAL_RX_QUEUE_LP, 1); + /* XXX inside IF_LOCK ? */ if ((ifp->if_drv_flags & IFF_DRV_OACTIVE) == 0) { #ifdef IEEE80211_SUPPORT_SUPERG @@ -812,6 +881,7 @@ ath_edma_dma_rxteardown(struct ath_softc { ATH_RX_LOCK(sc); + ath_edma_flush_deferred_queue(sc); ath_edma_rxfifo_flush(sc, HAL_RX_QUEUE_HP); ath_edma_rxfifo_free(sc, HAL_RX_QUEUE_HP); @@ -854,4 +924,7 @@ ath_recv_setup_edma(struct ath_softc *sc sc->sc_rx.recv_setup = ath_edma_dma_rxsetup; sc->sc_rx.recv_teardown = ath_edma_dma_rxteardown; + + sc->sc_rx.recv_sched = ath_edma_recv_sched; + sc->sc_rx.recv_sched_queue = ath_edma_recv_sched_queue; } Modified: head/sys/dev/ath/if_athvar.h ============================================================================== --- head/sys/dev/ath/if_athvar.h Tue Mar 19 17:55:36 2013 (r248528) +++ head/sys/dev/ath/if_athvar.h Tue Mar 19 19:32:28 2013 (r248529) @@ -442,6 +442,9 @@ typedef enum { } ATH_RESET_TYPE; struct ath_rx_methods { + void (*recv_sched_queue)(struct ath_softc *sc, + HAL_RX_QUEUE q, int dosched); + void (*recv_sched)(struct ath_softc *sc, int dosched); void (*recv_stop)(struct ath_softc *sc, int dodelay); int (*recv_start)(struct ath_softc *sc); void (*recv_flush)(struct ath_softc *sc); @@ -656,6 +659,7 @@ struct ath_softc { struct ath_descdma sc_rxdma; /* RX descriptors */ ath_bufhead sc_rxbuf; /* receive buffer */ + ath_bufhead sc_rx_rxlist; /* deferred RX completion */ u_int32_t *sc_rxlink; /* link ptr in last RX desc */ struct task sc_rxtask; /* rx int processing */ u_int8_t sc_defant; /* current default antenna */