Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 11 Apr 2008 05:47:03 GMT
From:      Sam Leffler <sam@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 139779 for review
Message-ID:  <200804110547.m3B5l3lE028921@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=139779

Change 139779 by sam@sam_ebb on 2008/04/11 05:46:03

	o must grab softc lock when submitting device cmd in wme callback
	o add lock assert in iwn_cmd to catch any other cases
	o split iwn_stop to create locked variant and use it to eliminate
	  some recursive locking
	o pull softc lock up higher in iwn_ops to cover debug printf (to
	  avoid msg intermingling on smp) and to simplify logic
	o return consistently after canceling scan (needs overhaul)

Affected files ...

.. //depot/projects/vap/sys/dev/iwn/if_iwn.c#8 edit

Differences ...

==== //depot/projects/vap/sys/dev/iwn/if_iwn.c#8 (text+kox) ====

@@ -2649,6 +2649,8 @@
 	struct iwn_tx_cmd *cmd;
 	bus_addr_t paddr;
 
+	IWN_LOCK_ASSERT(sc);
+
 	KASSERT(size <= sizeof cmd->data, ("Command too big"));
 
 	desc = &ring->desc[ring->cur];
@@ -2819,7 +2821,10 @@
 		cmd.ac[i].txoplimit =
 		    htole16(IWN_TXOP_TO_US(wmep->wmep_txopLimit));
 	}
-	return iwn_cmd(sc, IWN_CMD_EDCA_PARAMS, &cmd, sizeof cmd, 1);
+	IWN_LOCK(sc);
+	(void) iwn_cmd(sc, IWN_CMD_EDCA_PARAMS, &cmd, sizeof cmd, 1 /*async*/);
+	IWN_UNLOCK(sc);
+	return 0;
 #undef IWN_TXOP_TO_US
 #undef IWN_EXP2
 }
@@ -4229,14 +4234,14 @@
 	IWN_UNLOCK(sc);
 }
 
-void
-iwn_stop(struct iwn_softc *sc)
+static void
+iwn_stop_locked(struct iwn_softc *sc)
 {
 	struct ifnet *ifp = sc->sc_ifp;
 	uint32_t tmp;
 	int i;
 
-	IWN_LOCK(sc);
+	IWN_LOCK_ASSERT(sc);
 
 	IWN_WRITE(sc, IWN_RESET, IWN_NEVO_RESET);
 
@@ -4268,7 +4273,13 @@
 
 	tmp = IWN_READ(sc, IWN_RESET);
 	IWN_WRITE(sc, IWN_RESET, tmp | IWN_SW_RESET);
+}
 
+void
+iwn_stop(struct iwn_softc *sc)
+{
+	IWN_LOCK(sc);
+	iwn_stop_locked(sc);
 	IWN_UNLOCK(sc);
 }
 
@@ -4369,16 +4380,15 @@
 		sc->sc_cmd_cur = (sc->sc_cmd_cur + 1) % IWN_CMD_MAXOPS;
 		IWN_CMD_UNLOCK(sc);
 
+		IWN_LOCK(sc);		/* NB: sync debug printfs on smp */
 		DPRINTF(sc, IWN_DEBUG_OPS, "%s: %s (cmd 0x%x)\n",
 		    __func__, iwn_ops_str(cmd), cmd);
 
 		vap = TAILQ_FIRST(&ic->ic_vaps);	/* XXX */
 		switch (cmd) {
 		case IWN_SCAN_START:
-			IWN_LOCK(sc);
 			/* make the link LED blink while we're scanning */
 			iwn_set_led(sc, IWN_LED_LINK, 20, 2);
-			IWN_UNLOCK(sc);
 			break;
 		case IWN_SCAN_STOP:
 			break;
@@ -4386,19 +4396,21 @@
 			ieee80211_scan_next(vap);
 			break;
 		case IWN_SCAN_CURCHAN:
-			IWN_LOCK(sc);
 			error = iwn_scan(sc);
-			IWN_UNLOCK(sc);
-			if (error != 0)
+			if (error != 0) {
+				IWN_UNLOCK(sc);
 				ieee80211_cancel_scan(vap);
+				IWN_LOCK(sc);
+				return;
+			}
 			break;
 		case IWN_SET_CHAN:
-			IWN_LOCK(sc);
 			error = iwn_config(sc);
-			IWN_UNLOCK(sc);
 			if (error != 0) {
 				DPRINTF(sc, IWN_DEBUG_STATE,
-				    "%s: cancel scan\n", __func__);
+				    "%s: set chan failed, cancel scan\n",
+				    __func__);
+				IWN_UNLOCK(sc);
 				//XXX Handle failed scan correctly
 				ieee80211_cancel_scan(vap);
 				return;
@@ -4406,7 +4418,6 @@
 			break;
 		case IWN_AUTH:
 		case IWN_RUN:
-			IWN_LOCK(sc);
 			if (cmd == IWN_AUTH) {
 				error = iwn_auth(sc);
 				nstate = IEEE80211_S_AUTH;
@@ -4414,13 +4425,14 @@
 				error = iwn_run(sc);
 				nstate = IEEE80211_S_RUN;
 			}
-			IWN_UNLOCK(sc);
 			if (error == 0) {
+				IWN_UNLOCK(sc);
 				IEEE80211_LOCK(ic);
 				IWN_VAP(vap)->iv_newstate(vap, nstate, arg);
 				if (vap->iv_newstate_cb != NULL)
 					vap->iv_newstate_cb(vap, nstate, arg);
 				IEEE80211_UNLOCK(ic);
+				IWN_LOCK(sc);
 			} else {
 				device_printf(sc->sc_dev,
 				    "%s: %s state change failed, error %d\n",
@@ -4431,7 +4443,7 @@
 		case IWN_REINIT:
 			//XXX DEBUG
 			break;
-			iwn_stop(sc);
+			iwn_stop_locked(sc);
 			iwn_init(sc);
 			break;
 		case IWN_RADIO_ENABLE:
@@ -4440,9 +4452,10 @@
 			iwn_init(sc);
 			break;
 		case IWN_RADIO_DISABLE:
-			iwn_stop(sc);
+			iwn_stop_locked(sc);
 			break;
 		}
+		IWN_UNLOCK(sc);
 	}
 }
 



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