Author: mdf
Date: Wed Oct 13 22:59:04 2010
New Revision: 213813
URL: http://svn.freebsd.org/changeset/base/213813
Log:
Use a safer mechanism for determining if a task is currently running,
that does not rely on the lifetime of pointers being the same. This also
restores the task KBI.
Suggested by: jhb
MFC after: 1 month
Modified:
head/share/man/man9/taskqueue.9
head/sys/kern/subr_taskqueue.c
head/sys/sys/_task.h
head/sys/sys/taskqueue.h
Modified: head/share/man/man9/taskqueue.9
==============================================================================
--- head/share/man/man9/taskqueue.9 Wed Oct 13 22:29:48 2010
(r213812)
+++ head/share/man/man9/taskqueue.9 Wed Oct 13 22:59:04 2010
(r213813)
@@ -68,7 +68,7 @@ struct task {
.Ft int
.Fn taskqueue_member "struct taskqueue *queue" "struct thread *td"
.Ft void
-.Fn taskqueue_run "struct taskqueue *queue" "struct task **tpp"
+.Fn taskqueue_run "struct taskqueue *queue"
.Fn TASK_INIT "struct task *task" "int priority" "task_fn_t *func" "void
*context"
.Fn TASKQUEUE_DECLARE "name"
.Fn TASKQUEUE_DEFINE "name" "taskqueue_enqueue_fn enqueue" "void *context"
"init"
@@ -185,13 +185,6 @@ The
function will run all pending tasks in the specified
.Fa queue .
Normally this function is only used internally.
-The
-.Fa tpp
-argument is a pointer to a
-.Vt struct task *
-that is used to record the currently running task.
-The lifetime of this pointer must match that of the
-.Fa queue .
.Pp
A convenience macro,
.Fn TASK_INIT "task" "priority" "func" "context"
Modified: head/sys/kern/subr_taskqueue.c
==============================================================================
--- head/sys/kern/subr_taskqueue.c Wed Oct 13 22:29:48 2010
(r213812)
+++ head/sys/kern/subr_taskqueue.c Wed Oct 13 22:59:04 2010
(r213813)
@@ -46,12 +46,17 @@ static MALLOC_DEFINE(M_TASKQUEUE, "taskq
static void *taskqueue_giant_ih;
static void *taskqueue_ih;
+struct taskqueue_busy {
+ struct task *tb_running;
+ TAILQ_ENTRY(taskqueue_busy) tb_link;
+};
+
struct taskqueue {
STAILQ_HEAD(, task) tq_queue;
const char *tq_name;
taskqueue_enqueue_fn tq_enqueue;
void *tq_context;
- struct task *tq_running;
+ TAILQ_HEAD(, taskqueue_busy) tq_active;
struct mtx tq_mutex;
struct thread **tq_threads;
int tq_tcount;
@@ -102,6 +107,7 @@ _taskqueue_create(const char *name, int
return NULL;
STAILQ_INIT(&queue->tq_queue);
+ TAILQ_INIT(&queue->tq_active);
queue->tq_name = name;
queue->tq_enqueue = enqueue;
queue->tq_context = context;
@@ -140,6 +146,7 @@ taskqueue_free(struct taskqueue *queue)
TQ_LOCK(queue);
queue->tq_flags &= ~TQ_FLAGS_ACTIVE;
taskqueue_terminate(queue->tq_threads, queue);
+ KASSERT(TAILQ_EMPTY(&queue->tq_active), ("Tasks still running?"));
mtx_destroy(&queue->tq_mutex);
free(queue->tq_threads, M_TASKQUEUE);
free(queue, M_TASKQUEUE);
@@ -214,13 +221,17 @@ taskqueue_unblock(struct taskqueue *queu
TQ_UNLOCK(queue);
}
-void
-taskqueue_run(struct taskqueue *queue, struct task **tpp)
+static void
+taskqueue_run_locked(struct taskqueue *queue)
{
+ struct taskqueue_busy tb;
struct task *task;
int pending;
mtx_assert(&queue->tq_mutex, MA_OWNED);
+ tb.tb_running = NULL;
+ TAILQ_INSERT_TAIL(&queue->tq_active, &tb, tb_link);
+
while (STAILQ_FIRST(&queue->tq_queue)) {
/*
* Carefully remove the first task from the queue and
@@ -230,16 +241,38 @@ taskqueue_run(struct taskqueue *queue, s
STAILQ_REMOVE_HEAD(&queue->tq_queue, ta_link);
pending = task->ta_pending;
task->ta_pending = 0;
- task->ta_running = tpp;
- *tpp = task;
+ tb.tb_running = task;
TQ_UNLOCK(queue);
task->ta_func(task->ta_context, pending);
TQ_LOCK(queue);
- *tpp = NULL;
+ tb.tb_running = NULL;
wakeup(task);
}
+ TAILQ_REMOVE(&queue->tq_active, &tb, tb_link);
+}
+
+void
+taskqueue_run(struct taskqueue *queue)
+{
+
+ TQ_LOCK(queue);
+ taskqueue_run_locked(queue);
+ TQ_UNLOCK(queue);
+}
+
+static int
+task_is_running(struct taskqueue *queue, struct task *task)
+{
+ struct taskqueue_busy *tb;
+
+ mtx_assert(&queue->tq_mutex, MA_OWNED);
+ TAILQ_FOREACH(tb, &queue->tq_active, tb_link) {
+ if (tb->tb_running == task)
+ return (1);
+ }
+ return (0);
}
void
@@ -250,10 +283,8 @@ taskqueue_drain(struct taskqueue *queue,
WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__);
TQ_LOCK(queue);
- while (task->ta_pending != 0 ||
- (task->ta_running != NULL && task == *task->ta_running)) {
+ while (task->ta_pending != 0 || task_is_running(queue, task))
TQ_SLEEP(queue, task, &queue->tq_mutex, PWAIT, "-", 0);
- }
TQ_UNLOCK(queue);
}
@@ -266,9 +297,7 @@ taskqueue_swi_enqueue(void *context)
static void
taskqueue_swi_run(void *dummy)
{
- TQ_LOCK(taskqueue_swi);
- taskqueue_run(taskqueue_swi, &taskqueue_swi->tq_running);
- TQ_UNLOCK(taskqueue_swi);
+ taskqueue_run(taskqueue_swi);
}
static void
@@ -280,9 +309,7 @@ taskqueue_swi_giant_enqueue(void *contex
static void
taskqueue_swi_giant_run(void *dummy)
{
- TQ_LOCK(taskqueue_swi_giant);
- taskqueue_run(taskqueue_swi_giant, &taskqueue_swi_giant->tq_running);
- TQ_UNLOCK(taskqueue_swi_giant);
+ taskqueue_run(taskqueue_swi_giant);
}
int
@@ -344,22 +371,12 @@ void
taskqueue_thread_loop(void *arg)
{
struct taskqueue **tqp, *tq;
- struct task *running;
-
- /*
- * The kernel stack space is globaly addressable, and it would
- * be an error to ask whether a task is running after the
- * taskqueue has been released. So it is safe to have the
- * task point back to an address in the taskqueue's stack to
- * determine if the task is running.
- */
- running = NULL;
tqp = arg;
tq = *tqp;
TQ_LOCK(tq);
while ((tq->tq_flags & TQ_FLAGS_ACTIVE) != 0) {
- taskqueue_run(tq, &running);
+ taskqueue_run_locked(tq);
/*
* Because taskqueue_run() can drop tq_mutex, we need to
* check if the TQ_FLAGS_ACTIVE flag wasn't removed in the
@@ -369,7 +386,7 @@ taskqueue_thread_loop(void *arg)
break;
TQ_SLEEP(tq, tq, &tq->tq_mutex, 0, "-", 0);
}
- taskqueue_run(tq, &running);
+ taskqueue_run_locked(tq);
/* rendezvous with thread that asked us to terminate */
tq->tq_tcount--;
@@ -426,9 +443,7 @@ taskqueue_fast_enqueue(void *context)
static void
taskqueue_fast_run(void *dummy)
{
- TQ_LOCK(taskqueue_fast);
- taskqueue_run(taskqueue_fast, &taskqueue_fast->tq_running);
- TQ_UNLOCK(taskqueue_fast);
+ taskqueue_run(taskqueue_fast);
}
TASKQUEUE_FAST_DEFINE(fast, taskqueue_fast_enqueue, NULL,
Modified: head/sys/sys/_task.h
==============================================================================
--- head/sys/sys/_task.h Wed Oct 13 22:29:48 2010 (r213812)
+++ head/sys/sys/_task.h Wed Oct 13 22:59:04 2010 (r213813)
@@ -44,7 +44,6 @@
typedef void task_fn_t(void *context, int pending);
struct task {
- struct task **ta_running; /* (q) queue's running task pointer */
STAILQ_ENTRY(task) ta_link; /* (q) link for queue */
u_short ta_pending; /* (q) count times queued */
u_short ta_priority; /* (c) Priority */
Modified: head/sys/sys/taskqueue.h
==============================================================================
--- head/sys/sys/taskqueue.h Wed Oct 13 22:29:48 2010 (r213812)
+++ head/sys/sys/taskqueue.h Wed Oct 13 22:59:04 2010 (r213813)
@@ -56,7 +56,7 @@ int taskqueue_start_threads(struct taskq
int taskqueue_enqueue(struct taskqueue *queue, struct task *task);
void taskqueue_drain(struct taskqueue *queue, struct task *task);
void taskqueue_free(struct taskqueue *queue);
-void taskqueue_run(struct taskqueue *queue, struct task **tpp);
+void taskqueue_run(struct taskqueue *queue);
void taskqueue_block(struct taskqueue *queue);
void taskqueue_unblock(struct taskqueue *queue);
int taskqueue_member(struct taskqueue *queue, struct thread *td);
@@ -75,7 +75,6 @@ void taskqueue_thread_enqueue(void *cont
(task)->ta_priority = (priority); \
(task)->ta_func = (func); \
(task)->ta_context = (context); \
- (task)->ta_running = NULL; \
} while (0)
/*
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
