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 */
_______________________________________________
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"
