It looks to me like taskqueue_drain(taskqueue_thread, foo) will not correctly detect whether or not a task is currently running. The check is against a field in the taskqueue struct, but for the taskqueue_thread queue with more than one thread, multiple threads can simultaneously be running a task, thus stomping over the tq_running field. I have not seen any problem with the code as-is in actual use, so this is purely an inspection bug. The following patch should fix the problem. Because it changes the size of struct task I'm not sure if it would be suitable for MFC. Thanks, matthew diff --git a/sys/kern/subr_taskqueue.c b/sys/kern/subr_taskqueue.c index 8405b3d..3b18269 100644 --- a/sys/kern/subr_taskqueue.c +++ b/sys/kern/subr_taskqueue.c _at__at_ -51,7 +51,6 _at__at_ __FBSDID("$FreeBSD$"); const char *tq_name; taskqueue_enqueue_fn tq_enqueue; void *tq_context; - struct task *tq_running; struct mtx tq_mutex; struct thread **tq_threads; int tq_tcount; _at__at_ -233,13 +232,13 _at__at_ taskqueue_run(struct taskqueue *queue) STAILQ_REMOVE_HEAD(&queue->tq_queue, ta_link); pending = task->ta_pending; task->ta_pending = 0; - queue->tq_running = task; + task->ta_flags |= TA_FLAGS_RUNNING; TQ_UNLOCK(queue); task->ta_func(task->ta_context, pending); TQ_LOCK(queue); - queue->tq_running = NULL; + task->ta_flags &= ~TA_FLAGS_RUNNING; wakeup(task); } _at__at_ -256,14 +255,16 _at__at_ taskqueue_drain(struct taskqueue *queue, struct task *task) { if (queue->tq_spin) { /* XXX */ mtx_lock_spin(&queue->tq_mutex); - while (task->ta_pending != 0 || task == queue->tq_running) + while (task->ta_pending != 0 || + (task->ta_flags & TA_FLAGS_RUNNING) != 0) msleep_spin(task, &queue->tq_mutex, "-", 0); mtx_unlock_spin(&queue->tq_mutex); } else { WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__); mtx_lock(&queue->tq_mutex); - while (task->ta_pending != 0 || task == queue->tq_running) + while (task->ta_pending != 0 || + (task->ta_flags & TA_FLAGS_RUNNING) != 0) msleep(task, &queue->tq_mutex, PWAIT, "-", 0); mtx_unlock(&queue->tq_mutex); } diff --git a/sys/sys/_task.h b/sys/sys/_task.h index 2a51e1b..c57bdd5 100644 --- a/sys/sys/_task.h +++ b/sys/sys/_task.h _at__at_ -36,15 +36,21 _at__at_ * taskqueue_run(). The first argument is taken from the 'ta_context' * field of struct task and the second argument is a count of how many * times the task was enqueued before the call to taskqueue_run(). + * + * List of locks + * (c) const after init + * (q) taskqueue lock */ typedef void task_fn_t(void *context, int pending); struct task { - STAILQ_ENTRY(task) ta_link; /* link for queue */ - u_short ta_pending; /* count times queued */ - u_short ta_priority; /* Priority */ - task_fn_t *ta_func; /* task handler */ - void *ta_context; /* argument for handler */ + STAILQ_ENTRY(task) ta_link; /* (q) link for queue */ + u_char ta_flags; /* (q) state of this task */ +#define TA_FLAGS_RUNNING 0x01 + u_short ta_pending; /* (q) count times queued */ + u_short ta_priority; /* (c) Priority */ + task_fn_t *ta_func; /* (c) task handler */ + void *ta_context; /* (c) argument for handler */Received on Wed Apr 28 2010 - 22:00:00 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:03 UTC