From owner-svn-src-all@FreeBSD.ORG Tue Jan 15 18:01:25 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 520611E6; Tue, 15 Jan 2013 18:01:25 +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 351BEC06; Tue, 15 Jan 2013 18:01:25 +0000 (UTC) Received: from svn.freebsd.org (svn.FreeBSD.org [8.8.178.70]) by svn.freebsd.org (8.14.5/8.14.5) with ESMTP id r0FI1OlF023475; Tue, 15 Jan 2013 18:01:24 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.5/8.14.5/Submit) id r0FI1NYQ023469; Tue, 15 Jan 2013 18:01:23 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201301151801.r0FI1NYQ023469@svn.freebsd.org> From: Adrian Chadd Date: Tue, 15 Jan 2013 18:01:23 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r245465 - 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-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Jan 2013 18:01:25 -0000 Author: adrian Date: Tue Jan 15 18:01:23 2013 New Revision: 245465 URL: http://svnweb.freebsd.org/changeset/base/245465 Log: Implement frame (data) transmission using if_transmit(), rather than if_start(). This removes the overlapping data path TX from occuring, which solves quite a number of the potential TX queue races in ath(4). It doesn't fix the net80211 layer TX queue races and it doesn't fix the raw TX path yet, but it's an important step towards this. This hasn't dropped the TX performance in my testing; primarily because now the TX path can quickly queue frames and continue along processing. This involves a few rather deep changes: * Use the ath_buf as a queue placeholder for now, as we need to be able to support queuing a list of mbufs (ie, when transmitting fragments) and m_nextpkt can't be used here (because it's what is joining the fragments together) * if_transmit() now simply allocates the ath_buf and queues it to a driver TX staging queue. * TX is now moved into a taskqueue function. * The TX taskqueue function now dequeues and transmits frames. * Fragments are handled correctly here - as the current API passes the fragment list as one mbuf list (joined with m_nextpkt) through to the driver if_transmit(). * For the couple of places where ath_start() may be called (mostly from net80211 when starting the VAP up again), just reimplement it using the new enqueue and taskqueue methods. What I don't like (about this work and the TX code in general): * I'm using the same lock for the staging TX queue management and the actual TX. This isn't required; I'm just being slack. * I haven't yet moved TX to a separate taskqueue (but the taskqueue is created); it's easy enough to do this later if necessary. I just need to make sure it's a higher priority queue, so TX has the same behaviour as it used to (where it would preempt existing RX..) * I need to re-review the TX path a little more and make sure that ieee80211_node_*() functions aren't called within the TX lock. When queueing, I should just push failed frames into a queue and when I'm wrapping up the TX code, unlock the TX lock and call ieee80211_node_free() on each. * It would be nice if I could hold the TX lock for the entire TX and TX completion, rather than this release/re-acquire behaviour. But that requires that I shuffle around the TX completion code to handle actual ath_buf free and net80211 callback/free outside of the TX lock. That's one of my next projects. * the ic_raw_xmit() path doesn't use this yet - so it still has sequencing problems with parallel, overlapping calls to the data path. I'll fix this later. Tested: * Hostap - AR9280, AR9220 * STA - AR5212, AR9280, AR5416 Modified: head/sys/dev/ath/if_ath.c head/sys/dev/ath/if_ath_misc.h head/sys/dev/ath/if_ath_sysctl.c head/sys/dev/ath/if_ath_tx.c head/sys/dev/ath/if_athvar.h Modified: head/sys/dev/ath/if_ath.c ============================================================================== --- head/sys/dev/ath/if_ath.c Tue Jan 15 17:50:07 2013 (r245464) +++ head/sys/dev/ath/if_ath.c Tue Jan 15 18:01:23 2013 (r245465) @@ -152,7 +152,6 @@ static void ath_init(void *); static void ath_stop_locked(struct ifnet *); static void ath_stop(struct ifnet *); static int ath_reset_vap(struct ieee80211vap *, u_long); -static void ath_start_queue(struct ifnet *ifp); static int ath_media_change(struct ifnet *); static void ath_watchdog(void *); static int ath_ioctl(struct ifnet *, u_long, caddr_t); @@ -213,6 +212,14 @@ static void ath_dfs_tasklet(void *, int) static void ath_node_powersave(struct ieee80211_node *, int); static int ath_node_set_tim(struct ieee80211_node *, int); +static int ath_transmit(struct ifnet *ifp, struct mbuf *m); +static void ath_qflush(struct ifnet *ifp); + +static void ath_txq_qinit(struct ifnet *ifp); +static void ath_txq_qflush(struct ifnet *ifp); +static int ath_txq_qadd(struct ifnet *ifp, struct mbuf *m0); +static void ath_txq_qrun(struct ifnet *ifp); + #ifdef IEEE80211_SUPPORT_TDMA #include #endif @@ -429,12 +436,20 @@ ath_attach(u_int16_t devid, struct ath_s taskqueue_start_threads(&sc->sc_tq, 1, PI_NET, "%s taskq", ifp->if_xname); + sc->sc_tx_tq = taskqueue_create("ath_tx_taskq", M_NOWAIT, + taskqueue_thread_enqueue, &sc->sc_tx_tq); + taskqueue_start_threads(&sc->sc_tx_tq, 1, PI_NET, + "%s TX taskq", ifp->if_xname); + TASK_INIT(&sc->sc_rxtask, 0, sc->sc_rx.recv_tasklet, sc); TASK_INIT(&sc->sc_bmisstask, 0, ath_bmiss_proc, sc); TASK_INIT(&sc->sc_bstucktask,0, ath_bstuck_proc, sc); TASK_INIT(&sc->sc_resettask,0, ath_reset_proc, sc); - TASK_INIT(&sc->sc_txqtask,0, ath_txq_sched_tasklet, sc); - TASK_INIT(&sc->sc_fataltask,0, ath_fatal_proc, sc); + TASK_INIT(&sc->sc_txqtask, 0, ath_txq_sched_tasklet, sc); + TASK_INIT(&sc->sc_fataltask, 0, ath_fatal_proc, sc); + + /* XXX make this a higher priority taskqueue? */ + TASK_INIT(&sc->sc_txpkttask, 0, ath_start_task, sc); /* * Allocate hardware transmit queues: one queue for @@ -554,13 +569,18 @@ ath_attach(u_int16_t devid, struct ath_s ifp->if_softc = sc; ifp->if_flags = IFF_SIMPLEX | IFF_BROADCAST | IFF_MULTICAST; - ifp->if_start = ath_start_queue; + /* XXX net80211 uses if_start to re-start ifnet processing */ + ifp->if_start = ath_start; + ifp->if_transmit = ath_transmit; + ifp->if_qflush = ath_qflush; ifp->if_ioctl = ath_ioctl; ifp->if_init = ath_init; IFQ_SET_MAXLEN(&ifp->if_snd, ifqmaxlen); ifp->if_snd.ifq_drv_maxlen = ifqmaxlen; IFQ_SET_READY(&ifp->if_snd); + ath_txq_qinit(ifp); + ic->ic_ifp = ifp; /* XXX not right but it's not used anywhere important */ ic->ic_phytype = IEEE80211_T_OFDM; @@ -966,16 +986,16 @@ ath_detach(struct ath_softc *sc) ath_stop(ifp); ieee80211_ifdetach(ifp->if_l2com); taskqueue_free(sc->sc_tq); + taskqueue_free(sc->sc_tx_tq); #ifdef ATH_TX99_DIAG if (sc->sc_tx99 != NULL) sc->sc_tx99->detach(sc->sc_tx99); #endif ath_rate_detach(sc->sc_rc); - #ifdef ATH_DEBUG_ALQ if_ath_alq_tidyup(&sc->sc_alq); #endif - + ath_txq_qflush(ifp); ath_spectral_detach(sc); ath_dfs_detach(sc); ath_desc_free(sc); @@ -2454,6 +2474,14 @@ ath_buf_clone(struct ath_softc *sc, cons tbf->bf_flags = bf->bf_flags & ~ATH_BUF_BUSY; 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; @@ -2498,13 +2526,70 @@ ath_getbuf(struct ath_softc *sc, ath_buf } static void -ath_start_queue(struct ifnet *ifp) +ath_qflush(struct ifnet *ifp) { - struct ath_softc *sc = ifp->if_softc; - ATH_KTR(sc, ATH_KTR_TX, 0, "ath_start_queue: start"); + /* XXX complete/suspend TX */ + ath_txq_qflush(ifp); + + /* Unsuspend TX? */ +} + +/* + * Transmit a frame from net80211. + */ +static int +ath_transmit(struct ifnet *ifp, struct mbuf *m) +{ + struct ieee80211_node *ni; + struct ath_softc *sc = (struct ath_softc *) ifp->if_softc; + + ni = (struct ieee80211_node *) m->m_pkthdr.rcvif; + + if (ath_txq_qadd(ifp, m) < 0) { + /* + * If queuing fails, the if_transmit() API makes the + * callee responsible for freeing the mbuf (rather than + * the caller, who just assumes the mbuf has been dealt + * with somehow). + * + * BUT, net80211 will free node references if if_transmit() + * fails _on encapsulated buffers_. Since drivers + * only get fully encapsulated frames from net80211 (via + * raw or otherwise APIs), we must be absolutely careful + * to not free the node ref or things will get loopy + * down the track. + * + * For tx fragments, the TX code must free whatever + * new references it created, but NOT the original + * TX node ref that was passed in. + */ + ath_freetx(m); + return (ENOBUFS); + } + + /* + * Unconditionally kick the taskqueue. + * + * Now, there's a subtle race condition possible here if we + * went down the path of only kicking the taskqueue if it + * wasn't running. If we're not absolutely, positively + * careful, we could have a small race window between + * finishing the taskqueue and clearing the TX flag, which + * would be interpreted in _this_ context as "we don't need + * to kick the TX taskqueue, as said taskqueue is already + * running." + * + * It's a problem in some of the 1GE/10GE NIC drivers. + * So until a _correct_ method for implementing this is + * drafted up and written, which avoids (potentially) + * large amounts of locking contention per-frame, let's + * just do the inefficient "kick taskqueue each time" + * method. + */ ath_tx_kick(sc); - ATH_KTR(sc, ATH_KTR_TX, 0, "ath_start_queue: finished"); + + return (0); } void @@ -2531,9 +2616,7 @@ ath_start_task(void *arg, int npending) sc->sc_txstart_cnt++; ATH_PCU_UNLOCK(sc); - ATH_TX_LOCK(sc); - ath_start(sc->sc_ifp); - ATH_TX_UNLOCK(sc); + ath_txq_qrun(ifp); ATH_PCU_LOCK(sc); sc->sc_txstart_cnt--; @@ -2541,91 +2624,298 @@ ath_start_task(void *arg, int npending) ATH_KTR(sc, ATH_KTR_TX, 0, "ath_start_task: finished"); } -void -ath_start(struct ifnet *ifp) +/* + * Pending TX buffer chain management routines. + */ + + +/* + * Initialise the TX queue! + */ +static void +ath_txq_qinit(struct ifnet *ifp) +{ + struct ath_softc *sc = ifp->if_softc; + + TAILQ_INIT(&sc->sc_txbuf_list); +} + +/* + * Add this mbuf to the TX buffer chain. + * + * This allocates an ath_buf, links the mbuf into it, and + * appends it to the end of the TX buffer chain. + * It doesn't fill out the ath_buf in any way besides + * that. + * + * Since the mbuf may be a list of mbufs representing + * 802.11 fragments, handle allocating ath_bufs for each + * of the mbuf fragments. + * + * If we queued it, 0 is returned. Else, < 0 is returned. + * + * If <0 is returned, the sender is responsible for + * freeing the mbuf if appropriate. + */ +static int +ath_txq_qadd(struct ifnet *ifp, struct mbuf *m0) { struct ath_softc *sc = ifp->if_softc; - struct ieee80211_node *ni; struct ath_buf *bf; - struct mbuf *m, *next; ath_bufhead frags; - int npkts = 0; + struct ieee80211_node *ni; + struct mbuf *m; - if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0 || sc->sc_invalid) - return; + /* XXX recursive TX completion -> TX? */ + ATH_TX_UNLOCK_ASSERT(sc); - ATH_TX_LOCK_ASSERT(sc); + /* + * We grab the node pointer, but we don't deref + * the node. The caller must be responsible for + * freeing the node reference if it decides to + * free the mbuf. + */ + ni = (struct ieee80211_node *) m0->m_pkthdr.rcvif; - ATH_KTR(sc, ATH_KTR_TX, 0, "ath_start: called"); + ATH_TXBUF_LOCK(sc); + if (sc->sc_txbuf_cnt <= sc->sc_txq_data_minfree) { + /* XXX increment counter? */ + ATH_TXBUF_UNLOCK(sc); + IF_LOCK(&ifp->if_snd); + ifp->if_drv_flags |= IFF_DRV_OACTIVE; + IF_UNLOCK(&ifp->if_snd); + return (-1); + } + ATH_TXBUF_UNLOCK(sc); - for (;;) { + /* + * Grab a TX buffer and associated resources. + */ + bf = ath_getbuf(sc, ATH_BUFTYPE_NORMAL); + if (bf == NULL) { + device_printf(sc->sc_dev, + "%s: couldn't allocate a buffer\n", + __func__); + return (-1); + } + + /* Setup the initial buffer node contents */ + bf->bf_m = m0; + bf->bf_node = ni; + + /* + * Check for fragmentation. If this frame + * has been broken up verify we have enough + * buffers to send all the fragments so all + * go out or none... + */ + TAILQ_INIT(&frags); + if (m0->m_flags & M_FRAG) + DPRINTF(sc, ATH_DEBUG_XMIT, "%s: txfrag\n", __func__); + if ((m0->m_flags & M_FRAG) && + !ath_txfrag_setup(sc, &frags, m0, ni)) { + DPRINTF(sc, ATH_DEBUG_XMIT, + "%s: out of txfrag buffers\n", __func__); + sc->sc_stats.ast_tx_nofrag++; + ifp->if_oerrors++; + goto bad; + } + + /* + * Don't stuff the non-fragment frame onto the fragment + * queue. ath_txfrag_cleanup() should only be called on fragments - + * ie, the _extra_ ieee80211_node references - and not the single + * node reference already done as part of the net08211 TX call + * into the driver. + */ + + ATH_TX_LOCK(sc); + + /* + * Throw the single frame onto the queue. + */ + TAILQ_INSERT_TAIL(&sc->sc_txbuf_list, bf, bf_list); + + /* + * Update next packet duration length if it's a fragment. + * It's needed for accurate NAV calculations (which for + * fragments include the length of the NEXT fragment.) + */ + if (m0->m_nextpkt != NULL) + bf->bf_state.bfs_nextpktlen = + m0->m_nextpkt->m_pkthdr.len; + + /* + * Append the fragments. We have to populate bf and node + * references here as although the txfrag setup code does + * create buffers and increment the node ref, it doesn't + * populate the fields for us. + */ + m = m0->m_nextpkt; + while ( (bf = TAILQ_FIRST(&frags)) != NULL) { + bf->bf_m = m; + bf->bf_node = ni; + device_printf(sc->sc_dev, "%s: adding bf=%p, m=%p, ni=%p\n", + __func__, + bf, + bf->bf_m, + bf->bf_node); + TAILQ_REMOVE(&frags, bf, bf_list); + TAILQ_INSERT_TAIL(&sc->sc_txbuf_list, bf, bf_list); + + /* + * For duration (NAV) calculations, we need + * to know the next fragment size. + * + * XXX This isn't entirely accurate as it doesn't + * take pad bytes and such into account, but it'll do + * for fragment length / NAV calculations. + */ + if (m->m_nextpkt != NULL) + bf->bf_state.bfs_nextpktlen = + m->m_nextpkt->m_pkthdr.len; + + m = m->m_nextpkt; + } + ATH_TX_UNLOCK(sc); + + return (0); +bad: + device_printf(sc->sc_dev, "%s: bad?!\n", __func__); + bf->bf_m = NULL; + bf->bf_node = NULL; + ATH_TXBUF_LOCK(sc); + ath_returnbuf_head(sc, bf); + ath_txfrag_cleanup(sc, &frags, ni); + ATH_TXBUF_UNLOCK(sc); + return (-1); +} + +/* + * Flush the pending TX buffer chain. + */ +static void +ath_txq_qflush(struct ifnet *ifp) +{ + struct ath_softc *sc = ifp->if_softc; + ath_bufhead txlist; + struct ath_buf *bf; + + device_printf(sc->sc_dev, "%s: called\n", __func__); + TAILQ_INIT(&txlist); + + /* Grab lock */ + ATH_TX_LOCK(sc); + + /* Copy everything out of sc_txbuf_list into txlist */ + TAILQ_CONCAT(&txlist, &sc->sc_txbuf_list, bf_list); + + /* Unlock */ + ATH_TX_UNLOCK(sc); + + /* Now, walk the list, freeing things */ + while ((bf = TAILQ_FIRST(&txlist)) != NULL) { + TAILQ_REMOVE(&txlist, bf, bf_list); + + if (bf->bf_node) + ieee80211_free_node(bf->bf_node); + + m_free(bf->bf_m); + + /* XXX paranoia! */ + bf->bf_m = NULL; + bf->bf_node = NULL; + + /* + * XXX Perhaps do a second pass with the TXBUF lock + * held and free them all at once? + */ ATH_TXBUF_LOCK(sc); - if (sc->sc_txbuf_cnt <= sc->sc_txq_data_minfree) { - /* XXX increment counter? */ - ATH_TXBUF_UNLOCK(sc); - IF_LOCK(&ifp->if_snd); - ifp->if_drv_flags |= IFF_DRV_OACTIVE; - IF_UNLOCK(&ifp->if_snd); - break; - } + ath_returnbuf_head(sc, bf); ATH_TXBUF_UNLOCK(sc); - + } +} + +/* + * Walk the TX buffer queue and call ath_tx_start() on each + * of them. + */ +static void +ath_txq_qrun(struct ifnet *ifp) +{ + struct ath_softc *sc = ifp->if_softc; + ath_bufhead txlist; + struct ath_buf *bf, *bf_next; + struct ieee80211_node *ni; + struct mbuf *m; + + if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0 || sc->sc_invalid) + return; + + TAILQ_INIT(&txlist); + + /* + * Grab the frames to transmit from the tx queue + */ + + /* Copy everything out of sc_txbuf_list into txlist */ + ATH_TX_LOCK(sc); + TAILQ_CONCAT(&txlist, &sc->sc_txbuf_list, bf_list); + ATH_TX_UNLOCK(sc); + + /* + * For now, the ath_tx_start() code sits behind the same lock; + * worry about serialising this in a taskqueue later. + */ + + ATH_TX_LOCK(sc); + + /* + * Attempt to transmit each frame. + * + * In the old code path - if a TX fragment fails, subsequent + * fragments in that group would be aborted. + * + * It would be nice to chain together TX fragments in this + * way so they can be aborted together. + */ + TAILQ_FOREACH_SAFE(bf, &txlist, bf_list, bf_next) { /* - * Grab a TX buffer and associated resources. + * Clear, because we're going to reuse this + * as a real ath_buf now */ - bf = ath_getbuf(sc, ATH_BUFTYPE_NORMAL); - if (bf == NULL) - break; + ni = bf->bf_node; + m = bf->bf_m; + + bf->bf_node = NULL; + bf->bf_m = NULL; - IFQ_DEQUEUE(&ifp->if_snd, m); - if (m == NULL) { - ATH_TXBUF_LOCK(sc); - ath_returnbuf_head(sc, bf); - ATH_TXBUF_UNLOCK(sc); - break; - } - ni = (struct ieee80211_node *) m->m_pkthdr.rcvif; - npkts ++; /* - * Check for fragmentation. If this frame - * has been broken up verify we have enough - * buffers to send all the fragments so all - * go out or none... - */ - TAILQ_INIT(&frags); - if ((m->m_flags & M_FRAG) && - !ath_txfrag_setup(sc, &frags, m, ni)) { - DPRINTF(sc, ATH_DEBUG_XMIT, - "%s: out of txfrag buffers\n", __func__); - sc->sc_stats.ast_tx_nofrag++; - ifp->if_oerrors++; - ath_freetx(m); - goto bad; - } - ifp->if_opackets++; - nextfrag: + * Remove it from the list. + */ + TAILQ_REMOVE(&txlist, bf, bf_list); + /* - * Pass the frame to the h/w for transmission. - * Fragmented frames have each frag chained together - * with m_nextpkt. We know there are sufficient ath_buf's - * to send all the frags because of work done by - * ath_txfrag_setup. We leave m_nextpkt set while - * calling ath_tx_start so it can use it to extend the - * the tx duration to cover the subsequent frag and - * so it can reclaim all the mbufs in case of an error; - * ath_tx_start clears m_nextpkt once it commits to - * handing the frame to the hardware. + * If we fail, free this buffer and go to the next one; + * ath_tx_start() frees the mbuf but not the node + * reference. */ - next = m->m_nextpkt; if (ath_tx_start(sc, ni, bf, m)) { - bad: + /* + * XXX m is freed by ath_tx_start(); node reference + * is not! + */ + DPRINTF(sc, ATH_DEBUG_XMIT, + "%s: failed; bf=%p, ni=%p, m=%p\n", + __func__, + bf, + ni, + m); ifp->if_oerrors++; - reclaim: bf->bf_m = NULL; bf->bf_node = NULL; ATH_TXBUF_LOCK(sc); ath_returnbuf_head(sc, bf); - ath_txfrag_cleanup(sc, &frags, ni); ATH_TXBUF_UNLOCK(sc); /* * XXX todo, free the node outside of @@ -2633,37 +2923,84 @@ ath_start(struct ifnet *ifp) */ if (ni != NULL) ieee80211_free_node(ni); - continue; + } else { + /* + * Check here if the node is in power save state. + * XXX we should hold a node ref here, and release + * it after the TX has completed. + */ + ath_tx_update_tim(sc, ni, 1); + ifp->if_opackets++; } /* - * Check here if the node is in power save state. + * XXX should check for state change and flip out + * if needed. */ - ath_tx_update_tim(sc, ni, 1); + } + ATH_TX_UNLOCK(sc); - if (next != NULL) { - /* - * Beware of state changing between frags. - * XXX check sta power-save state? - */ - if (ni->ni_vap->iv_state != IEEE80211_S_RUN) { - DPRINTF(sc, ATH_DEBUG_XMIT, - "%s: flush fragmented packet, state %s\n", - __func__, - ieee80211_state_name[ni->ni_vap->iv_state]); - ath_freetx(next); - goto reclaim; - } - m = next; - bf = TAILQ_FIRST(&frags); - KASSERT(bf != NULL, ("no buf for txfrag")); - TAILQ_REMOVE(&frags, bf, bf_list); - goto nextfrag; + /* + * If we break out early (eg a state change) we should prepend these + * frames onto the TX queue. + */ +} + +/* + * This is now primarily used by the net80211 layer to kick-start + * queue processing. + */ +void +ath_start(struct ifnet *ifp) +{ + struct mbuf *m; + struct ath_softc *sc = ifp->if_softc; + struct ieee80211_node *ni; + int npkts = 0; + + ATH_TX_UNLOCK_ASSERT(sc); + + if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0 || sc->sc_invalid) + return; + + /* + * If we're below the free buffer limit, don't dequeue anything. + * The original code would not dequeue anything from the queue + * if allocating an ath_buf failed. + * + * For if_transmit, we have to either queue or drop the frame. + * So we have to try and queue it _somewhere_. + */ + for (;;) { + IFQ_DEQUEUE(&ifp->if_snd, m); + if (m == NULL) { + break; } - sc->sc_wd_timer = 5; + /* + * If we do fail here, just break out for now + * and wait until we've transmitted something + * before we attempt again? + */ + if (ath_txq_qadd(ifp, m) < 0) { + DPRINTF(sc, ATH_DEBUG_XMIT, + "%s: ath_txq_qadd failed\n", + __func__); + ni = (struct ieee80211_node *) m->m_pkthdr.rcvif; + if (ni != NULL) + ieee80211_free_node(ni); + ath_freetx(m); + break; + } + npkts++; } - ATH_KTR(sc, ATH_KTR_TX, 1, "ath_start: finished; npkts=%d", npkts); + + /* + * Kick the taskqueue into activity, but only if we + * queued something. + */ + if (npkts > 0) + ath_tx_kick(sc); } static int Modified: head/sys/dev/ath/if_ath_misc.h ============================================================================== --- head/sys/dev/ath/if_ath_misc.h Tue Jan 15 17:50:07 2013 (r245464) +++ head/sys/dev/ath/if_ath_misc.h Tue Jan 15 18:01:23 2013 (r245465) @@ -124,9 +124,8 @@ static inline void ath_tx_kick(struct ath_softc *sc) { - ATH_TX_LOCK(sc); - ath_start(sc->sc_ifp); - ATH_TX_UNLOCK(sc); + /* XXX eventually try sc_tx_tq? */ + taskqueue_enqueue(sc->sc_tq, &sc->sc_txpkttask); } #endif Modified: head/sys/dev/ath/if_ath_sysctl.c ============================================================================== --- head/sys/dev/ath/if_ath_sysctl.c Tue Jan 15 17:50:07 2013 (r245464) +++ head/sys/dev/ath/if_ath_sysctl.c Tue Jan 15 18:01:23 2013 (r245465) @@ -505,6 +505,33 @@ ath_sysctl_forcebstuck(SYSCTL_HANDLER_AR return 0; } +static int +ath_sysctl_hangcheck(SYSCTL_HANDLER_ARGS) +{ + struct ath_softc *sc = arg1; + int val = 0; + int error; + uint32_t mask = 0xffffffff; + uint32_t *sp; + uint32_t rsize; + struct ath_hal *ah = sc->sc_ah; + + error = sysctl_handle_int(oidp, &val, 0, req); + if (error || !req->newptr) + return error; + if (val == 0) + return 0; + + /* Do a hang check */ + if (!ath_hal_getdiagstate(ah, HAL_DIAG_CHECK_HANGS, + &mask, sizeof(mask), + (void *) &sp, &rsize)) + return (0); + device_printf(sc->sc_dev, "%s: sp=0x%08x\n", __func__, *sp); + + val = 0; + return 0; +} #ifdef ATH_DEBUG_ALQ static int @@ -661,6 +688,10 @@ ath_sysctlattach(struct ath_softc *sc) "forcebstuck", CTLTYPE_INT | CTLFLAG_RW, sc, 0, ath_sysctl_forcebstuck, "I", ""); + SYSCTL_ADD_PROC(ctx, SYSCTL_CHILDREN(tree), OID_AUTO, + "hangcheck", CTLTYPE_INT | CTLFLAG_RW, sc, 0, + ath_sysctl_hangcheck, "I", ""); + if (ath_hal_hasintmit(ah)) { SYSCTL_ADD_PROC(ctx, SYSCTL_CHILDREN(tree), OID_AUTO, "intmit", CTLTYPE_INT | CTLFLAG_RW, sc, 0, Modified: head/sys/dev/ath/if_ath_tx.c ============================================================================== --- head/sys/dev/ath/if_ath_tx.c Tue Jan 15 17:50:07 2013 (r245464) +++ head/sys/dev/ath/if_ath_tx.c Tue Jan 15 18:01:23 2013 (r245465) @@ -1124,7 +1124,11 @@ ath_tx_calc_duration(struct ath_softc *s dur = rt->info[rix].lpAckDuration; if (wh->i_fc[1] & IEEE80211_FC1_MORE_FRAG) { dur += dur; /* additional SIFS+ACK */ - KASSERT(bf->bf_m->m_nextpkt != NULL, ("no fragment")); + if (bf->bf_state.bfs_nextpktlen == 0) { + device_printf(sc->sc_dev, + "%s: next txfrag len=0?\n", + __func__); + } /* * Include the size of next fragment so NAV is * updated properly. The last fragment uses only @@ -1135,7 +1139,7 @@ ath_tx_calc_duration(struct ath_softc *s * first fragment! */ dur += ath_hal_computetxtime(ah, rt, - bf->bf_m->m_nextpkt->m_pkthdr.len, + bf->bf_state.bfs_nextpktlen, rix, shortPreamble); } if (isfrag) { Modified: head/sys/dev/ath/if_athvar.h ============================================================================== --- head/sys/dev/ath/if_athvar.h Tue Jan 15 17:50:07 2013 (r245464) +++ head/sys/dev/ath/if_athvar.h Tue Jan 15 18:01:23 2013 (r245465) @@ -279,6 +279,8 @@ struct ath_buf { int32_t bfs_keyix; /* crypto key index */ int32_t bfs_txantenna; /* TX antenna config */ + uint16_t bfs_nextpktlen; /* length of next frag pkt */ + /* Make this an 8 bit value? */ enum ieee80211_protmode bfs_protmode; @@ -494,6 +496,13 @@ struct ath_softc { struct ath_tx_methods sc_tx; struct ath_tx_edma_fifo sc_txedma[HAL_NUM_TX_QUEUES]; + /* + * This is (currently) protected by the TX queue lock; + * it should migrate to a separate lock later + * so as to minimise contention. + */ + ath_bufhead sc_txbuf_list; + int sc_rx_statuslen; int sc_tx_desclen; int sc_tx_statuslen; @@ -514,6 +523,7 @@ struct ath_softc { struct mtx sc_tx_mtx; /* TX access mutex */ char sc_tx_mtx_name[32]; struct taskqueue *sc_tq; /* private task queue */ + struct taskqueue *sc_tx_tq; /* private TX task queue */ struct ath_hal *sc_ah; /* Atheros HAL */ struct ath_ratectrl *sc_rc; /* tx rate control support */ struct ath_tx99 *sc_tx99; /* tx99 adjunct state */ @@ -660,6 +670,7 @@ struct ath_softc { struct ath_txq *sc_ac2q[5]; /* WME AC -> h/w q map */ struct task sc_txtask; /* tx int processing */ struct task sc_txqtask; /* tx proc processing */ + struct task sc_txpkttask; /* tx frame processing */ struct ath_descdma sc_txcompdma; /* TX EDMA completion */ struct mtx sc_txcomplock; /* TX EDMA completion lock */