On Sat, Aug 21, 2010 at 08:25:23AM +0000, Jacob Meuser wrote:
> +volatile int usb_run_tasks;
> +volatile int discover_pending = 0;
Do these really need to be volatile?
+/* called by usbd_finish when lasst usb is detached */
s/lasst/last/
+void
+usb_end_tasks(void)
+{
+ usb_run_tasks = 0;
+}
I think we need a wakeup(&usb_run_tasks) after the assignment.
> - s = splusb();
> - for (;;) {
> - task = TAILQ_FIRST(&usb_all_tasks);
> - if (task == NULL) {
> - tsleep(&usb_all_tasks, PWAIT, "usbtsk", 0);
> - task = TAILQ_FIRST(&usb_all_tasks);
> + while (usb_run_tasks) {
> + s = splusb();
> + while (usb_run_tasks &&
> + (!TAILQ_EMPTY(&usb_hub_tasks) ||
> + !TAILQ_EMPTY(&usb_device_tasks))) {
> +
> + task = TAILQ_FIRST(&usb_hub_tasks);
> + while (task != NULL && usb_run_tasks) {
> + TAILQ_REMOVE(&usb_hub_tasks, task, next);
> + task->onqueue = 0;
> + splx(s);
> + task->fun(task->arg);
> + s = splusb();
> + task = TAILQ_FIRST(&usb_hub_tasks);
> + }
> +
> + task = TAILQ_FIRST(&usb_device_tasks);
> + while (task != NULL && usb_run_tasks) {
> + TAILQ_REMOVE(&usb_device_tasks, task, next);
> + task->onqueue = 0;
> + splx(s);
> + task->fun(task->arg);
> + s = splusb();
> + if (!TAILQ_EMPTY(&usb_hub_tasks))
> + break;
> + task = TAILQ_FIRST(&usb_device_tasks);
> + }
> }
> - DPRINTFN(2,("usb_task_thread: woke up task=%p\n", task));
> - if (task != NULL) {
> - TAILQ_REMOVE(&usb_all_tasks, task, next);
> - task->onqueue = 0;
> - splx(s);
> - task->fun(task->arg);
> - s = splusb();
> + splx(s);
> +
> + if (usb_run_tasks) {
> + /* XXX Is not timing out too optimistic? */
> + tsleep(&usb_run_tasks, PWAIT, "usbtsk", 0);
> + DPRINTFN(2,("%s: woke up\n", __func__));
> }
> }
If a bunch of device tasks get enqueued and then while we're running
them, a discover task gets enqueued, the device tasks will finish
running before we run the discover task.
How about writing the task thread loop like this:
s = splusb();
for (usb_run_tasks) {
if ((task = TAILQ_FIRST(&usb_hub_tasks)) != NULL)
TAILQ_REMOVE(&usb_hub_tasks, task, next);
else if ((task = TAILQ_FIRST(&usb_device_tasks)) != NULL)
TAILQ_REMOVE(&usb_device_tasks, task, next);
else {
tsleep(&usb_run_tasks, PWAIT, "usbtsk", 0);
continue;
}
task->onqueue = 0;
splx(s);
task->fun(task->arg);
s = splusb();
}
splx(s);
Also, is there a chance that there are tasks still enqueued when
usb_end_tasks() is called? Do they need any cleanup?
