svn commit: r236583 - head/sys/dev/ath

[Adrian Chadd]

Author: adrian
Date: Mon Jun  4 22:01:12 2012
New Revision: 236583
URL: http://svn.freebsd.org/changeset/base/236583

Log:
  Migrate the TX path to a taskqueue for now, until a better way of
  implementing parallel TX and TX/RX completion can be done without
  simply abusing long-held locks.
  
  Right now, multiple concurrent ath_start() entries can result in
  frames being dequeued out of order.  Well, they're dequeued in order
  fine, but if there's any preemption or race between CPUs between:
  
  * removing the frame from the ifnet, and
  * calling and runningath_tx_start(), until the frame is placed on a
    software or hardware TXQ
  
  Then although dequeueing the frame is in-order, queueing it to the hardware
  may be out of order.
  
  This is solved in a lot of other drivers by just holding a TX lock over
  a rather long period of time.  This lets them continue to direct dispatch
  without races between dequeue and hardware queue.
  
  Note to observers: if_transmit() doesn't necessarily solve this.
  It removes the ifnet from the main path, but the same issue exists if
  there's some intermediary queue (eg a bufring, which as an aside also
  may pull in ifnet when you're using ALTQ.)
  
  So, until I can sit down and code up a much better way of doing parallel
  TX, I'm going to leave the TX path using a deferred taskqueue task.
  What I will likely head towards is doing a direct dispatch to hardware
  or software via if_transmit(), but it'll require some driver changes to
  allow queues to be made without using the really large ath_buf / ath_desc
  entries.
  
  TODO:
  
  * Look at how feasible it'll be to just do direct dispatch to
    ath_tx_start() from if_transmit(), avoiding doing _any_ intermediary
    serialisation into a global queue.  This may break ALTQ for example,
    so I have to be delicate.
  
  * It's quite likely that I should break up ath_tx_start() so it
    deposits frames onto the software queues first, and then only fill
    in the 802.11 fields when it's being queued to the hardware.
    That will make the if_transmit() -> software queue path very
    quick and lightweight.
  
  * This has some very bad behaviour when using ACPI and Cx states.
    I'll do some subsequent analysis using KTR and schedgraph and file
    a follow-up PR or two.
  
  PR:		kern/168649

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

Modified: head/sys/dev/ath/if_ath.c
==============================================================================
--- head/sys/dev/ath/if_ath.c	Mon Jun  4 21:34:49 2012	(r236582)
+++ head/sys/dev/ath/if_ath.c	Mon Jun  4 22:01:12 2012	(r236583)
@@ -373,6 +373,7 @@ ath_attach(u_int16_t devid, struct ath_s
 		"%s taskq", ifp->if_xname);
 
 	TASK_INIT(&sc->sc_rxtask, 0, ath_rx_tasklet, sc);
+	TASK_INIT(&sc->sc_txstarttask, 0, ath_tx_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);
@@ -2325,6 +2326,15 @@ void
 ath_start(struct ifnet *ifp)
 {
 	struct ath_softc *sc = ifp->if_softc;
+
+	taskqueue_enqueue(sc->sc_tq, &sc->sc_txstarttask);
+}
+
+void
+ath_tx_tasklet(void *arg, int npending)
+{
+	struct ath_softc *sc = (struct ath_softc *) arg;
+	struct ifnet *ifp = sc->sc_ifp;
 	struct ieee80211_node *ni;
 	struct ath_buf *bf;
 	struct mbuf *m, *next;
@@ -3499,7 +3509,8 @@ ath_tx_proc_q0(void *arg, int npending)
 	sc->sc_txproc_cnt--;
 	ATH_PCU_UNLOCK(sc);
 
-	ath_start(ifp);
+	// ath_start(ifp);
+	ath_tx_tasklet(sc, 1);
 }
 
 /*
@@ -3549,7 +3560,8 @@ ath_tx_proc_q0123(void *arg, int npendin
 	sc->sc_txproc_cnt--;
 	ATH_PCU_UNLOCK(sc);
 
-	ath_start(ifp);
+	//ath_start(ifp);
+	ath_tx_tasklet(sc, 1);
 }
 
 /*
@@ -3592,7 +3604,8 @@ ath_tx_proc(void *arg, int npending)
 	sc->sc_txproc_cnt--;
 	ATH_PCU_UNLOCK(sc);
 
-	ath_start(ifp);
+	//ath_start(ifp);
+	ath_tx_tasklet(sc, 1);
 }
 #undef	TXQACTIVE
 

Modified: head/sys/dev/ath/if_ath_misc.h
==============================================================================
--- head/sys/dev/ath/if_ath_misc.h	Mon Jun  4 21:34:49 2012	(r236582)
+++ head/sys/dev/ath/if_ath_misc.h	Mon Jun  4 22:01:12 2012	(r236583)
@@ -83,6 +83,7 @@ extern void ath_setslottime(struct ath_s
  * we can kill this.
  */
 extern void ath_start(struct ifnet *ifp);
+extern void ath_tx_tasklet(void *arg, int npending);
 
 
 #endif

Modified: head/sys/dev/ath/if_ath_rx.c
==============================================================================
--- head/sys/dev/ath/if_ath_rx.c	Mon Jun  4 21:34:49 2012	(r236582)
+++ head/sys/dev/ath/if_ath_rx.c	Mon Jun  4 22:01:12 2012	(r236583)
@@ -899,7 +899,8 @@ rx_proc_next:
 		ieee80211_ff_age_all(ic, 100);
 #endif
 		if (!IFQ_IS_EMPTY(&ifp->if_snd))
-			ath_start(ifp);
+			ath_tx_tasklet(sc, 1);
+			//ath_start(ifp);
 	}
 #undef PA2DESC
 

Modified: head/sys/dev/ath/if_athvar.h
==============================================================================
--- head/sys/dev/ath/if_athvar.h	Mon Jun  4 21:34:49 2012	(r236582)
+++ head/sys/dev/ath/if_athvar.h	Mon Jun  4 22:01:12 2012	(r236583)
@@ -476,6 +476,7 @@ struct ath_softc {
 	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 */
+	struct task		sc_txstarttask;	/* ath_start() 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 */