Date: Tue, 1 Aug 2006 17:25:04 GMT From: Paolo Pisati <piso@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 102954 for review Message-ID: <200608011725.k71HP4ol019342@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=102954 Change 102954 by piso@piso_newluxor on 2006/08/01 17:24:16 Convert ath to use a filter+ithread handler: use a spinlock (inside softc) to guard against races when accessing sc_status or the interrupt registers, and axe all the taskqueue jobs from ath_intr(). Affected files ... .. //depot/projects/soc2006/intr_filter/dev/ath/if_ath.c#3 edit .. //depot/projects/soc2006/intr_filter/dev/ath/if_ath_pci.c#3 edit .. //depot/projects/soc2006/intr_filter/dev/ath/if_athvar.h#3 edit Differences ... ==== //depot/projects/soc2006/intr_filter/dev/ath/if_ath.c#3 (text+ko) ==== @@ -115,8 +115,8 @@ static void ath_watchdog(struct ifnet *); static int ath_ioctl(struct ifnet *, u_long, caddr_t); static void ath_fatal_proc(void *, int); -static void ath_rxorn_proc(void *, int); -static void ath_bmiss_proc(void *, int); +static void ath_rxorn_proc(void *); +static void ath_bmiss_proc(void *); static void ath_radar_proc(void *, int); static int ath_key_alloc(struct ieee80211com *, const struct ieee80211_key *, @@ -149,7 +149,7 @@ struct ieee80211_node *ni, int subtype, int rssi, u_int32_t rstamp); static void ath_setdefantenna(struct ath_softc *, u_int); -static void ath_rx_proc(void *, int); +static void ath_rx_proc(void *); static void ath_txq_init(struct ath_softc *sc, struct ath_txq *, int); static struct ath_txq *ath_txq_setup(struct ath_softc*, int qtype, int subtype); static int ath_tx_setup(struct ath_softc *, int, int); @@ -158,9 +158,9 @@ static void ath_tx_cleanup(struct ath_softc *); static int ath_tx_start(struct ath_softc *, struct ieee80211_node *, struct ath_buf *, struct mbuf *); -static void ath_tx_proc_q0(void *, int); -static void ath_tx_proc_q0123(void *, int); -static void ath_tx_proc(void *, int); +static void ath_tx_proc_q0(void *); +static void ath_tx_proc_q0123(void *); +static void ath_tx_proc(void *); static int ath_chan_set(struct ath_softc *, struct ieee80211_channel *); static void ath_draintxq(struct ath_softc *); static void ath_stoprecv(struct ath_softc *); @@ -396,10 +396,7 @@ taskqueue_thread_enqueue, &sc->sc_tq); taskqueue_start_threads(&sc->sc_tq, 1, PI_NET, "%s taskq", ifp->if_xname); - - TASK_INIT(&sc->sc_rxtask, 0, ath_rx_proc, sc); - TASK_INIT(&sc->sc_rxorntask, 0, ath_rxorn_proc, 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_radartask, 0, ath_radar_proc, sc); @@ -457,13 +454,13 @@ */ switch (sc->sc_txqsetup &~ (1<<sc->sc_cabq->axq_qnum)) { case 0x01: - TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc_q0, sc); + sc->tx_func = &ath_tx_proc_q0; break; case 0x0f: - TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc_q0123, sc); + sc->tx_func = &ath_tx_proc_q0123; break; default: - TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc, sc); + sc->tx_func = &ath_tx_proc; break; } @@ -711,16 +708,13 @@ ath_stop(ifp); } -/* - * Interrupt handler. Most of the actual processing is deferred. - */ -void -ath_intr(void *arg) +int +ath_filter(void *arg) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; struct ath_hal *ah = sc->sc_ah; - HAL_INT status; + int ret = FILTER_HANDLED; if (sc->sc_invalid) { /* @@ -728,17 +722,19 @@ * Note this can happen early on if the IRQ is shared. */ DPRINTF(sc, ATH_DEBUG_ANY, "%s: invalid; ignored\n", __func__); - return; + return (FILTER_STRAY); } + mtx_lock_spin(&sc->sc_smtx); if (!ath_hal_intrpend(ah)) /* shared irq, not for us */ - return; + return (FILTER_STRAY); if (!((ifp->if_flags & IFF_UP) && (ifp->if_drv_flags & - IFF_DRV_RUNNING))) { + IFF_DRV_RUNNING))) { DPRINTF(sc, ATH_DEBUG_ANY, "%s: if_flags 0x%x\n", __func__, ifp->if_flags); - ath_hal_getisr(ah, &status); /* clear ISR */ + ath_hal_getisr(ah, &sc->sc_status); /* clear ISR */ ath_hal_intrset(ah, 0); /* disable further intr's */ - return; + mtx_unlock_spin(&sc->sc_smtx); + return (ret); } /* * Figure out the reason(s) for the interrupt. Note @@ -746,9 +742,40 @@ * bits we haven't explicitly enabled so we mask the * value to insure we only process bits we requested. */ - ath_hal_getisr(ah, &status); /* NB: clears ISR too */ - DPRINTF(sc, ATH_DEBUG_INTR, "%s: status 0x%x\n", __func__, status); - status &= sc->sc_imask; /* discard unasked for bits */ + ath_hal_getisr(ah, &sc->sc_status); /* NB: clears ISR too */ + DPRINTF(sc, ATH_DEBUG_INTR, "%s: status 0x%x\n", __func__, sc->sc_status); + sc->sc_status &= sc->sc_imask; /* discard unasked for bits */ + if (sc->sc_status & HAL_INT_RXEOL) { + /* + * NB: the hardware should re-read the link when + * RXE bit is written, but it doesn't work at + * least on older hardware revs. + */ + sc->sc_stats.ast_rxeol++; + sc->sc_rxlink = NULL; + sc->sc_status &= ~HAL_INT_RXEOL; + } + + if (sc->sc_status) { + ath_hal_intrset(ah, 0); + ret |= FILTER_SCHEDULE_THREAD; + } + mtx_unlock_spin(&sc->sc_smtx); + return (ret); +} + +/* + * Interrupt handler. Most of the actual processing is deferred. + */ +void +ath_intr(void *arg) +{ + struct ath_softc *sc = arg; + struct ath_hal *ah = sc->sc_ah; + HAL_INT status; + + mtx_lock_spin(&sc->sc_smtx); + status = sc->sc_status; if (status & HAL_INT_FATAL) { sc->sc_stats.ast_hardware++; ath_hal_intrset(ah, 0); /* disable intr's until reset */ @@ -756,7 +783,7 @@ } else if (status & HAL_INT_RXORN) { sc->sc_stats.ast_rxorn++; ath_hal_intrset(ah, 0); /* disable intr's until reset */ - taskqueue_enqueue(sc->sc_tq, &sc->sc_rxorntask); + ath_rxorn_proc(sc); } else { if (status & HAL_INT_SWBA) { /* @@ -767,27 +794,18 @@ */ ath_beacon_proc(sc, 0); } - if (status & HAL_INT_RXEOL) { - /* - * NB: the hardware should re-read the link when - * RXE bit is written, but it doesn't work at - * least on older hardware revs. - */ - sc->sc_stats.ast_rxeol++; - sc->sc_rxlink = NULL; - } 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) - taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); + ath_rx_proc(sc); if (status & HAL_INT_TX) - taskqueue_enqueue(sc->sc_tq, &sc->sc_txtask); + sc->tx_func(sc); if (status & HAL_INT_BMISS) { sc->sc_stats.ast_bmiss++; - taskqueue_enqueue(sc->sc_tq, &sc->sc_bmisstask); + ath_bmiss_proc(sc); } if (status & HAL_INT_MIB) { sc->sc_stats.ast_mib++; @@ -804,6 +822,9 @@ ath_hal_intrset(ah, sc->sc_imask); } } + /* Enable interrupts */ + ath_hal_intrset(ah, sc->sc_imask); + mtx_unlock_spin(&sc->sc_smtx); } static void @@ -830,7 +851,7 @@ } static void -ath_rxorn_proc(void *arg, int pending) +ath_rxorn_proc(void *arg) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; @@ -840,12 +861,12 @@ } static void -ath_bmiss_proc(void *arg, int pending) +ath_bmiss_proc(void *arg) { struct ath_softc *sc = arg; struct ieee80211com *ic = &sc->sc_ic; - DPRINTF(sc, ATH_DEBUG_ANY, "%s: pending %u\n", __func__, pending); + DPRINTF(sc, ATH_DEBUG_ANY, "%s\n", __func__); KASSERT(ic->ic_opmode == IEEE80211_M_STA, ("unexpect operating mode %u", ic->ic_opmode)); if (ic->ic_state == IEEE80211_S_RUN) { @@ -985,7 +1006,9 @@ */ if (sc->sc_needmib && ic->ic_opmode == IEEE80211_M_STA) sc->sc_imask |= HAL_INT_MIB; + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); + mtx_unlock_spin(&sc->sc_smtx); ifp->if_drv_flags |= IFF_DRV_RUNNING; ic->ic_state = IEEE80211_S_INIT; @@ -1051,7 +1074,9 @@ !sc->sc_ledon); sc->sc_blinking = 0; } + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); + mtx_unlock_spin(&sc->sc_smtx); } ath_draintxq(sc); if (!sc->sc_invalid) { @@ -1109,8 +1134,10 @@ c = ic->ic_curchan; sc->sc_curchan.channel = c->ic_freq; sc->sc_curchan.channelFlags = ath_chan2flags(ic, c); - + + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); /* disable interrupts */ + mtx_unlock_spin(&sc->sc_smtx); ath_draintxq(sc); /* stop xmit side */ ath_stoprecv(sc); /* stop recv side */ /* NB: indicate channel change so we do a full reset */ @@ -1131,7 +1158,9 @@ if_printf(ifp, "%s: unable to start recv logic\n", __func__); if (ic->ic_state == IEEE80211_S_RUN) ath_beacon_config(sc); /* restart beacons */ + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); + mtx_unlock_spin(&sc->sc_smtx); ath_start(ifp); /* restart xmit */ return 0; @@ -2336,12 +2365,18 @@ , bs.bs_cfpnext , bs.bs_timoffset ); + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); + mtx_unlock_spin(&sc->sc_smtx); ath_hal_beacontimers(ah, &bs); sc->sc_imask |= HAL_INT_BMISS; + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); + mtx_unlock_spin(&sc->sc_smtx); } else { + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); + mtx_unlock_spin(&sc->sc_smtx); if (nexttbtt == intval) intval |= HAL_BEACON_RESET_TSF; if (ic->ic_opmode == IEEE80211_M_IBSS) { @@ -2378,7 +2413,9 @@ } ath_hal_beaconinit(ah, nexttbtt, intval); sc->sc_bmisscount = 0; + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); + mtx_unlock_spin(&sc->sc_smtx); /* * When using a self-linked beacon descriptor in * ibss mode load it once here. @@ -2826,7 +2863,7 @@ } static void -ath_rx_proc(void *arg, int npending) +ath_rx_proc(void *arg) { #define PA2DESC(_sc, _pa) \ ((struct ath_desc *)((caddr_t)(_sc)->sc_rxdma.dd_desc + \ @@ -2848,7 +2885,7 @@ NET_LOCK_GIANT(); /* XXX */ - DPRINTF(sc, ATH_DEBUG_RX_PROC, "%s: pending %u\n", __func__, npending); + DPRINTF(sc, ATH_DEBUG_RX_PROC, "%s\n", __func__); ngood = 0; nf = ath_hal_getchannoise(ah, &sc->sc_curchan); tsf = ath_hal_gettsf64(ah); @@ -3941,7 +3978,7 @@ * for a single hardware transmit queue (e.g. 5210 and 5211). */ static void -ath_tx_proc_q0(void *arg, int npending) +ath_tx_proc_q0(void *arg) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; @@ -3964,7 +4001,7 @@ * for four hardware queues, 0-3 (e.g. 5212 w/ WME support). */ static void -ath_tx_proc_q0123(void *arg, int npending) +ath_tx_proc_q0123(void *arg) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; @@ -4000,7 +4037,7 @@ * Deferred processing of transmit interrupt. */ static void -ath_tx_proc(void *arg, int npending) +ath_tx_proc(void *arg) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; @@ -4304,7 +4341,9 @@ * hardware at the new frequency, and then re-enable * the relevant bits of the h/w. */ + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); /* disable interrupts */ + mtx_unlock_spin(&sc->sc_smtx); ath_draintxq(sc); /* clear pending tx frames */ ath_stoprecv(sc); /* turn off frame recv */ if (!ath_hal_reset(ah, sc->sc_opmode, &hchan, AH_TRUE, &status)) { @@ -4358,7 +4397,9 @@ /* * Re-enable interrupts. */ + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); + mtx_unlock_spin(&sc->sc_smtx); } return 0; } @@ -4464,7 +4505,9 @@ /* * NB: disable interrupts so we don't rx frames. */ + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask &~ HAL_INT_GLOBAL); + mtx_unlock_spin(&sc->sc_smtx); /* * Notify the rate control algorithm. */ @@ -4573,8 +4616,10 @@ sc->sc_halstats.ns_avgrssi = ATH_RSSI_DUMMY_MARKER; sc->sc_halstats.ns_avgtxrssi = ATH_RSSI_DUMMY_MARKER; } else { + mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask &~ (HAL_INT_SWBA | HAL_INT_BMISS)); + mtx_unlock_spin(&sc->sc_smtx); sc->sc_imask &= ~(HAL_INT_SWBA | HAL_INT_BMISS); } done: ==== //depot/projects/soc2006/intr_filter/dev/ath/if_ath_pci.c#3 (text+ko) ==== @@ -168,7 +168,7 @@ goto bad1; } if (bus_setup_intr(dev, psc->sc_irq, - INTR_TYPE_NET | INTR_MPSAFE, NULL, + INTR_TYPE_NET | INTR_MPSAFE, ath_filter, ath_intr, sc, &psc->sc_ih)) { device_printf(dev, "could not establish interrupt\n"); goto bad2; @@ -194,11 +194,14 @@ } ATH_LOCK_INIT(sc); + mtx_init(&sc->sc_smtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, + MTX_SPIN); error = ath_attach(pci_get_device(dev), sc); if (error == 0) return error; + mtx_destroy(&sc->sc_smtx); ATH_LOCK_DESTROY(sc); bus_dma_tag_destroy(sc->sc_dmat); bad3: ==== //depot/projects/soc2006/intr_filter/dev/ath/if_athvar.h#3 (text+ko) ==== @@ -223,6 +223,13 @@ u_int sc_mcastrate; /* ieee rate for mcastrateix */ u_int sc_txantenna; /* tx antenna (fixed or auto) */ HAL_INT sc_imask; /* interrupt mask copy */ + HAL_INT sc_status; + struct mtx sc_smtx; /* + * guard access to sc_status + * and to the interrupt handling + * registers (?!?!?!?!) + */ + void (*tx_func)(void *); u_int sc_keymax; /* size of key cache */ u_int8_t sc_keymap[ATH_KEYBYTES];/* key use bit map */ @@ -254,7 +261,6 @@ struct ath_descdma sc_rxdma; /* RX descriptos */ ath_bufhead sc_rxbuf; /* receive buffer */ u_int32_t *sc_rxlink; /* link ptr in last RX desc */ - struct task sc_rxtask; /* rx int processing */ struct task sc_rxorntask; /* rxorn int processing */ struct task sc_radartask; /* radar processing */ u_int8_t sc_defant; /* current default antenna */ @@ -270,7 +276,6 @@ u_int sc_txintrperiod;/* tx interrupt batching */ struct ath_txq sc_txq[HAL_NUM_TX_QUEUES]; struct ath_txq *sc_ac2q[5]; /* WME AC -> h/w q map */ - struct task sc_txtask; /* tx int processing */ struct ath_descdma sc_bdma; /* beacon descriptors */ ath_bufhead sc_bbuf; /* beacon buffers */ @@ -324,6 +329,7 @@ void ath_resume(struct ath_softc *); void ath_suspend(struct ath_softc *); void ath_shutdown(struct ath_softc *); +int ath_filter(void *); void ath_intr(void *); /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200608011725.k71HP4ol019342>