From owner-svn-src-user@FreeBSD.ORG Wed Aug 3 09:32:58 2011
Return-Path: <owner-svn-src-user@FreeBSD.ORG>
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 34CF21065767;
Wed, 3 Aug 2011 09:32:58 +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 0B3C38FC15;
Wed, 3 Aug 2011 09:32:58 +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 p739Wvoc095314;
Wed, 3 Aug 2011 09:32:57 GMT (envelope-from adrian@svn.freebsd.org)
Received: (from adrian@localhost)
by svn.freebsd.org (8.14.4/8.14.4/Submit) id p739WvIL095312;
Wed, 3 Aug 2011 09:32:57 GMT (envelope-from adrian@svn.freebsd.org)
Message-Id: <201108030932.p739WvIL095312@svn.freebsd.org>
From: Adrian Chadd <adrian@FreeBSD.org>
Date: Wed, 3 Aug 2011 09:32:57 +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: r224630 - 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" <svn-src-user.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-user>,
<mailto:svn-src-user-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-user>
List-Post: <mailto:svn-src-user@freebsd.org>
List-Help: <mailto:svn-src-user-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-user>,
<mailto:svn-src-user-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 03 Aug 2011 09:32:58 -0000
Author: adrian
Date: Wed Aug 3 09:32:57 2011
New Revision: 224630
URL: http://svn.freebsd.org/changeset/base/224630
Log:
Revert the previous commit - it meant a LOR was unavoidable between the
TXQ lock and the IEEE80211_NODE_LOCK.
Eg:
1st 0xc086c6cc ath1_node_lock (ath1_node_lock) @ /data/freebsd/mips/if_ath_tx/src/sys/net80211/ieee80211_node.c:1702
2nd 0x80a02738 ath1_txq1 (ath1_txq1) @ /data/freebsd/mips/if_ath_tx/src/sys/dev/ath/if_ath_tx.c:1631
1st 0x80a027c8 ath1_txq3 (ath1_txq3) @ /data/freebsd/mips/if_ath_tx/src/sys/dev/ath/if_ath.c:4147
2nd 0xc086c6cc ath1_node_lock (ath1_node_lock) @ /data/freebsd/mips/if_ath_tx/src/sys/net80211/ieee80211_node.c:1702
I'm going to do some digging to see whether this is a false LOR - ie, whether
it's possible for the LOR to involve the same node AND TXQ.
For now, this means completion functions need to do their own TXQ locking
if they're going to fiddle with TID state. This introduces a few potential
race conditions - which is why I'd like to hold this lock across the
completion function if possible.
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 Wed Aug 3 09:21:52 2011 (r224629)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Wed Aug 3 09:32:57 2011 (r224630)
@@ -4146,9 +4146,11 @@ ath_tx_processq(struct ath_softc *sc, st
nacked = 0;
ATH_TXQ_LOCK(txq);
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];
@@ -4161,6 +4163,7 @@ 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);
@@ -4177,6 +4180,7 @@ 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;
/*
@@ -4220,6 +4224,14 @@ 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);