Date: Thu, 11 Aug 2011 08:51:53 +0000 (UTC) From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r224774 - user/adrian/if_ath_tx/sys/dev/ath Message-ID: <201108110851.p7B8prYL083821@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: adrian Date: Thu Aug 11 08:51:53 2011 New Revision: 224774 URL: http://svn.freebsd.org/changeset/base/224774 Log: Fix the TXQ recursive locking bug that sometimes occurs when an interface is destroyed. Only lock the TXQ if it isn't locked by us. Modified: user/adrian/if_ath_tx/sys/dev/ath/README user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Modified: user/adrian/if_ath_tx/sys/dev/ath/README ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/README Thu Aug 11 06:45:12 2011 (r224773) +++ user/adrian/if_ath_tx/sys/dev/ath/README Thu Aug 11 08:51:53 2011 (r224774) @@ -59,8 +59,15 @@ fork_trampoline+10 (?,?,?,?) ra 0 sp c77 pid 0 + + +Fixed issues: + * Recursive TXQ lock on interface destruction: + - Fixed by only locking the TXQ in ath_tx_node_flush if the TXQ + isn't already locked by us. + drian-home-mips# ifconfig wlan0 destroy ath1: ath_tx_node_flush: called ar5212StopDmaReceive: dma failed to stop in 10ms Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Thu Aug 11 06:45:12 2011 (r224773) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Thu Aug 11 08:51:53 2011 (r224774) @@ -1818,41 +1818,34 @@ ath_tx_tid_free_pkts(struct ath_softc *s /* * Flush all software queued packets for the given node. * - * XXX there's a recursive lock here which currently panics - * XXX the kernel. - * - * bringing the interface down/up causes a flush on interface - * _up_; the stack looks like this: - * - * <panic> - * ath_tx_node_flush - locks each hardware txq - * ieee80211_free_node - * ath_tx_freebuf - * ath_tx_default_comp - * ath_tx_processq - locks the hardware txq - * - * To fix? Not (yet) sure. Perhaps unsched the TID in freebuf - * if there's no further buffers for it; then only flush - * nodes w/ a lock below if the qlen is 0. But can the tid - * txq length be checked without having the txq locked? - * It'll be locked by someone, but if this node is being freed, - * in theory all references to the node should be released - * and thus there shouldn't be any packets in the TIDs for that - * node. Hm. + * Work around recursive TXQ locking. + * This occurs when a completion handler frees the last buffer + * for a node, and the node is thus freed. This causes the node + * to be cleaned up, which ends up calling ath_tx_node_flush. */ void ath_tx_node_flush(struct ath_softc *sc, struct ath_node *an) { int tid; + int is_owned; for (tid = 0; tid < IEEE80211_TID_SIZE; tid++) { struct ath_tid *atid = &an->an_tid[tid]; struct ath_txq *txq = sc->sc_ac2q[atid->ac]; - ATH_TXQ_LOCK(txq); + /* + * Only lock if we don't have it locked. + * It may be locked already if this is being called + * when the last buffer for an ieee80211_node is + * being freed. + */ + is_owned = ATH_TXQ_IS_LOCKED(txq); + if (! is_owned) + ATH_TXQ_LOCK(txq); /* Remove this tid from the list of active tids */ ath_tx_tid_unsched(sc, an, tid); - ATH_TXQ_UNLOCK(txq); + if (! is_owned) + ATH_TXQ_UNLOCK(txq); /* Free packets */ ath_tx_tid_free_pkts(sc, an, tid); Modified: user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Thu Aug 11 06:45:12 2011 (r224773) +++ user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Thu Aug 11 08:51:53 2011 (r224774) @@ -249,6 +249,7 @@ struct ath_txq { #define ATH_TXQ_LOCK(_tq) mtx_lock(&(_tq)->axq_lock) #define ATH_TXQ_UNLOCK(_tq) mtx_unlock(&(_tq)->axq_lock) #define ATH_TXQ_LOCK_ASSERT(_tq) mtx_assert(&(_tq)->axq_lock, MA_OWNED) +#define ATH_TXQ_IS_LOCKED(_tq) mtx_owned(&(_tq)->axq_lock) #define ATH_TXQ_INSERT_TAIL(_tq, _elm, _field) do { \ STAILQ_INSERT_TAIL(&(_tq)->axq_q, (_elm), _field); \
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108110851.p7B8prYL083821>