Date: Fri, 5 Aug 2011 06:57:44 +0000 (UTC) From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r224654 - user/adrian/if_ath_tx/sys/dev/ath Message-ID: <201108050657.p756viGF094536@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: adrian Date: Fri Aug 5 06:57:44 2011 New Revision: 224654 URL: http://svn.freebsd.org/changeset/base/224654 Log: Flip back to having the TXQ lock held during the whole of ath_tx_processq(). I'll deal with correcting the locking later so LORs don't occur. For now, this will let me write TX packet completion functions to implement BAW tracking and aggregation, which will update per-TID state. Having the TXQ locked in this way means that: (a) the order of TXQ completions being processed will be correct - ie, multiple concurrent calls to ath_tx_processq() for the same TXQ won't mess things up; (b) the per-TID state is correctly locked, so concurrent TX scheduling and TX completion isn't going to end up with subtle race conditions. As long as completions are only ever called from ath_tx_proc(), this should be fine - it won't race with itself. The race conditions are what I'm worried about. Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Thu Aug 4 17:17:57 2011 (r224653) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Fri Aug 5 06:57:44 2011 (r224654) @@ -4139,17 +4139,16 @@ ath_tx_processq(struct ath_softc *sc, st int nacked; HAL_STATUS status; + ATH_TXQ_LOCK(txq); DPRINTF(sc, ATH_DEBUG_TX_PROC, "%s: tx queue %u head %p link %p\n", __func__, txq->axq_qnum, (caddr_t)(uintptr_t) ath_hal_gettxbuf(sc->sc_ah, txq->axq_qnum), txq->axq_link); nacked = 0; for (;;) { - ATH_TXQ_LOCK(txq); txq->axq_intrcnt = 0; /* reset periodic desc intr count */ bf = STAILQ_FIRST(&txq->axq_q); if (bf == NULL) { - ATH_TXQ_UNLOCK(txq); break; } ds0 = &bf->bf_desc[0]; @@ -4162,7 +4161,6 @@ ath_tx_processq(struct ath_softc *sc, st status == HAL_OK); #endif if (status == HAL_EINPROGRESS) { - ATH_TXQ_UNLOCK(txq); break; } ATH_TXQ_REMOVE_HEAD(txq, bf_list); @@ -4179,7 +4177,6 @@ ath_tx_processq(struct ath_softc *sc, st if (txq->axq_depth == 0) #endif txq->axq_link = NULL; - ATH_TXQ_UNLOCK(txq); ni = bf->bf_node; /* @@ -4226,14 +4223,6 @@ ath_tx_processq(struct ath_softc *sc, st #endif /* Kick the TXQ scheduler */ - /* - * We can't hold the TXQ lock across the completion function; - * the IEEE80211_NODE_LOCK is likely going to be held there - * during buffer/node free. It's up to the completion - * function to grab the TXQ lock before updating TID state - * (eg sliding the BAW along.) - */ - ATH_TXQ_LOCK(txq); ath_txq_sched(sc, txq); ATH_TXQ_UNLOCK(txq);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108050657.p756viGF094536>