PERFORCE change 150048 for review

Hans Petter Selasky hselasky at FreeBSD.org
Thu Sep 18 20:46:41 UTC 2008


http://perforce.freebsd.org/chv.cgi?CH=150048

Change 150048 by hselasky at hselasky_laptop001 on 2008/09/18 20:45:48

	
	Fix a synchronisation issue in USB WLAN drivers.

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_config_td.c#9 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_config_td.h#4 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_process.c#9 edit
.. //depot/projects/usb/src/sys/dev/usb2/wlan/if_rum2.c#13 edit
.. //depot/projects/usb/src/sys/dev/usb2/wlan/if_ural2.c#13 edit
.. //depot/projects/usb/src/sys/dev/usb2/wlan/if_zyd2.c#14 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_config_td.c#9 (text+ko) ====

@@ -33,6 +33,8 @@
 #include <dev/usb2/core/usb2_config_td.h>
 #include <dev/usb2/core/usb2_debug.h>
 
+static void usb2_config_td_sync_cb(struct usb2_config_td_softc *sc, struct usb2_config_td_cc *cc, uint16_t ref);
+
 static void
 usb2_config_td_dispatch(struct usb2_proc_msg *pm)
 {
@@ -146,21 +148,23 @@
  *	usb2_config_td_queue_command
  *
  * This function will enter a command into the config thread queue for
- * execution. The "command_qcount" gives the maximum number of
- * equivalent commands that will be kept on the queue before queueing
- * the next command. "command_ref" is the reference count for the
- * current command which is passed on to the "command_post_func"
+ * execution. The "command_sync" field was previously used to indicate
+ * the queue count which is now fixed at two elements. If the
+ * "command_sync" field is equal to "USB2_CONFIG_TD_SYNC" the command
+ * will be executed synchronously from the config thread.  The
+ * "command_ref" argument is the reference count for the current
+ * command which is passed on to the "command_post_func"
  * function. This parameter can be used to make a command
  * unique. "command_pre_func" is called from this function when we
  * have the final queue element. "command_post_func" is called from
  * the USB config thread when the command reaches the beginning of the
- * USB config thread queue.
+ * USB config thread queue. This function must be called locked.
  *------------------------------------------------------------------------*/
 void
 usb2_config_td_queue_command(struct usb2_config_td *ctd,
     usb2_config_td_command_t *command_pre_func,
     usb2_config_td_command_t *command_post_func,
-    uint16_t command_qcount,
+    uint16_t command_sync,
     uint16_t command_ref)
 {
 	struct usb2_config_td_item *pi;
@@ -227,6 +231,10 @@
 	if (command_pre_func) {
 		(command_pre_func) (ctd->p_softc, (void *)(pi + 1), command_ref);
 	}
+
+	if (command_sync == USB2_CONFIG_TD_SYNC) {
+		usb2_proc_mwait(&ctd->usb2_proc, pi_0, pi_1);
+	}
 	return;
 }
 
@@ -278,3 +286,38 @@
 done:
 	return (is_gone);
 }
+
+/*------------------------------------------------------------------------*
+ *	usb2_config_td_sync
+ *
+ * This function will wait until all commands have been executed on
+ * the config thread.  This function must be called locked and can
+ * sleep.
+ *
+ * Return values:
+ *    0: success
+ * Else: config thread is gone
+ *------------------------------------------------------------------------*/
+uint8_t
+usb2_config_td_sync(struct usb2_config_td *ctd)
+{
+  if (usb2_config_td_is_gone(ctd)) {
+	return (1);
+  }
+
+  usb2_config_td_queue_command(ctd, NULL, 
+       &usb2_config_td_sync_cb, USB2_CONFIG_TD_SYNC, 0);
+
+  if (usb2_config_td_is_gone(ctd)) {
+	return (1);
+  }
+
+  return (0);
+}
+
+static void
+usb2_config_td_sync_cb(struct usb2_config_td_softc *sc, 
+    struct usb2_config_td_cc *cc, uint16_t ref)
+{
+	return;
+}

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_config_td.h#4 (text+ko) ====

@@ -30,6 +30,8 @@
 struct usb2_config_td_softc;
 struct usb2_config_td_cc;
 
+#define USB2_CONFIG_TD_SYNC 0xFFFF /* magic value */
+
 typedef void (usb2_config_td_command_t)(struct usb2_config_td_softc *sc, struct usb2_config_td_cc *cc, uint16_t reference);
 typedef void (usb2_config_td_end_of_commands_t)(struct usb2_config_td_softc *sc);
 
@@ -61,8 +63,9 @@
 uint8_t	usb2_config_td_setup(struct usb2_config_td *ctd, void *priv_sc, struct mtx *priv_mtx, usb2_config_td_end_of_commands_t *p_func_eoc, uint16_t item_size, uint16_t item_count);
 void	usb2_config_td_drain(struct usb2_config_td *ctd);
 void	usb2_config_td_unsetup(struct usb2_config_td *ctd);
-void	usb2_config_td_queue_command(struct usb2_config_td *ctd, usb2_config_td_command_t *pre_func, usb2_config_td_command_t *post_func, uint16_t command_qcount, uint16_t command_ref);
+void	usb2_config_td_queue_command(struct usb2_config_td *ctd, usb2_config_td_command_t *pre_func, usb2_config_td_command_t *post_func, uint16_t command_sync, uint16_t command_ref);
 uint8_t	usb2_config_td_is_gone(struct usb2_config_td *ctd);
 uint8_t	usb2_config_td_sleep(struct usb2_config_td *ctd, uint32_t timeout);
+uint8_t	usb2_config_td_sync(struct usb2_config_td *ctd);
 
 #endif					/* _USB2_CONFIG_TD_H_ */

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_process.c#9 (text+ko) ====

@@ -147,6 +147,7 @@
 
 			continue;
 		}
+		/* end if messages - check if anyone is waiting for sync */
 		if (up->up_dsleep) {
 			up->up_dsleep = 0;
 			usb2_cv_broadcast(&up->up_drain);
@@ -328,7 +329,8 @@
  *	usb2_proc_mwait
  *
  * This function will return when the USB process message pointed to
- * by "pm" is no longer on a queue.
+ * by "pm" is no longer on a queue. This function must be called
+ * having "up->up_mtx" locked.
  *------------------------------------------------------------------------*/
 void
 usb2_proc_mwait(struct usb2_process *up, void *_pm0, void *_pm1)
@@ -336,7 +338,8 @@
 	struct usb2_proc_msg *pm0 = _pm0;
 	struct usb2_proc_msg *pm1 = _pm1;
 
-	mtx_lock(up->up_mtx);
+	mtx_assert(up->up_mtx, MA_OWNED);
+
 	if (up->up_curtd == curthread) {
 		/* Just remove the messages from the queue. */
 		if (pm0->pm_qentry.tqe_prev) {
@@ -347,12 +350,14 @@
 			TAILQ_REMOVE(&up->up_qhead, pm1, pm_qentry);
 			pm1->pm_qentry.tqe_prev = NULL;
 		}
-	} else if (pm0->pm_qentry.tqe_prev ||
+	} else while (pm0->pm_qentry.tqe_prev ||
 	    pm1->pm_qentry.tqe_prev) {
+		/* check if config thread is gone */
+		if (up->up_gone)
+			break;
 		up->up_dsleep = 1;
 		usb2_cv_wait(&up->up_drain, up->up_mtx);
 	}
-	mtx_unlock(up->up_mtx);
 	return;
 }
 
@@ -390,11 +395,6 @@
 			up->up_csleep = 0;
 			usb2_cv_signal(&up->up_cv);
 		}
-		/* Check if someone is waiting - should not happen */
-
-		if (up->up_dsleep) {
-			printf("WARNING: Someone is waiting for USB process drain!\n");
-		}
 		/* Check if we are still cold booted */
 
 		if (cold) {
@@ -404,6 +404,14 @@
 		}
 		usb2_cv_wait(&up->up_cv, up->up_mtx);
 	}
+	/* Check if someone is waiting - should not happen */
+
+	if (up->up_dsleep) {
+		up->up_dsleep = 0;
+		usb2_cv_broadcast(&up->up_drain);
+		DPRINTF("WARNING: Someone is waiting "
+		  "for USB process drain!\n");
+	}
 	mtx_unlock(up->up_mtx);
 	return;
 }

==== //depot/projects/usb/src/sys/dev/usb2/wlan/if_rum2.c#13 (text+ko) ====

@@ -2559,6 +2559,16 @@
 {
 	struct rum_vap *rvp;
 	struct ieee80211vap *vap;
+	struct rum_softc *sc = ic->ic_ifp->if_softc;
+
+	/* Need to sync with config thread: */
+	mtx_lock(&sc->sc_mtx);
+	if (usb2_config_td_sync(&sc->sc_config_td)) {
+		mtx_unlock(&sc->sc_mtx);
+		/* config thread is gone */
+		return (NULL);
+	}
+	mtx_unlock(&sc->sc_mtx);
 
 	if (!TAILQ_EMPTY(&ic->ic_vaps))	/* only one at a time */
 		return NULL;
@@ -2592,6 +2602,14 @@
 rum_vap_delete(struct ieee80211vap *vap)
 {
 	struct rum_vap *rvp = RUM_VAP(vap);
+	struct rum_softc *sc = vap->iv_ic->ic_ifp->if_softc;
+
+	/* Need to sync with config thread: */
+	mtx_lock(&sc->sc_mtx);
+	if (usb2_config_td_sync(&sc->sc_config_td)) {
+		/* ignore */
+	}
+	mtx_unlock(&sc->sc_mtx);
 
 	ieee80211_amrr_cleanup(&rvp->amrr);
 	ieee80211_vap_detach(vap);

==== //depot/projects/usb/src/sys/dev/usb2/wlan/if_ural2.c#13 (text+ko) ====

@@ -2351,6 +2351,16 @@
 {
 	struct ural_vap *uvp;
 	struct ieee80211vap *vap;
+	struct ural_softc *sc = ic->ic_ifp->if_softc;
+
+	/* Need to sync with config thread: */
+	mtx_lock(&sc->sc_mtx);
+	if (usb2_config_td_sync(&sc->sc_config_td)) {
+		mtx_unlock(&sc->sc_mtx);
+		/* config thread is gone */
+		return (NULL);
+	}
+	mtx_unlock(&sc->sc_mtx);
 
 	if (!TAILQ_EMPTY(&ic->ic_vaps))	/* only one at a time */
 		return NULL;
@@ -2385,6 +2395,14 @@
 ural_vap_delete(struct ieee80211vap *vap)
 {
 	struct ural_vap *uvp = URAL_VAP(vap);
+	struct ural_softc *sc = vap->iv_ic->ic_ifp->if_softc;
+
+	/* Need to sync with config thread: */
+	mtx_lock(&sc->sc_mtx);
+	if (usb2_config_td_sync(&sc->sc_config_td)) {
+		/* ignore */
+	}
+	mtx_unlock(&sc->sc_mtx);
 
 	ieee80211_amrr_cleanup(&uvp->amrr);
 	ieee80211_vap_detach(vap);

==== //depot/projects/usb/src/sys/dev/usb2/wlan/if_zyd2.c#14 (text+ko) ====

@@ -3003,6 +3003,16 @@
 {
 	struct zyd_vap *zvp;
 	struct ieee80211vap *vap;
+	struct zyd_softc *sc = ic->ic_ifp->if_softc;
+
+	/* Need to sync with config thread: */
+	mtx_lock(&sc->sc_mtx);
+	if (usb2_config_td_sync(&sc->sc_config_td)) {
+		mtx_unlock(&sc->sc_mtx);
+		/* config thread is gone */
+		return (NULL);
+	}
+	mtx_unlock(&sc->sc_mtx);
 
 	if (!TAILQ_EMPTY(&ic->ic_vaps))	/* only one at a time */
 		return NULL;
@@ -3034,6 +3044,14 @@
 zyd_vap_delete(struct ieee80211vap *vap)
 {
 	struct zyd_vap *zvp = ZYD_VAP(vap);
+	struct zyd_softc *sc = vap->iv_ic->ic_ifp->if_softc;
+
+	/* Need to sync with config thread: */
+	mtx_lock(&sc->sc_mtx);
+	if (usb2_config_td_sync(&sc->sc_config_td)) {
+		/* ignore */
+	}
+	mtx_unlock(&sc->sc_mtx);
 
 	ieee80211_amrr_cleanup(&zvp->amrr);
 	ieee80211_vap_detach(vap);


More information about the p4-projects mailing list