Author: adrian
Date: Tue Nov  8 18:10:04 2011
New Revision: 227346
URL: http://svn.freebsd.org/changeset/base/227346

Log:
  Merge in some fixes from the if_ath_tx branch.
  
  * Close down some of the kickpcu races, where the interrupt handler
    can and will run concurrently with the taskqueue.
  * Close down the TXQ active/completed race between the interrupt
    handler and the concurrently running tx completion taskqueue
    function.
  * Add some tx and rx interrupt count tracking, for debugging.
  * Fix the kickpcu logic in ath_rx_proc() to not simply drain and
    restart the TX queue - instead, assume the hardware isn't
    (too) confused and just restart RX DMA. This may break on
    previous chipsets, so if it does I'll add a HAL flag and
    conditionally handle this (ie, for broken chipsets, I'll
    just restore the "stop PCU / flush things / restart PCU"
    logic.)
  * Misc stuff
  
  Sponsored by: Hobnob, Inc.

Modified:
  head/sys/dev/ath/if_ath.c
  head/sys/dev/ath/if_ath_tx.h
  head/sys/dev/ath/if_athvar.h

Modified: head/sys/dev/ath/if_ath.c
==============================================================================
--- head/sys/dev/ath/if_ath.c   Tue Nov  8 17:23:43 2011        (r227345)
+++ head/sys/dev/ath/if_ath.c   Tue Nov  8 18:10:04 2011        (r227346)
@@ -1304,6 +1304,7 @@ ath_intr(void *arg)
        struct ifnet *ifp = sc->sc_ifp;
        struct ath_hal *ah = sc->sc_ah;
        HAL_INT status = 0;
+       uint32_t txqs;
 
        if (sc->sc_invalid) {
                /*
@@ -1377,7 +1378,7 @@ ath_intr(void *arg)
                        }
                }
                if (status & HAL_INT_RXEOL) {
-                       int imask = sc->sc_imask;
+                       int imask;
                        /*
                         * NB: the hardware should re-read the link when
                         *     RXE bit is written, but it doesn't work at
@@ -1393,26 +1394,55 @@ ath_intr(void *arg)
                         * by a call to ath_reset() somehow, the
                         * interrupt mask will be correctly reprogrammed.
                         */
+                       ATH_LOCK(sc);
+                       imask = sc->sc_imask;
                        imask &= ~(HAL_INT_RXEOL | HAL_INT_RXORN);
                        ath_hal_intrset(ah, imask);
                        /*
+                        * Only blank sc_rxlink if we've not yet kicked
+                        * the PCU.
+                        *
+                        * This isn't entirely correct - the correct solution
+                        * would be to have a PCU lock and engage that for
+                        * the duration of the PCU fiddling; which would include
+                        * running the RX process. Otherwise we could end up
+                        * messing up the RX descriptor chain and making the
+                        * RX desc list much shorter.
+                        */
+                       if (! sc->sc_kickpcu)
+                               sc->sc_rxlink = NULL;
+                       sc->sc_kickpcu = 1;
+                       ATH_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);
-                       sc->sc_rxlink = NULL;
-                       sc->sc_kickpcu = 1;
                }
                if (status & HAL_INT_TXURN) {
                        sc->sc_stats.ast_txurn++;
                        /* bump tx trigger level */
                        ath_hal_updatetxtriglevel(ah, AH_TRUE);
                }
-               if (status & HAL_INT_RX)
+               if (status & HAL_INT_RX) {
+                       sc->sc_stats.ast_rx_intr++;
                        taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask);
-               if (status & HAL_INT_TX)
+               }
+               if (status & HAL_INT_TX) {
+                       sc->sc_stats.ast_tx_intr++;
+                       /*
+                        * Grab all the currently set bits in the HAL txq bitmap
+                        * and blank them. This is the only place we should be
+                        * doing this.
+                        */
+                       ATH_LOCK(sc);
+                       txqs = 0xffffffff;
+                       ath_hal_gettxintrtxqs(sc->sc_ah, &txqs);
+                       sc->sc_txq_active |= txqs;
+                       ATH_UNLOCK(sc);
                        taskqueue_enqueue(sc->sc_tq, &sc->sc_txtask);
+               }
                if (status & HAL_INT_BMISS) {
                        sc->sc_stats.ast_bmiss++;
                        taskqueue_enqueue(sc->sc_tq, &sc->sc_bmisstask);
@@ -1433,7 +1463,16 @@ ath_intr(void *arg)
                         * clear whatever condition caused the interrupt.
                         */
                        ath_hal_mibevent(ah, &sc->sc_halstats);
-                       ath_hal_intrset(ah, sc->sc_imask);
+                       /*
+                        * Don't reset the interrupt if we've just
+                        * kicked the PCU, or we may get a nested
+                        * RXEOL before the rxproc has had a chance
+                        * to run.
+                        */
+                       ATH_LOCK(sc);
+                       if (sc->sc_kickpcu == 0)
+                               ath_hal_intrset(ah, sc->sc_imask);
+                       ATH_UNLOCK(sc);
                }
                if (status & HAL_INT_RXORN) {
                        /* NB: hal marks HAL_INT_FATAL when RXORN is fatal */
@@ -1609,6 +1648,13 @@ ath_init(void *arg)
        sc->sc_beacons = 0;
 
        /*
+        * Initial aggregation settings.
+        */
+       sc->sc_hwq_limit = ATH_AGGR_MIN_QDEPTH;
+       sc->sc_tid_hwq_lo = ATH_AGGR_SCHED_LOW;
+       sc->sc_tid_hwq_hi = ATH_AGGR_SCHED_HIGH;
+
+       /*
         * Setup the hardware after reset: the key cache
         * is filled as needed and the receive engine is
         * set going.  Frame transmit is handled entirely
@@ -3478,6 +3524,7 @@ ath_rx_proc(void *arg, int npending)
        HAL_STATUS status;
        int16_t nf;
        u_int64_t tsf;
+       int npkts = 0;
 
        DPRINTF(sc, ATH_DEBUG_RX_PROC, "%s: pending %u\n", __func__, npending);
        ngood = 0;
@@ -3537,6 +3584,7 @@ ath_rx_proc(void *arg, int npending)
                        break;
 
                TAILQ_REMOVE(&sc->sc_rxbuf, bf, bf_list);
+               npkts++;
 
                /* These aren't specifically errors */
                if (rs->rs_flags & HAL_RX_GI)
@@ -3823,17 +3871,20 @@ rx_next:
         * been an RXEOL condition.
         */
        if (sc->sc_kickpcu) {
-               sc->sc_kickpcu = 0;
-               ath_stoprecv(sc);
-               sc->sc_imask |= (HAL_INT_RXEOL | HAL_INT_RXORN);
-               if (ath_startrecv(sc) != 0) {
-                       if_printf(ifp,
-                           "%s: couldn't restart RX after RXEOL; resetting\n",
-                           __func__);
-                       ath_reset(ifp);
-                       return;
-               }
+               device_printf(sc->sc_dev, "%s: kickpcu; handled %d packets\n",
+                   __func__, npkts);
+
+               /* XXX rxslink? */
+               bf = TAILQ_FIRST(&sc->sc_rxbuf);
+               ath_hal_putrxbuf(ah, bf->bf_daddr);
+               ath_hal_rxena(ah);              /* enable recv descriptors */
+               ath_mode_init(sc);              /* set filters, etc. */
+               ath_hal_startpcurecv(ah);       /* re-enable PCU/DMA engine */
+
+               ATH_LOCK(sc);
                ath_hal_intrset(ah, sc->sc_imask);
+               sc->sc_kickpcu = 0;
+               ATH_UNLOCK(sc);
        }
 
        if ((ifp->if_drv_flags & IFF_DRV_OACTIVE) == 0) {
@@ -4218,13 +4269,7 @@ ath_tx_processq(struct ath_softc *sc, st
        return nacked;
 }
 
-static __inline int
-txqactive(struct ath_hal *ah, int qnum)
-{
-       u_int32_t txqs = 1<<qnum;
-       ath_hal_gettxintrtxqs(ah, &txqs);
-       return (txqs & (1<<qnum));
-}
+#define        TXQACTIVE(t, q)         ( (t) & (1 << (q)))
 
 /*
  * Deferred processing of transmit interrupt; special-cased
@@ -4235,10 +4280,17 @@ ath_tx_proc_q0(void *arg, int npending)
 {
        struct ath_softc *sc = arg;
        struct ifnet *ifp = sc->sc_ifp;
+       uint32_t txqs;
 
-       if (txqactive(sc->sc_ah, 0) && ath_tx_processq(sc, &sc->sc_txq[0]))
+       ATH_LOCK(sc);
+       txqs = sc->sc_txq_active;
+       sc->sc_txq_active &= ~txqs;
+       ATH_UNLOCK(sc);
+
+       if (TXQACTIVE(txqs, 0) && ath_tx_processq(sc, &sc->sc_txq[0]))
+               /* XXX why is lastrx updated in tx code? */
                sc->sc_lastrx = ath_hal_gettsf64(sc->sc_ah);
-       if (txqactive(sc->sc_ah, sc->sc_cabq->axq_qnum))
+       if (TXQACTIVE(txqs, sc->sc_cabq->axq_qnum))
                ath_tx_processq(sc, sc->sc_cabq);
        ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
        sc->sc_wd_timer = 0;
@@ -4259,20 +4311,26 @@ ath_tx_proc_q0123(void *arg, int npendin
        struct ath_softc *sc = arg;
        struct ifnet *ifp = sc->sc_ifp;
        int nacked;
+       uint32_t txqs;
+
+       ATH_LOCK(sc);
+       txqs = sc->sc_txq_active;
+       sc->sc_txq_active &= ~txqs;
+       ATH_UNLOCK(sc);
 
        /*
         * Process each active queue.
         */
        nacked = 0;
-       if (txqactive(sc->sc_ah, 0))
+       if (TXQACTIVE(txqs, 0))
                nacked += ath_tx_processq(sc, &sc->sc_txq[0]);
-       if (txqactive(sc->sc_ah, 1))
+       if (TXQACTIVE(txqs, 1))
                nacked += ath_tx_processq(sc, &sc->sc_txq[1]);
-       if (txqactive(sc->sc_ah, 2))
+       if (TXQACTIVE(txqs, 2))
                nacked += ath_tx_processq(sc, &sc->sc_txq[2]);
-       if (txqactive(sc->sc_ah, 3))
+       if (TXQACTIVE(txqs, 3))
                nacked += ath_tx_processq(sc, &sc->sc_txq[3]);
-       if (txqactive(sc->sc_ah, sc->sc_cabq->axq_qnum))
+       if (TXQACTIVE(txqs, sc->sc_cabq->axq_qnum))
                ath_tx_processq(sc, sc->sc_cabq);
        if (nacked)
                sc->sc_lastrx = ath_hal_gettsf64(sc->sc_ah);
@@ -4295,13 +4353,19 @@ ath_tx_proc(void *arg, int npending)
        struct ath_softc *sc = arg;
        struct ifnet *ifp = sc->sc_ifp;
        int i, nacked;
+       uint32_t txqs;
+
+       ATH_LOCK(sc);
+       txqs = sc->sc_txq_active;
+       sc->sc_txq_active &= ~txqs;
+       ATH_UNLOCK(sc);
 
        /*
         * Process each active queue.
         */
        nacked = 0;
        for (i = 0; i < HAL_NUM_TX_QUEUES; i++)
-               if (ATH_TXQ_SETUP(sc, i) && txqactive(sc->sc_ah, i))
+               if (ATH_TXQ_SETUP(sc, i) && TXQACTIVE(txqs, i))
                        nacked += ath_tx_processq(sc, &sc->sc_txq[i]);
        if (nacked)
                sc->sc_lastrx = ath_hal_gettsf64(sc->sc_ah);

Modified: head/sys/dev/ath/if_ath_tx.h
==============================================================================
--- head/sys/dev/ath/if_ath_tx.h        Tue Nov  8 17:23:43 2011        
(r227345)
+++ head/sys/dev/ath/if_ath_tx.h        Tue Nov  8 18:10:04 2011        
(r227346)
@@ -31,6 +31,24 @@
 #ifndef        __IF_ATH_TX_H__
 #define        __IF_ATH_TX_H__
 
+/*
+ * How 'busy' to try and keep the hardware txq
+ */
+#define        ATH_AGGR_MIN_QDEPTH             2
+
+/*
+ * Watermark for scheduling TIDs in order to maximise aggregation.
+ *
+ * If hwq_depth is greater than this, don't schedule the TID
+ * for packet scheduling - the hardware is already busy servicing
+ * this TID.
+ *
+ * If hwq_depth is less than this, schedule the TID for packet
+ * scheduling in the completion handler.
+ */
+#define        ATH_AGGR_SCHED_HIGH             4
+#define        ATH_AGGR_SCHED_LOW              2
+
 extern void ath_freetx(struct mbuf *m);
 extern void ath_txfrag_cleanup(struct ath_softc *sc, ath_bufhead *frags,
     struct ieee80211_node *ni);

Modified: head/sys/dev/ath/if_athvar.h
==============================================================================
--- head/sys/dev/ath/if_athvar.h        Tue Nov  8 17:23:43 2011        
(r227345)
+++ head/sys/dev/ath/if_athvar.h        Tue Nov  8 18:10:04 2011        
(r227346)
@@ -312,7 +312,6 @@ struct ath_txq {
        TAILQ_REMOVE(&(_tq)->axq_q, _elm, _field); \
        (_tq)->axq_depth--; \
 } while (0)
-/* NB: this does not do the "head empty check" that STAILQ_LAST does */
 #define        ATH_TXQ_LAST(_tq, _field)       TAILQ_LAST(&(_tq)->axq_q, 
_field)
 
 struct ath_vap {
@@ -397,7 +396,6 @@ struct ath_softc {
                                sc_setcca   : 1,/* set/clr CCA with TDMA */
                                sc_resetcal : 1,/* reset cal state next trip */
                                sc_rxslink  : 1,/* do self-linked final 
descriptor */
-                               sc_kickpcu  : 1,/* kick PCU RX on next RX proc 
*/
                                sc_rxtsf32  : 1;/* RX dec TSF is 32 bits */
        uint32_t                sc_eerd;        /* regdomain from EEPROM */
        uint32_t                sc_eecc;        /* country code from EEPROM */
@@ -424,7 +422,19 @@ struct ath_softc {
        u_int                   sc_fftxqmin;    /* min frames before staging */
        u_int                   sc_fftxqmax;    /* max frames before drop */
        u_int                   sc_txantenna;   /* tx antenna (fixed or auto) */
+
        HAL_INT                 sc_imask;       /* interrupt mask copy */
+       /*
+        * These are modified in the interrupt handler as well as
+        * the task queues and other contexts. Thus these must be
+        * protected by a mutex, or they could clash.
+        *
+        * For now, access to these is behind the ATH_LOCK,
+        * just to save time.
+        */
+       uint32_t                sc_txq_active;  /* bitmap of active TXQs */
+       uint32_t                sc_kickpcu;     /* whether to kick the PCU */
+
        u_int                   sc_keymax;      /* size of key cache */
        u_int8_t                sc_keymap[ATH_KEYBYTES];/* key use bit map */
 
_______________________________________________
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"

Reply via email to