From a4a55a0c86f3172ba57d76208a5810c06775a402 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 1 Aug 2018 15:10:53 +0000 Subject: [PATCH] Fix usb_rem_task_wait API. - Return whether it removed task from queue or not. . True if it was on the queue and we intercepted it before it ran. . False if we could not intercept it: either it wasn't queued, or it already ran. (Up to caller to distinguish these cases.) - Pass an optional interlock like callout_halt. While here, simplify. --- sys/dev/usb/if_athn_usb.c | 6 ++-- sys/dev/usb/if_atu.c | 2 +- sys/dev/usb/if_aue.c | 6 ++-- sys/dev/usb/if_axe.c | 3 +- sys/dev/usb/if_axen.c | 2 +- sys/dev/usb/if_cue.c | 6 ++-- sys/dev/usb/if_otus.c | 2 +- sys/dev/usb/if_rum.c | 2 +- sys/dev/usb/if_run.c | 3 +- sys/dev/usb/if_smsc.c | 6 ++-- sys/dev/usb/if_udav.c | 6 ++-- sys/dev/usb/if_upgt.c | 6 ++-- sys/dev/usb/if_ural.c | 2 +- sys/dev/usb/if_url.c | 6 ++-- sys/dev/usb/if_urtw.c | 5 +-- sys/dev/usb/if_urtwn.c | 3 +- sys/dev/usb/if_zyd.c | 2 +- sys/dev/usb/uatp.c | 2 +- sys/dev/usb/usb.c | 85 +++++++++++++++++++++-------------------------- sys/dev/usb/usb_subr.c | 3 +- sys/dev/usb/usbdi.h | 3 +- 21 files changed, 85 insertions(+), 76 deletions(-) diff --git a/sys/dev/usb/if_athn_usb.c b/sys/dev/usb/if_athn_usb.c index 75c4586962cb..df153c44fcd3 100644 --- a/sys/dev/usb/if_athn_usb.c +++ b/sys/dev/usb/if_athn_usb.c @@ -335,7 +335,8 @@ 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_wait(usc->usc_udev, &usc->usc_task, USB_TASKQ_DRIVER); + usb_rem_task_wait(usc->usc_udev, &usc->usc_task, USB_TASKQ_DRIVER, + NULL); cv_destroy(&usc->usc_cmd_cv); cv_destroy(&usc->usc_msg_cv); @@ -501,7 +502,8 @@ athn_usb_detach(device_t self, int flags) athn_usb_wait_async(usc); - usb_rem_task_wait(usc->usc_udev, &usc->usc_task, USB_TASKQ_DRIVER); + usb_rem_task_wait(usc->usc_udev, &usc->usc_task, USB_TASKQ_DRIVER, + NULL); /* 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 d431d4ea036e..491cb130ace2 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_wait(sc->atu_udev, &sc->sc_task, USB_TASKQ_DRIVER); + usb_rem_task_wait(sc->atu_udev, &sc->sc_task, USB_TASKQ_DRIVER, NULL); 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 54778583d514..79a4ed3e65b2 100644 --- a/sys/dev/usb/if_aue.c +++ b/sys/dev/usb/if_aue.c @@ -894,8 +894,10 @@ aue_detach(device_t self, int flags) * deactivation guaranteed to have already happened? */ 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); + usb_rem_task_wait(sc->aue_udev, &sc->aue_tick_task, USB_TASKQ_DRIVER, + NULL); + usb_rem_task_wait(sc->aue_udev, &sc->aue_stop_task, USB_TASKQ_DRIVER, + NULL); 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 ed6994c0f2e3..5dc5678ef45b 100644 --- a/sys/dev/usb/if_axe.c +++ b/sys/dev/usb/if_axe.c @@ -1112,7 +1112,8 @@ axe_detach(device_t self, int flags) usbd_abort_pipe(sc->axe_ep[AXE_ENDPT_INTR]); callout_halt(&sc->axe_stat_ch, NULL); - usb_rem_task_wait(sc->axe_udev, &sc->axe_tick_task, USB_TASKQ_DRIVER); + usb_rem_task_wait(sc->axe_udev, &sc->axe_tick_task, USB_TASKQ_DRIVER, + NULL); s = splusb(); diff --git a/sys/dev/usb/if_axen.c b/sys/dev/usb/if_axen.c index ea2b3f1b361c..9a36ca82d6ce 100644 --- a/sys/dev/usb/if_axen.c +++ b/sys/dev/usb/if_axen.c @@ -829,7 +829,7 @@ axen_detach(device_t self, int flags) callout_halt(&sc->axen_stat_ch, NULL); usb_rem_task_wait(sc->axen_udev, &sc->axen_tick_task, - USB_TASKQ_DRIVER); + USB_TASKQ_DRIVER, NULL); s = splusb(); diff --git a/sys/dev/usb/if_cue.c b/sys/dev/usb/if_cue.c index e36fc32ab13b..29cd4ccf98c6 100644 --- a/sys/dev/usb/if_cue.c +++ b/sys/dev/usb/if_cue.c @@ -579,8 +579,10 @@ cue_detach(device_t self, int flags) * deactivation guaranteed to have already happened? */ 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); + usb_rem_task_wait(sc->cue_udev, &sc->cue_tick_task, USB_TASKQ_DRIVER, + NULL); + usb_rem_task_wait(sc->cue_udev, &sc->cue_stop_task, USB_TASKQ_DRIVER, + NULL); 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 19e4ff4542a3..92f31cc5d541 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_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER, NULL); 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 6f606be3570e..b4533f962b73 100644 --- a/sys/dev/usb/if_rum.c +++ b/sys/dev/usb/if_rum.c @@ -498,7 +498,7 @@ rum_detach(device_t self, int flags) rum_stop(ifp, 1); 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER, NULL); bpf_detach(ifp); ieee80211_ifdetach(ic); /* free all nodes */ diff --git a/sys/dev/usb/if_run.c b/sys/dev/usb/if_run.c index 69d267923b90..f50e1066d81a 100644 --- a/sys/dev/usb/if_run.c +++ b/sys/dev/usb/if_run.c @@ -762,7 +762,8 @@ run_detach(device_t self, int flags) 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER, + NULL); } ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE); diff --git a/sys/dev/usb/if_smsc.c b/sys/dev/usb/if_smsc.c index 830e5989e763..47082721c039 100644 --- a/sys/dev/usb/if_smsc.c +++ b/sys/dev/usb/if_smsc.c @@ -1149,8 +1149,10 @@ smsc_detach(device_t self, int flags) if (sc->sc_ep[SMSC_ENDPT_INTR] != NULL) usbd_abort_pipe(sc->sc_ep[SMSC_ENDPT_INTR]); - 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_tick_task, USB_TASKQ_DRIVER, + NULL); + usb_rem_task_wait(sc->sc_udev, &sc->sc_stop_task, USB_TASKQ_DRIVER, + NULL); s = splusb(); diff --git a/sys/dev/usb/if_udav.c b/sys/dev/usb/if_udav.c index 1e14d52ea20a..c94a447e013b 100644 --- a/sys/dev/usb/if_udav.c +++ b/sys/dev/usb/if_udav.c @@ -355,8 +355,10 @@ udav_detach(device_t self, int flags) callout_halt(&sc->sc_stat_ch, NULL); /* Remove any pending tasks */ - 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_tick_task, USB_TASKQ_DRIVER, + NULL); + usb_rem_task_wait(sc->sc_udev, &sc->sc_stop_task, USB_TASKQ_DRIVER, + NULL); s = splusb(); diff --git a/sys/dev/usb/if_upgt.c b/sys/dev/usb/if_upgt.c index 4723fca6bba2..8f333e0d5119 100644 --- a/sys/dev/usb/if_upgt.c +++ b/sys/dev/usb/if_upgt.c @@ -506,8 +506,10 @@ upgt_detach(device_t self, int flags) /* remove tasks and timeouts */ 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task_newstate, USB_TASKQ_DRIVER, + NULL); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task_tx, USB_TASKQ_DRIVER, + NULL); callout_destroy(&sc->scan_to); callout_destroy(&sc->led_to); diff --git a/sys/dev/usb/if_ural.c b/sys/dev/usb/if_ural.c index 08692b01a0b3..40a2eee99ed8 100644 --- a/sys/dev/usb/if_ural.c +++ b/sys/dev/usb/if_ural.c @@ -536,7 +536,7 @@ ural_detach(device_t self, int flags) ural_stop(ifp, 1); 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER, NULL); bpf_detach(ifp); ieee80211_ifdetach(ic); diff --git a/sys/dev/usb/if_url.c b/sys/dev/usb/if_url.c index 93db57eb7804..449c19ea812c 100644 --- a/sys/dev/usb/if_url.c +++ b/sys/dev/usb/if_url.c @@ -356,8 +356,10 @@ url_detach(device_t self, int flags) callout_halt(&sc->sc_stat_ch, NULL); /* Remove any pending tasks */ - 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_tick_task, USB_TASKQ_DRIVER, + NULL); + usb_rem_task_wait(sc->sc_udev, &sc->sc_stop_task, USB_TASKQ_DRIVER, + NULL); s = splusb(); diff --git a/sys/dev/usb/if_urtw.c b/sys/dev/usb/if_urtw.c index d7625d62fd28..d528f4a5b3af 100644 --- a/sys/dev/usb/if_urtw.c +++ b/sys/dev/usb/if_urtw.c @@ -779,8 +779,9 @@ urtw_detach(device_t self, int flags) callout_destroy(&sc->scan_to); callout_destroy(&sc->sc_led_ch); - 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER, NULL); + usb_rem_task_wait(sc->sc_udev, &sc->sc_ledtask, USB_TASKQ_DRIVER, + NULL); if (ifp->if_softc != NULL) { bpf_detach(ifp); diff --git a/sys/dev/usb/if_urtwn.c b/sys/dev/usb/if_urtwn.c index 9afe3667fa91..2fa0800d5e36 100644 --- a/sys/dev/usb/if_urtwn.c +++ b/sys/dev/usb/if_urtwn.c @@ -544,7 +544,8 @@ urtwn_detach(device_t self, int flags) if (ISSET(sc->sc_flags, URTWN_FLAG_ATTACHED)) { urtwn_stop(ifp, 0); - usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER, + NULL); 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 4930c879ecee..e7e595b4ab69 100644 --- a/sys/dev/usb/if_zyd.c +++ b/sys/dev/usb/if_zyd.c @@ -468,7 +468,7 @@ zyd_detach(device_t self, int flags) zyd_stop(ifp, 1); 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); + usb_rem_task_wait(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER, NULL); /* Abort, etc. done by zyd_stop */ zyd_close_pipes(sc); diff --git a/sys/dev/usb/uatp.c b/sys/dev/usb/uatp.c index 87a43f3f0981..f3b147465b1a 100644 --- a/sys/dev/usb/uatp.c +++ b/sys/dev/usb/uatp.c @@ -1363,7 +1363,7 @@ geyser34_finalize(struct uatp_softc *sc) DPRINTF(sc, UATP_DEBUG_MISC, ("finalizing\n")); usb_rem_task_wait(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_reset_task, - USB_TASKQ_DRIVER); + USB_TASKQ_DRIVER, NULL); return 0; } diff --git a/sys/dev/usb/usb.c b/sys/dev/usb/usb.c index c497b0fbdebe..a8aa874de6fa 100644 --- a/sys/dev/usb/usb.c +++ b/sys/dev/usb/usb.c @@ -458,28 +458,13 @@ 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) + * usb_rem_task_wait(dev, task, queue, interlock) * * If task is scheduled to run, remove it from the queue. If it - * may have already begun to run, wait for it to complete. + * may have already begun to run, drop interlock if not null, wait + * for it to complete, and reacquire interlock if not null. + * Return true if it successfully removed the task from the queue, + * false if not. * * Caller MUST guarantee that task will not be scheduled on a * _different_ queue, at least until after this returns. @@ -490,11 +475,13 @@ usb_taskq_wait(struct usb_taskq *taskq, struct usb_task *task) * * May sleep. */ -void -usb_rem_task_wait(struct usbd_device *dev, struct usb_task *task, int queue) +bool +usb_rem_task_wait(struct usbd_device *dev, struct usb_task *task, int queue, + kmutex_t *interlock) { struct usb_taskq *taskq; int queue1; + bool removed; USBHIST_FUNC(); USBHIST_CALLED(usbdebug); ASSERT_SLEEPABLE(); @@ -502,38 +489,40 @@ usb_rem_task_wait(struct usbd_device *dev, struct usb_task *task, int queue) KASSERT(queue < USB_NUM_TASKQS); taskq = &usb_taskq[queue]; - - if ((queue1 = task->queue) == USB_NUM_TASKQS) { + mutex_enter(&taskq->lock); + queue1 = task->queue; + if (queue1 == USB_NUM_TASKQS) { /* - * It is not on the queue, but it may have already run. - * Wait for it. + * It is not on the queue. It may be about to run, or + * it may have already finished running -- there is no + * stopping it now. Wait for it if it is running. */ - mutex_enter(&taskq->lock); - usb_taskq_wait(taskq, task); - mutex_exit(&taskq->lock); + if (interlock) + mutex_exit(interlock); + while (taskq->current_task == task) + cv_wait(&taskq->cv, &taskq->lock); + removed = false; } 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. + * It is still on the queue. We can stop it before the + * task thread will run it. */ - 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); + KASSERTMSG(queue1 == queue, "task %p on q%d expected on q%d", + task, queue1, queue); + TAILQ_REMOVE(&taskq->tasks, task, next); + task->queue = USB_NUM_TASKQS; + removed = true; } + mutex_exit(&taskq->lock); + + /* + * If there's an interlock, and we dropped it to wait, + * reacquire it. + */ + if (interlock && !removed) + mutex_enter(interlock); + + return removed; } void diff --git a/sys/dev/usb/usb_subr.c b/sys/dev/usb/usb_subr.c index 974345335329..6960ce0b10f4 100644 --- a/sys/dev/usb/usb_subr.c +++ b/sys/dev/usb/usb_subr.c @@ -823,7 +823,8 @@ usbd_kill_pipe(struct usbd_pipe *pipe) usbd_lock_pipe(pipe); pipe->up_methods->upm_close(pipe); usbd_unlock_pipe(pipe); - usb_rem_task_wait(pipe->up_dev, &pipe->up_async_task, USB_TASKQ_DRIVER); + usb_rem_task_wait(pipe->up_dev, &pipe->up_async_task, USB_TASKQ_DRIVER, + NULL); 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 291e87d1226f..03ee738aa3a3 100644 --- a/sys/dev/usb/usbdi.h +++ b/sys/dev/usb/usbdi.h @@ -221,7 +221,8 @@ 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); +bool usb_rem_task_wait(struct usbd_device *, struct usb_task *, int, + kmutex_t *); #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 { -- 2.11.0