From owner-svn-src-user@FreeBSD.ORG Fri Aug 5 06:57:44 2011 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B36351065670; Fri, 5 Aug 2011 06:57:44 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id A28D18FC0C; Fri, 5 Aug 2011 06:57:44 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p756vibc094539; Fri, 5 Aug 2011 06:57:44 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p756viGF094536; Fri, 5 Aug 2011 06:57:44 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201108050657.p756viGF094536@svn.freebsd.org> From: Adrian Chadd Date: Fri, 5 Aug 2011 06:57:44 +0000 (UTC) To: src-committers@freebsd.org, svn-src-user@freebsd.org X-SVN-Group: user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r224654 - user/adrian/if_ath_tx/sys/dev/ath X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Aug 2011 06:57:44 -0000 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);