From owner-svn-src-all@FreeBSD.ORG Wed Oct 13 22:59:04 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DD95D1065670; Wed, 13 Oct 2010 22:59:04 +0000 (UTC) (envelope-from mdf@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id CBB358FC14; Wed, 13 Oct 2010 22:59:04 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o9DMx4So003617; Wed, 13 Oct 2010 22:59:04 GMT (envelope-from mdf@svn.freebsd.org) Received: (from mdf@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o9DMx43Z003612; Wed, 13 Oct 2010 22:59:04 GMT (envelope-from mdf@svn.freebsd.org) Message-Id: <201010132259.o9DMx43Z003612@svn.freebsd.org> From: Matthew D Fleming Date: Wed, 13 Oct 2010 22:59:04 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r213813 - in head: share/man/man9 sys/kern sys/sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Oct 2010 22:59:05 -0000 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) /*