PERFORCE change 102954 for review

Paolo Pisati piso at FreeBSD.org
Tue Aug 1 17:25:05 UTC 2006


http://perforce.freebsd.org/chv.cgi?CH=102954

Change 102954 by piso at 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 *);
 
 /*


More information about the p4-projects mailing list