Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 3 Aug 2011 09:32:57 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   svn commit: r224630 - user/adrian/if_ath_tx/sys/dev/ath
Message-ID:  <201108030932.p739WvIL095312@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108030932.p739WvIL095312>