tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

usb_rem_task_wait



The attached patch implements usb_rem_task_wait and uses it in various
drivers' detach routines so that they have a chance of guaranteeing
not to leave pending tasks floating around using memory after free.
Also changes various callout_stops near usb_rem_task to callout_halt
if they seemed relevant in 30sec of analysis.

Compile-tested only so far.  Run-testers invited.  Comments?
diff --git a/sys/dev/usb/if_athn_usb.c b/sys/dev/usb/if_athn_usb.c
index 7861fc3f6dc9..8c889fb54953 100644
--- a/sys/dev/usb/if_athn_usb.c
+++ b/sys/dev/usb/if_athn_usb.c
@@ -335,7 +335,7 @@ athn_usb_attach(device_t parent, device_t self, void *aux)
 	athn_usb_free_tx_cmd(usc);
 	athn_usb_free_tx_msg(usc);
 	athn_usb_close_pipes(usc);
-	usb_rem_task(usc->usc_udev, &usc->usc_task);
+	usb_rem_task_wait(usc->usc_udev, &usc->usc_task, USB_TASKQ_DRIVER);
 
 	cv_destroy(&usc->usc_cmd_cv);
 	cv_destroy(&usc->usc_msg_cv);
@@ -501,7 +501,7 @@ athn_usb_detach(device_t self, int flags)
 
 	athn_usb_wait_async(usc);
 
-	usb_rem_task(usc->usc_udev, &usc->usc_task);
+	usb_rem_task_wait(usc->usc_udev, &usc->usc_task, USB_TASKQ_DRIVER);
 
 	/* Abort Tx/Rx pipes. */
 	athn_usb_abort_pipes(usc);
diff --git a/sys/dev/usb/if_atu.c b/sys/dev/usb/if_atu.c
index f526038a0037..87b369c4e0b8 100644
--- a/sys/dev/usb/if_atu.c
+++ b/sys/dev/usb/if_atu.c
@@ -2235,7 +2235,7 @@ atu_stop(struct ifnet *ifp, int disable)
 	ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
 	ifp->if_timer = 0;
 
-	usb_rem_task(sc->atu_udev, &sc->sc_task);
+	usb_rem_task_wait(sc->atu_udev, &sc->sc_task, USB_TASKQ_DRIVER);
 	ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
 
 	/* Stop transfers. */
diff --git a/sys/dev/usb/if_aue.c b/sys/dev/usb/if_aue.c
index 56f872141dec..309df518d005 100644
--- a/sys/dev/usb/if_aue.c
+++ b/sys/dev/usb/if_aue.c
@@ -886,13 +886,16 @@ aue_detach(device_t self, int flags)
 		return 0;
 	}
 
-	callout_stop(&sc->aue_stat_ch);
 	/*
-	 * Remove any pending tasks.  They cannot be executing because they run
-	 * in the same thread as detach.
+	 * XXX Halting callout guarantees no more tick tasks.  What
+	 * guarantees no more stop tasks?  What guarantees no more
+	 * calls to aue_send?  Don't we need to wait for if_detach or
+	 * something?  Should we set sc->aue_dying here?  Is device
+	 * deactivation guaranteed to have already happened?
 	 */
-	usb_rem_task(sc->aue_udev, &sc->aue_tick_task);
-	usb_rem_task(sc->aue_udev, &sc->aue_stop_task);
+	callout_halt(&sc->aue_stat_ch, NULL);
+	usb_rem_task_wait(sc->aue_udev, &sc->aue_tick_task, USB_TASKQ_DRIVER);
+	usb_rem_task_wait(sc->aue_udev, &sc->aue_stop_task, USB_TASKQ_DRIVER);
 
 	sc->aue_closing = 1;
 	cv_signal(&sc->aue_domc);
diff --git a/sys/dev/usb/if_axe.c b/sys/dev/usb/if_axe.c
index 35ee22469064..a8b5c0b94875 100644
--- a/sys/dev/usb/if_axe.c
+++ b/sys/dev/usb/if_axe.c
@@ -1111,11 +1111,8 @@ axe_detach(device_t self, int flags)
 	if (sc->axe_ep[AXE_ENDPT_INTR] != NULL)
 		usbd_abort_pipe(sc->axe_ep[AXE_ENDPT_INTR]);
 
-	/*
-	 * Remove any pending tasks.  They cannot be executing because they run
-	 * in the same thread as detach.
-	 */
-	usb_rem_task(sc->axe_udev, &sc->axe_tick_task);
+	callout_halt(&sc->axe_stat_ch, NULL);
+	usb_rem_task_wait(sc->axe_udev, &sc->axe_tick_task, USB_TASKQ_DRIVER);
 
 	s = splusb();
 
diff --git a/sys/dev/usb/if_axen.c b/sys/dev/usb/if_axen.c
index 571d5cdce7d8..379b85ed2230 100644
--- a/sys/dev/usb/if_axen.c
+++ b/sys/dev/usb/if_axen.c
@@ -826,11 +826,9 @@ axen_detach(device_t self, int flags)
 
 	sc->axen_dying = true;
 
-	/*
-	 * Remove any pending tasks.  They cannot be executing because they run
-	 * in the same thread as detach.
-	 */
-	usb_rem_task(sc->axen_udev, &sc->axen_tick_task);
+	callout_halt(&sc->axen_stat_ch, NULL);
+	usb_rem_task_wait(sc->axen_udev, &sc->axen_tick_task,
+	    USB_TASKQ_DRIVER);
 
 	s = splusb();
 
diff --git a/sys/dev/usb/if_cue.c b/sys/dev/usb/if_cue.c
index 7a91adb6a121..3eb4856329d0 100644
--- a/sys/dev/usb/if_cue.c
+++ b/sys/dev/usb/if_cue.c
@@ -571,13 +571,16 @@ cue_detach(device_t self, int flags)
 
 	DPRINTFN(2,("%s: %s: enter\n", device_xname(sc->cue_dev), __func__));
 
-	callout_stop(&sc->cue_stat_ch);
 	/*
-	 * Remove any pending task.  It cannot be executing because it run
-	 * in the same thread as detach.
+	 * XXX Halting callout guarantees no more tick tasks.  What
+	 * guarantees no more stop tasks?  What guarantees no more
+	 * calls to cue_send?  Don't we need to wait for if_detach or
+	 * something?  Should we set sc->cue_dying here?  Is device
+	 * deactivation guaranteed to have already happened?
 	 */
-	usb_rem_task(sc->cue_udev, &sc->cue_tick_task);
-	usb_rem_task(sc->cue_udev, &sc->cue_stop_task);
+	callout_halt(&sc->cue_stat_ch, NULL);
+	usb_rem_task_wait(sc->cue_udev, &sc->cue_tick_task, USB_TASKQ_DRIVER);
+	usb_rem_task_wait(sc->cue_udev, &sc->cue_stop_task, USB_TASKQ_DRIVER);
 
 	if (!sc->cue_attached) {
 		/* Detached before attached finished, so just bail out. */
diff --git a/sys/dev/usb/if_otus.c b/sys/dev/usb/if_otus.c
index 809d3a3e08c1..0876d4d641d1 100644
--- a/sys/dev/usb/if_otus.c
+++ b/sys/dev/usb/if_otus.c
@@ -701,7 +701,7 @@ otus_detach(device_t self, int flags)
 	if (ifp != NULL)	/* Failed to attach properly */
 		otus_stop(ifp);
 
-	usb_rem_task(sc->sc_udev, &sc->sc_task);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER);
 	callout_destroy(&sc->sc_scan_to);
 	callout_destroy(&sc->sc_calib_to);
 
diff --git a/sys/dev/usb/if_rum.c b/sys/dev/usb/if_rum.c
index f38f9b914c42..e14483f588aa 100644
--- a/sys/dev/usb/if_rum.c
+++ b/sys/dev/usb/if_rum.c
@@ -496,9 +496,9 @@ rum_detach(device_t self, int flags)
 	s = splusb();
 
 	rum_stop(ifp, 1);
-	usb_rem_task(sc->sc_udev, &sc->sc_task);
-	callout_stop(&sc->sc_scan_ch);
-	callout_stop(&sc->sc_amrr_ch);
+	callout_halt(&sc->sc_scan_ch, NULL);
+	callout_halt(&sc->sc_amrr_ch, NULL);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER);
 
 	bpf_detach(ifp);
 	ieee80211_ifdetach(ic);	/* free all nodes */
@@ -735,6 +735,11 @@ rum_newstate(struct ieee80211com *ic, enum ieee80211_state nstate, int arg)
 {
 	struct rum_softc *sc = ic->ic_ifp->if_softc;
 
+	/*
+	 * XXXSMP: This does not wait for the task, if it is in flight,
+	 * to complete.  If this code works at all, it must rely on the
+	 * kernel lock to serialize with the USB task thread.
+	 */
 	usb_rem_task(sc->sc_udev, &sc->sc_task);
 	callout_stop(&sc->sc_scan_ch);
 	callout_stop(&sc->sc_amrr_ch);
diff --git a/sys/dev/usb/if_run.c b/sys/dev/usb/if_run.c
index 2207549804ab..abddf1b4e505 100644
--- a/sys/dev/usb/if_run.c
+++ b/sys/dev/usb/if_run.c
@@ -759,8 +759,10 @@ run_detach(device_t self, int flags)
 	sc->sc_flags |= RUN_DETACHING;
 
 	if (ifp->if_flags & IFF_RUNNING) {
-		usb_rem_task(sc->sc_udev, &sc->sc_task);
 		run_stop(ifp, 0);
+		callout_halt(&sc->scan_to, NULL);
+		callout_halt(&sc->calib_to, NULL);
+		usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER);
 	}
 
 	ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
diff --git a/sys/dev/usb/if_smsc.c b/sys/dev/usb/if_smsc.c
index 43c9b3e4a5a0..bb582ab822b3 100644
--- a/sys/dev/usb/if_smsc.c
+++ b/sys/dev/usb/if_smsc.c
@@ -1140,7 +1140,7 @@ smsc_detach(device_t self, int flags)
 	struct ifnet *ifp = &sc->sc_ec.ec_if;
 	int s;
 
-	callout_stop(&sc->sc_stat_ch);
+	callout_halt(&sc->sc_stat_ch, NULL);
 
 	if (sc->sc_ep[SMSC_ENDPT_TX] != NULL)
 		usbd_abort_pipe(sc->sc_ep[SMSC_ENDPT_TX]);
@@ -1149,12 +1149,8 @@ smsc_detach(device_t self, int flags)
 	if (sc->sc_ep[SMSC_ENDPT_INTR] != NULL)
 		usbd_abort_pipe(sc->sc_ep[SMSC_ENDPT_INTR]);
 
-	/*
-	 * Remove any pending tasks.  They cannot be executing because they run
-	 * in the same thread as detach.
-	 */
-	usb_rem_task(sc->sc_udev, &sc->sc_tick_task);
-	usb_rem_task(sc->sc_udev, &sc->sc_stop_task);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_tick_task, USB_TASKQ_DRIVER);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_stop_task, USB_TASKQ_DRIVER);
 
 	s = splusb();
 
diff --git a/sys/dev/usb/if_udav.c b/sys/dev/usb/if_udav.c
index 84afa2502960..175301d3ab08 100644
--- a/sys/dev/usb/if_udav.c
+++ b/sys/dev/usb/if_udav.c
@@ -352,11 +352,11 @@ udav_detach(device_t self, int flags)
 	if (!sc->sc_attached)
 		return 0;
 
-	callout_stop(&sc->sc_stat_ch);
+	callout_halt(&sc->sc_stat_ch, NULL);
 
 	/* Remove any pending tasks */
-	usb_rem_task(sc->sc_udev, &sc->sc_tick_task);
-	usb_rem_task(sc->sc_udev, &sc->sc_stop_task);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_tick_task, USB_TASKQ_DRIVER);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_stop_task, USB_TASKQ_DRIVER);
 
 	s = splusb();
 
diff --git a/sys/dev/usb/if_upgt.c b/sys/dev/usb/if_upgt.c
index 21da0680be09..dc92c446a7b3 100644
--- a/sys/dev/usb/if_upgt.c
+++ b/sys/dev/usb/if_upgt.c
@@ -504,8 +504,10 @@ upgt_detach(device_t self, int flags)
 		upgt_stop(sc);
 
 	/* remove tasks and timeouts */
-	usb_rem_task(sc->sc_udev, &sc->sc_task_newstate);
-	usb_rem_task(sc->sc_udev, &sc->sc_task_tx);
+	callout_halt(&sc->scan_to, NULL);
+	callout_halt(&sc->led_to, NULL);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_task_newstate, USB_TASKQ_DRIVER);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_task_tx, USB_TASKQ_DRIVER);
 	callout_destroy(&sc->scan_to);
 	callout_destroy(&sc->led_to);
 
@@ -1346,6 +1348,11 @@ upgt_newstate(struct ieee80211com *ic, enum ieee80211_state nstate, int arg)
 {
 	struct upgt_softc *sc = ic->ic_ifp->if_softc;
 
+	/*
+	 * XXXSMP: This does not wait for the task, if it is in flight,
+	 * to complete.  If this code works at all, it must rely on the
+	 * kernel lock to serialize with the USB task thread.
+	 */
 	usb_rem_task(sc->sc_udev, &sc->sc_task_newstate);
 	callout_stop(&sc->scan_to);
 
diff --git a/sys/dev/usb/if_ural.c b/sys/dev/usb/if_ural.c
index 9ea1dc1b6e0c..93f42769c329 100644
--- a/sys/dev/usb/if_ural.c
+++ b/sys/dev/usb/if_ural.c
@@ -534,9 +534,9 @@ ural_detach(device_t self, int flags)
 	s = splusb();
 
 	ural_stop(ifp, 1);
-	usb_rem_task(sc->sc_udev, &sc->sc_task);
-	callout_stop(&sc->sc_scan_ch);
-	callout_stop(&sc->sc_amrr_ch);
+	callout_halt(&sc->sc_scan_ch, NULL);
+	callout_halt(&sc->sc_amrr_ch, NULL);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER);
 
 	bpf_detach(ifp);
 	ieee80211_ifdetach(ic);
@@ -784,6 +784,11 @@ ural_newstate(struct ieee80211com *ic, enum ieee80211_state nstate,
 {
 	struct ural_softc *sc = ic->ic_ifp->if_softc;
 
+	/*
+	 * XXXSMP: This does not wait for the task, if it is in flight,
+	 * to complete.  If this code works at all, it must rely on the
+	 * kernel lock to serialize with the USB task thread.
+	 */
 	usb_rem_task(sc->sc_udev, &sc->sc_task);
 	callout_stop(&sc->sc_scan_ch);
 	callout_stop(&sc->sc_amrr_ch);
diff --git a/sys/dev/usb/if_url.c b/sys/dev/usb/if_url.c
index c56b1278de62..15a7fa775a12 100644
--- a/sys/dev/usb/if_url.c
+++ b/sys/dev/usb/if_url.c
@@ -346,11 +346,18 @@ url_detach(device_t self, int flags)
 	if (!sc->sc_attached)
 		return 0;
 
-	callout_stop(&sc->sc_stat_ch);
+	/*
+	 * XXX Halting callout guarantees no more tick tasks.  What
+	 * guarantees no more stop tasks?  What guarantees no more
+	 * calls to url_send?  Don't we need to wait for if_detach or
+	 * something?  Should set sc->sc_dying here?  Is device
+	 * deactivation guaranteed to have already happened?
+	 */
+	callout_halt(&sc->sc_stat_ch, NULL);
 
 	/* Remove any pending tasks */
-	usb_rem_task(sc->sc_udev, &sc->sc_tick_task);
-	usb_rem_task(sc->sc_udev, &sc->sc_stop_task);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_tick_task, USB_TASKQ_DRIVER);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_stop_task, USB_TASKQ_DRIVER);
 
 	s = splusb();
 
diff --git a/sys/dev/usb/if_urtw.c b/sys/dev/usb/if_urtw.c
index f8c2b51c7978..8572b25abf54 100644
--- a/sys/dev/usb/if_urtw.c
+++ b/sys/dev/usb/if_urtw.c
@@ -774,11 +774,13 @@ urtw_detach(device_t self, int flags)
 
 	sc->sc_dying = true;
 
+	callout_halt(&sc->scan_to, NULL);
+	callout_halt(&sc->sc_led_ch, NULL);
 	callout_destroy(&sc->scan_to);
 	callout_destroy(&sc->sc_led_ch);
 
-	usb_rem_task(sc->sc_udev, &sc->sc_task);
-	usb_rem_task(sc->sc_udev, &sc->sc_ledtask);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_ledtask, USB_TASKQ_DRIVER);
 
 	if (ifp->if_softc != NULL) {
 		bpf_detach(ifp);
@@ -1042,6 +1044,11 @@ urtw_newstate(struct ieee80211com *ic, enum ieee80211_state nstate, int arg)
 {
 	struct urtw_softc *sc = ic->ic_ifp->if_softc;
 
+	/*
+	 * XXXSMP: This does not wait for the task, if it is in flight,
+	 * to complete.  If this code works at all, it must rely on the
+	 * kernel lock to serialize with the USB task thread.
+	 */
 	usb_rem_task(sc->sc_udev, &sc->sc_task);
 	callout_stop(&sc->scan_to);
 
diff --git a/sys/dev/usb/if_urtwn.c b/sys/dev/usb/if_urtwn.c
index 2a104787a9a0..6f55a881841d 100644
--- a/sys/dev/usb/if_urtwn.c
+++ b/sys/dev/usb/if_urtwn.c
@@ -539,12 +539,12 @@ urtwn_detach(device_t self, int flags)
 
 	sc->sc_dying = 1;
 
-	callout_stop(&sc->sc_scan_to);
-	callout_stop(&sc->sc_calib_to);
+	callout_halt(&sc->sc_scan_to, NULL);
+	callout_halt(&sc->sc_calib_to, NULL);
 
 	if (ISSET(sc->sc_flags, URTWN_FLAG_ATTACHED)) {
-		usb_rem_task(sc->sc_udev, &sc->sc_task);
 		urtwn_stop(ifp, 0);
+		usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER);
 
 		ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
 		bpf_detach(ifp);
diff --git a/sys/dev/usb/if_zyd.c b/sys/dev/usb/if_zyd.c
index e45a50f71d76..04123de313b8 100644
--- a/sys/dev/usb/if_zyd.c
+++ b/sys/dev/usb/if_zyd.c
@@ -466,9 +466,9 @@ zyd_detach(device_t self, int flags)
 	s = splusb();
 
 	zyd_stop(ifp, 1);
-	usb_rem_task(sc->sc_udev, &sc->sc_task);
-	callout_stop(&sc->sc_scan_ch);
-	callout_stop(&sc->sc_amrr_ch);
+	callout_halt(&sc->sc_scan_ch, NULL);
+	callout_halt(&sc->sc_amrr_ch, NULL);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER);
 
 	/* Abort, etc. done by zyd_stop */
 	zyd_close_pipes(sc);
@@ -761,6 +761,11 @@ zyd_newstate(struct ieee80211com *ic, enum ieee80211_state nstate, int arg)
 	if (!sc->attached)
 		return ENXIO;
 
+	/*
+	 * XXXSMP: This does not wait for the task, if it is in flight,
+	 * to complete.  If this code works at all, it must rely on the
+	 * kernel lock to serialize with the USB task thread.
+	 */
 	usb_rem_task(sc->sc_udev, &sc->sc_task);
 	callout_stop(&sc->sc_scan_ch);
 	callout_stop(&sc->sc_amrr_ch);
diff --git a/sys/dev/usb/uatp.c b/sys/dev/usb/uatp.c
index fcbbc6eac23e..cda7c802658c 100644
--- a/sys/dev/usb/uatp.c
+++ b/sys/dev/usb/uatp.c
@@ -1362,7 +1362,8 @@ geyser34_finalize(struct uatp_softc *sc)
 {
 
 	DPRINTF(sc, UATP_DEBUG_MISC, ("finalizing\n"));
-	usb_rem_task(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_reset_task);
+	usb_rem_task_wait(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_reset_task,
+	    USB_TASKQ_DRIVER);
 
 	return 0;
 }
diff --git a/sys/dev/usb/umcs.c b/sys/dev/usb/umcs.c
index c4ad74c6e355..b8f20401189d 100644
--- a/sys/dev/usb/umcs.c
+++ b/sys/dev/usb/umcs.c
@@ -517,7 +517,7 @@ umcs7840_detach(device_t self, int flags)
 		kmem_free(sc->sc_intr_buf, sc->sc_intr_buflen);
 		sc->sc_intr_pipe = NULL;
 	}
-	usb_rem_task(sc->sc_udev, &sc->sc_change_task);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_change_task, USB_TASKQ_DRIVER);
 
 	/* detach children */
 	for (i = 0; i < sc->sc_numports; i++) {
diff --git a/sys/dev/usb/usb.c b/sys/dev/usb/usb.c
index 5f691f8e76a4..dc4759ebb20c 100644
--- a/sys/dev/usb/usb.c
+++ b/sys/dev/usb/usb.c
@@ -146,6 +146,7 @@ struct usb_taskq {
 	kcondvar_t cv;
 	struct lwp *task_thread_lwp;
 	const char *name;
+	struct usb_task *current_task;
 };
 
 static struct usb_taskq usb_taskq[USB_NUM_TASKQS];
@@ -294,6 +295,7 @@ usb_once_init(void)
 		mutex_init(&taskq->lock, MUTEX_DEFAULT, IPL_USB);
 		cv_init(&taskq->cv, "usbtsk");
 		taskq->name = taskq_names[i];
+		taskq->current_task = NULL;
 		if (kthread_create(PRI_NONE, KTHREAD_MPSAFE, NULL,
 		    usb_task_thread, taskq, &taskq->task_thread_lwp,
 		    "%s", taskq->name)) {
@@ -426,8 +428,14 @@ usb_add_task(struct usbd_device *dev, struct usb_task *task, int queue)
 }
 
 /*
- * XXX This does not wait for completion!  Most uses need such an
- * operation.  Urgh...
+ * usb_rem_task(dev, task)
+ *
+ *	If task is queued to run, remove it from the queue.
+ *
+ *	Caller is _not_ guaranteed that the task is not running when
+ *	this is done.
+ *
+ *	Never sleeps.
  */
 void
 usb_rem_task(struct usbd_device *dev, struct usb_task *task)
@@ -449,6 +457,85 @@ usb_rem_task(struct usbd_device *dev, struct usb_task *task)
 	}
 }
 
+/*
+ * usb_taskq_wait(taskq, task)
+ *
+ *	Wait for taskq to finish executing task, if it is executing
+ *	task.  Caller must hold the taskq lock.
+ */
+static void
+usb_taskq_wait(struct usb_taskq *taskq, struct usb_task *task)
+{
+
+	KASSERT(mutex_owned(&taskq->lock));
+
+	while (taskq->current_task == task)
+		cv_wait(&taskq->cv, &taskq->lock);
+
+	KASSERT(taskq->current_task != task);
+}
+
+/*
+ * usb_rem_task_wait(dev, task, queue)
+ *
+ *	If task is scheduled to run, remove it from the queue.  If it
+ *	may have already begun to run, wait for it to complete.
+ *
+ *	Caller MUST guarantee that task will not be scheduled on a
+ *	_different_ queue, at least until after this returns.
+ *
+ *	If caller guarantees that task will not be scheduled on the
+ *	same queue before this returns, then caller is guaranteed that
+ *	the task is not running at all when this returns.
+ *
+ *	May sleep.
+ */
+void
+usb_rem_task_wait(struct usbd_device *dev, struct usb_task *task, int queue)
+{
+	struct usb_taskq *taskq;
+	int queue1;
+
+	USBHIST_FUNC(); USBHIST_CALLED(usbdebug);
+	ASSERT_SLEEPABLE();
+	KASSERT(0 <= queue);
+	KASSERT(queue < USB_NUM_TASKQS);
+
+	taskq = &usb_taskq[queue];
+
+	if ((queue1 = task->queue) == USB_NUM_TASKQS) {
+		/*
+		 * It is not on the queue, but it may have already run.
+		 * Wait for it.
+		 */
+		mutex_enter(&taskq->lock);
+		usb_taskq_wait(taskq, task);
+		mutex_exit(&taskq->lock);
+	} else {
+		/*
+		 * It may be on the queue (and not another one), but
+		 * the state may have changed by now because we don't
+		 * have the queue locked.  Lock and reload.
+		 */
+		KASSERTMSG(queue1 == queue,
+		    "task %p on q%d expected on q%d", task, queue1, queue);
+		mutex_enter(&taskq->lock);
+		queue1 = task->queue;
+		if (queue1 == queue) {
+			/* Still queued, not run.  Just remove it.  */
+			TAILQ_REMOVE(&taskq->tasks, task, next);
+			task->queue = USB_NUM_TASKQS;
+		} else {
+			/* Already ran.  Wait for it.  */
+			KASSERTMSG(queue1 == USB_NUM_TASKQS,
+			    "task %p on q%d expected on q%d",
+			    task, queue1, queue);
+			usb_taskq_wait(taskq, task);
+		}
+		mutex_exit(&taskq->lock);
+	}
+}
+
 void
 usb_event_thread(void *arg)
 {
@@ -517,6 +604,7 @@ usb_task_thread(void *arg)
 			mpsafe = ISSET(task->flags, USB_TASKQ_MPSAFE);
 			TAILQ_REMOVE(&taskq->tasks, task, next);
 			task->queue = USB_NUM_TASKQS;
+			taskq->current_task = task;
 			mutex_exit(&taskq->lock);
 
 			if (!mpsafe)
@@ -527,6 +615,10 @@ usb_task_thread(void *arg)
 				KERNEL_UNLOCK_ONE(curlwp);
 
 			mutex_enter(&taskq->lock);
+			KASSERTMSG(taskq->current_task == task,
+			    "somebody scribbled on usb taskq %p", taskq);
+			taskq->current_task = NULL;
+			cv_broadcast(&taskq->cv);
 		}
 	}
 	mutex_exit(&taskq->lock);
diff --git a/sys/dev/usb/usb_subr.c b/sys/dev/usb/usb_subr.c
index 4115f9932ab0..af41f6558100 100644
--- a/sys/dev/usb/usb_subr.c
+++ b/sys/dev/usb/usb_subr.c
@@ -823,7 +823,7 @@ usbd_kill_pipe(struct usbd_pipe *pipe)
 	usbd_lock_pipe(pipe);
 	pipe->up_methods->upm_close(pipe);
 	usbd_unlock_pipe(pipe);
-	usb_rem_task(pipe->up_dev, &pipe->up_async_task);
+	usb_rem_task_wait(pipe->up_dev, &pipe->up_async_task, USB_TASKQ_DRIVER);
 	pipe->up_endpoint->ue_refcnt--;
 	kmem_free(pipe, pipe->up_dev->ud_bus->ub_pipesize);
 }
diff --git a/sys/dev/usb/usbdi.h b/sys/dev/usb/usbdi.h
index 911d3f96a2c5..ae88d2c612f5 100644
--- a/sys/dev/usb/usbdi.h
+++ b/sys/dev/usb/usbdi.h
@@ -219,6 +219,7 @@ struct usb_task {
 
 void usb_add_task(struct usbd_device *, struct usb_task *, int);
 void usb_rem_task(struct usbd_device *, struct usb_task *);
+void usb_rem_task_wait(struct usbd_device *, struct usb_task *, int);
 #define usb_init_task(t, f, a, fl) ((t)->fun = (f), (t)->arg = (a), (t)->queue = USB_NUM_TASKQS, (t)->flags = (fl))
 
 struct usb_devno {


Home | Main Index | Thread Index | Old Index