Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 11 Sep 2011 16:52:07 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   svn commit: r225480 - user/adrian/if_ath_tx/sys/dev/ath
Message-ID:  <201109111652.p8BGq7eE087498@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Sun Sep 11 16:52:07 2011
New Revision: 225480
URL: http://svn.freebsd.org/changeset/base/225480

Log:
  Revamp how interrupts and handled and scheduled, bringing this driver
  in line with what the reference driver and linux ath9k does.
  
  Currently, ath_intr() would submit certain things to occur via
  taskqueue_enqueue(). In the case of handling TX interrupts, it's
  quite possible that a TX completion interrupt (EOL, OK, mitigated,
  etc) could occur whilst the TX completion task was running.
  
  This meant that a small race could occur where the TX completion
  call to txqactive() (which would call into the HAL to see whether
  the current TXQ had some active interrupt event to service, and
  then clear that flag) could race with ath_intr() and the call
  to ah_getPendingInterrupts(), which for ar5212/ar5416 will set
  the TXQ active bits based on the contents of the relevant interrupt
  registers.
  
  This also means that ath_intr() won't occur during one of the TX, RX
  or reset-during-fatal situations. The only current exception to this
  are fatal events or SWBA, which still is enabled.
  
  It didn't completely close up the initial TXQ hang issue I saw earlier,
  but it along with the previous missing TXEOL change from earlier did
  eliminate some of the TXQ hangs.
  
  I'm going to be porting more code over from the reference driver and
  ath9k, so having the interrupt handling model here match those will
  make it easier.
  
  Finally, the ath_start() calls have been moved from various places in
  the receive/tx-complete path and into the deferred interrupt handler.
  
  Finally finally - I've disabled the optimised queue proc handling
  (q0 and q0123) versions of ath_tx_proc(), this needs to be reintroduced
  before this is merged into HEAD.
  
  Finally finally finally - I need to reschedule a read event after
  SWBA (which still occurs in interrupt context) to kick along fast-frames
  handling.

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

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath.c	Sun Sep 11 16:42:03 2011	(r225479)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath.c	Sun Sep 11 16:52:07 2011	(r225480)
@@ -132,6 +132,7 @@ static int	ath_media_change(struct ifnet
 static void	ath_watchdog(void *);
 static int	ath_ioctl(struct ifnet *, u_long, caddr_t);
 static void	ath_fatal_proc(void *, int);
+static void	ath_handle_intr(void *, int);
 static void	ath_bmiss_vap(struct ieee80211vap *);
 static void	ath_bmiss_proc(void *, int);
 static void	ath_key_update_begin(struct ieee80211vap *);
@@ -173,8 +174,10 @@ static int	ath_tx_setup(struct ath_softc
 static int	ath_wme_update(struct ieee80211com *);
 static void	ath_tx_cleanupq(struct ath_softc *, struct ath_txq *);
 static void	ath_tx_cleanup(struct ath_softc *);
+#if 0
 static void	ath_tx_proc_q0(void *, int);
 static void	ath_tx_proc_q0123(void *, int);
+#endif
 static void	ath_tx_proc(void *, int);
 static int	ath_chan_set(struct ath_softc *, struct ieee80211_channel *);
 static void	ath_draintxq(struct ath_softc *);
@@ -390,14 +393,15 @@ ath_attach(u_int16_t devid, struct ath_s
 
 	ATH_TXBUF_LOCK_INIT(sc);
 
-	sc->sc_tq = taskqueue_create("ath_taskq", M_NOWAIT,
+	sc->sc_tq = taskqueue_create_fast("ath_taskq", M_NOWAIT,
 		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_bmisstask, 0, ath_bmiss_proc, sc);
 	TASK_INIT(&sc->sc_bstucktask,0, ath_bstuck_proc, sc);
+	TASK_INIT(&sc->sc_intrtask, 0, ath_handle_intr, sc);
+	TASK_INIT(&sc->sc_fataltask, 0, ath_fatal_proc, sc);
 
 	/*
 	 * Allocate hardware transmit queues: one queue for
@@ -446,23 +450,6 @@ ath_attach(u_int16_t devid, struct ath_s
 	}
 
 	/*
-	 * Special case certain configurations.  Note the
-	 * CAB queue is handled by these specially so don't
-	 * include them when checking the txq setup mask.
-	 */
-	switch (sc->sc_txqsetup &~ (1<<sc->sc_cabq->axq_qnum)) {
-	case 0x01:
-		TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc_q0, sc);
-		break;
-	case 0x0f:
-		TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc_q0123, sc);
-		break;
-	default:
-		TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc, sc);
-		break;
-	}
-
-	/*
 	 * Setup rate control.  Some rate control modules
 	 * call back to change the anntena state so expose
 	 * the necessary entry points.
@@ -1345,6 +1332,80 @@ ath_shutdown(struct ath_softc *sc)
 }
 
 /*
+ * Do deferred interrupt processing; then re-enable interrupts
+ * if required.
+ */
+static void
+ath_handle_intr(void *arg, int npending)
+{
+	struct ath_softc *sc = (struct ath_softc *) arg;
+	HAL_INT status;
+	struct ath_hal *ah = sc->sc_ah;
+	struct ifnet *ifp = sc->sc_ifp;
+	uint32_t txqs;
+
+	if (sc->sc_invalid) {
+		/*
+		 * The hardware is not ready/present, don't touch anything.
+		 * Note this can happen early on if the IRQ is shared.
+		 */
+		DPRINTF(sc, ATH_DEBUG_ANY, "%s: invalid; ignored\n", __func__);
+		return;
+	}
+
+	/*
+	 * Fetch the current interrupt status from the interrupt
+	 * handler. It's assumed that any interrupt which would lead
+	 * us here won't happen until the interrupt is cleared.
+	 *
+	 * XXX Thus I'm not using a lock just yet.
+	 */
+	status = sc->sc_intrstatus;
+
+	if (status & HAL_INT_FATAL) {
+		ath_hal_intrset(ah, 0);		/* disable intr's until reset */
+		ath_fatal_proc(sc, 0);
+		return;
+	}
+
+	if (status & (HAL_INT_RXEOL | HAL_INT_RX)) {
+		ath_rx_proc(sc, 1);
+		/* XXX this may reset the hardware? Should handle that! */
+	}
+	if (status & HAL_INT_TXURN) {
+		/* bump tx trigger level */
+		ath_hal_updatetxtriglevel(ah, AH_TRUE);
+	}
+	if (status & HAL_INT_TX) {
+		ath_tx_proc(sc, 1);
+	}
+
+	/*
+	 * If at this point, any of the TX interrupt status lines are
+	 * active, we've messed up. These should only be updated in
+	 * ath_intr().
+	 */
+	txqs = 0xff;
+	ath_hal_gettxintrtxqs(ah, &txqs);
+	if (txqs != 0) {
+		device_printf(sc->sc_dev,
+		    "%s: unserviced TXQs: txq mask=0x%.08x, int status=0x%.08x\n",
+		    __func__, txqs, status);
+		/* XXX since it's been cleared, there's no way to fix it.. */
+	}
+
+	/*
+	 * If TX or RX occured, call ath_start() if the interface
+	 * can grab some packets.
+	 */
+	if (!IFQ_IS_EMPTY(&ifp->if_snd))
+		ath_start(ifp);
+
+	/* re-enable interrupts */
+	ath_hal_intrset(sc->sc_ah, sc->sc_imask);
+}
+
+/*
  * Interrupt handler.  Most of the actual processing is deferred.
  */
 void
@@ -1354,6 +1415,7 @@ ath_intr(void *arg)
 	struct ifnet *ifp = sc->sc_ifp;
 	struct ath_hal *ah = sc->sc_ah;
 	HAL_INT status = 0;
+	int sched = 0;
 
 	if (sc->sc_invalid) {
 		/*
@@ -1389,10 +1451,12 @@ ath_intr(void *arg)
 	if (status == 0x0)
 		return;
 
+	/* XXX locking? */
+	sc->sc_intrstatus = status;
+
 	if (status & HAL_INT_FATAL) {
 		sc->sc_stats.ast_hardware++;
-		ath_hal_intrset(ah, 0);		/* disable intr's until reset */
-		ath_fatal_proc(sc, 0);
+		sched = 1;
 	} else {
 		if (status & HAL_INT_SWBA) {
 			/*
@@ -1422,12 +1486,12 @@ 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);
+				/* XXX there's no longer an rxtask? How to force? */
+				taskqueue_enqueue_fast(sc->sc_tq, &sc->sc_rxtask);
 #endif
 			}
 		}
 		if (status & HAL_INT_RXEOL) {
-			int imask = sc->sc_imask;
 			/*
 			 * NB: the hardware should re-read the link when
 			 *     RXE bit is written, but it doesn't work at
@@ -1443,33 +1507,27 @@ ath_intr(void *arg)
 			 * by a call to ath_reset() somehow, the
 			 * interrupt mask will be correctly reprogrammed.
 			 */
-			imask &= ~(HAL_INT_RXEOL | HAL_INT_RXORN);
-			ath_hal_intrset(ah, imask);
-			/*
-			 * 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);
+			/* XXX this is already reset in the sched call below */
 			sc->sc_rxlink = NULL;
 			sc->sc_kickpcu = 1;
+			sched = 1;
 		}
 		if (status & HAL_INT_TXURN) {
 			sc->sc_stats.ast_txurn++;
 			/* bump tx trigger level */
-			ath_hal_updatetxtriglevel(ah, AH_TRUE);
+			sched = 1;
 		}
 		if (status & HAL_INT_RX) {
 			sc->sc_stats.ast_rx_intr++;
-			taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask);
+			sched = 1;
 		}
 		if (status & HAL_INT_TX) {
 			sc->sc_stats.ast_tx_intr++;
-			taskqueue_enqueue(sc->sc_tq, &sc->sc_txtask);
+			sched = 1;
 		}
 		if (status & HAL_INT_BMISS) {
 			sc->sc_stats.ast_bmiss++;
-			taskqueue_enqueue(sc->sc_tq, &sc->sc_bmisstask);
+			taskqueue_enqueue_fast(sc->sc_tq, &sc->sc_bmisstask);
 		}
 		if (status & HAL_INT_GTT)
 			sc->sc_stats.ast_tx_timeout++;
@@ -1494,6 +1552,18 @@ ath_intr(void *arg)
 			sc->sc_stats.ast_rxorn++;
 		}
 	}
+
+	/*
+	 * If any of the above checks require an ISR schedule,
+	 * enqueue the task and disable interrupts.
+	 *
+	 * Since beacon handling is done in interrupt context at the
+	 * moment, always leave that enabled.
+	 */
+	if (sched == 1) {
+		ath_hal_intrset(ah, (sc->sc_imask & HAL_INT_SWBA));
+		taskqueue_enqueue_fast(sc->sc_tq, &sc->sc_intrtask);
+	}
 }
 
 static void
@@ -1826,6 +1896,9 @@ ath_reset(struct ifnet *ifp)
 	}
 	ath_hal_intrset(ah, sc->sc_imask);
 
+	/*
+	 * XXX should this be done in-line?
+	 */
 	ath_start(ifp);			/* restart xmit */
 	return 0;
 }
@@ -3950,23 +4023,20 @@ rx_next:
 		 * XXX kick the PCU again to continue RXing?
 		 */
 		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__);
+			/* XXX this must be scheduled! */
 			ath_reset(ifp);
 			return;
 		}
-		ath_hal_intrset(ah, sc->sc_imask);
 	}
 
 	if ((ifp->if_drv_flags & IFF_DRV_OACTIVE) == 0) {
 #ifdef IEEE80211_SUPPORT_SUPERG
 		ieee80211_ff_age_all(ic, 100);
 #endif
-		if (!IFQ_IS_EMPTY(&ifp->if_snd))
-			ath_start(ifp);
 	}
 #undef PA2DESC
 }
@@ -4449,6 +4519,7 @@ txqactive(struct ath_hal *ah, int qnum)
 	return (txqs & (1<<qnum));
 }
 
+#if 0
 /*
  * Deferred processing of transmit interrupt; special-cased
  * for a single hardware transmit queue (e.g. 5210 and 5211).
@@ -4509,6 +4580,7 @@ ath_tx_proc_q0123(void *arg, int npendin
 
 	ath_start(ifp);
 }
+#endif
 
 /*
  * Deferred processing of transmit interrupt.

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Sun Sep 11 16:42:03 2011	(r225479)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Sun Sep 11 16:52:07 2011	(r225480)
@@ -411,6 +411,7 @@ struct ath_softc {
 	u_int			sc_fftxqmax;	/* max frames before drop */
 	u_int			sc_txantenna;	/* tx antenna (fixed or auto) */
 	HAL_INT			sc_imask;	/* interrupt mask copy */
+	HAL_INT			sc_intrstatus;	/* current interrupt status */
 	u_int			sc_keymax;	/* size of key cache */
 	u_int8_t		sc_keymap[ATH_KEYBYTES];/* key use bit map */
 
@@ -429,7 +430,6 @@ struct ath_softc {
 	ath_bufhead		sc_rxbuf;	/* receive buffer */
 	struct mbuf		*sc_rxpending;	/* pending receive data */
 	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 */
 	u_int8_t		sc_rxotherant;	/* rx's on non-default antenna*/
 	u_int64_t		sc_lastrx;	/* tsf at last rx'd frame */
@@ -446,7 +446,6 @@ struct ath_softc {
 	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 */
 	int			sc_wd_timer;	/* count down for wd timer */
 	struct callout		sc_wd_ch;	/* tx watchdog timer */
 	struct ath_tx_radiotap_header sc_tx_th;
@@ -460,6 +459,8 @@ struct ath_softc {
 	struct ath_txq		*sc_cabq;	/* tx q for cab frames */
 	struct task		sc_bmisstask;	/* bmiss int processing */
 	struct task		sc_bstucktask;	/* stuck beacon processing */
+	struct task		sc_intrtask;	/* deferred interrupt processing */
+	struct task		sc_fataltask;	/* deferred reset processing */
 	enum {
 		OK,				/* no change needed */
 		UPDATE,				/* update pending */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201109111652.p8BGq7eE087498>