From owner-svn-src-head@FreeBSD.ORG Sun Jan 4 19:55:46 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 05F3110D; Sun, 4 Jan 2015 19:55:46 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E46CB340D; Sun, 4 Jan 2015 19:55:45 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id t04JtjqJ017344; Sun, 4 Jan 2015 19:55:45 GMT (envelope-from gibbs@FreeBSD.org) Received: (from gibbs@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id t04JtjCM017342; Sun, 4 Jan 2015 19:55:45 GMT (envelope-from gibbs@FreeBSD.org) Message-Id: <201501041955.t04JtjCM017342@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: gibbs set sender to gibbs@FreeBSD.org using -f From: "Justin T. Gibbs" Date: Sun, 4 Jan 2015 19:55:45 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r276665 - in head: share/man/man9 sys/kern X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Jan 2015 19:55:46 -0000 Author: gibbs Date: Sun Jan 4 19:55:44 2015 New Revision: 276665 URL: https://svnweb.freebsd.org/changeset/base/276665 Log: Prevent live-lock and access of destroyed data in taskqueue_drain_all(). Phabric: https://reviews.freebsd.org/D1247 Reviewed by: jhb, avg Sponsored by: Spectra Logic Corporation sys/kern_subr_taskqueue.c: Modify taskqueue_drain_all() processing to use a temporary "barrier task", rather than rely on a user task that may be destroyed during taskqueue_drain_all()'s execution. The barrier task is queued behind all previously queued tasks and then has its priority elevated so that future tasks cannot pass it in the queue. Use a similar barrier scheme to drain threads processing current tasks. This requires taskqueue_run_locked() to insert and remove the taskqueue_busy object for the running thread for every task processed. share/man/man9/taskqueue.9: Remove warning about live-lock issues with taskqueue_drain_all() and indicate that it does not wait for tasks queued after it begins processing. Modified: head/share/man/man9/taskqueue.9 head/sys/kern/subr_taskqueue.c Modified: head/share/man/man9/taskqueue.9 ============================================================================== --- head/share/man/man9/taskqueue.9 Sun Jan 4 19:05:16 2015 (r276664) +++ head/share/man/man9/taskqueue.9 Sun Jan 4 19:55:44 2015 (r276665) @@ -28,7 +28,7 @@ .\" .\" $FreeBSD$ .\" -.Dd May 24, 2014 +.Dd January 4, 2015 .Dt TASKQUEUE 9 .Os .Sh NAME @@ -285,10 +285,16 @@ The .Fn taskqueue_drain_all function is used to wait for all pending and running tasks that are enqueued on the taskqueue to finish. -The caller must arrange that the tasks are not re-enqueued. -Note that +Tasks posted to the taskqueue after .Fn taskqueue_drain_all -currently does not handle tasks with delayed enqueueing. +begins processing, +including pending enqueues scheduled by a previous call to +.Fn taskqueue_enqueue_timeout , +do not extend the wait time of +.Fn taskqueue_drain_all +and may complete after +.Fn taskqueue_drain_all +returns. .Pp The .Fn taskqueue_block Modified: head/sys/kern/subr_taskqueue.c ============================================================================== --- head/sys/kern/subr_taskqueue.c Sun Jan 4 19:05:16 2015 (r276664) +++ head/sys/kern/subr_taskqueue.c Sun Jan 4 19:55:44 2015 (r276665) @@ -56,6 +56,8 @@ struct taskqueue_busy { TAILQ_ENTRY(taskqueue_busy) tb_link; }; +struct task * const TB_DRAIN_WAITER = (struct task *)0x1; + struct taskqueue { STAILQ_HEAD(, task) tq_queue; taskqueue_enqueue_fn tq_enqueue; @@ -241,6 +243,7 @@ taskqueue_enqueue_locked(struct taskqueu /* Return with lock released. */ return (0); } + int taskqueue_enqueue(struct taskqueue *queue, struct task *task) { @@ -302,12 +305,82 @@ taskqueue_enqueue_timeout(struct taskque } static void -taskqueue_drain_running(struct taskqueue *queue) +taskqueue_task_nop_fn(void *context, int pending) +{ +} + +/* + * Block until all currently queued tasks in this taskqueue + * have begun execution. Tasks queued during execution of + * this function are ignored. + */ +static void +taskqueue_drain_tq_queue(struct taskqueue *queue) { + struct task t_barrier; - while (!TAILQ_EMPTY(&queue->tq_active)) - TQ_SLEEP(queue, &queue->tq_active, &queue->tq_mutex, - PWAIT, "-", 0); + if (STAILQ_EMPTY(&queue->tq_queue)) + return; + + /* + * Enqueue our barrier with the lowest possible priority + * so we are inserted after all current tasks. + */ + TASK_INIT(&t_barrier, 0, taskqueue_task_nop_fn, &t_barrier); + taskqueue_enqueue_locked(queue, &t_barrier); + + /* + * Raise the barrier's priority so newly queued tasks cannot + * pass it. + */ + t_barrier.ta_priority = USHRT_MAX; + + /* + * Once the barrier has executed, all previously queued tasks + * have completed or are currently executing. + */ + while (t_barrier.ta_pending != 0) + TQ_SLEEP(queue, &t_barrier, &queue->tq_mutex, PWAIT, "-", 0); +} + +/* + * Block until all currently executing tasks for this taskqueue + * complete. Tasks that begin execution during the execution + * of this function are ignored. + */ +static void +taskqueue_drain_tq_active(struct taskqueue *queue) +{ + struct taskqueue_busy tb_marker, *tb_first; + + if (TAILQ_EMPTY(&queue->tq_active)) + return; + + /* Block taskq_terminate().*/ + queue->tq_callouts++; + + /* + * Wait for all currently executing taskqueue threads + * to go idle. + */ + tb_marker.tb_running = TB_DRAIN_WAITER; + TAILQ_INSERT_TAIL(&queue->tq_active, &tb_marker, tb_link); + while (TAILQ_FIRST(&queue->tq_active) != &tb_marker) + TQ_SLEEP(queue, &tb_marker, &queue->tq_mutex, PWAIT, "-", 0); + TAILQ_REMOVE(&queue->tq_active, &tb_marker, tb_link); + + /* + * Wakeup any other drain waiter that happened to queue up + * without any intervening active thread. + */ + tb_first = TAILQ_FIRST(&queue->tq_active); + if (tb_first != NULL && tb_first->tb_running == TB_DRAIN_WAITER) + wakeup(tb_first); + + /* Release taskqueue_terminate(). */ + queue->tq_callouts--; + if ((queue->tq_flags & TQ_FLAGS_ACTIVE) == 0) + wakeup_one(queue->tq_threads); } void @@ -334,14 +407,16 @@ static void taskqueue_run_locked(struct taskqueue *queue) { struct taskqueue_busy tb; + struct taskqueue_busy *tb_first; struct task *task; int pending; TQ_ASSERT_LOCKED(queue); tb.tb_running = NULL; - TAILQ_INSERT_TAIL(&queue->tq_active, &tb, tb_link); while (STAILQ_FIRST(&queue->tq_queue)) { + TAILQ_INSERT_TAIL(&queue->tq_active, &tb, tb_link); + /* * Carefully remove the first task from the queue and * zero its pending count. @@ -358,10 +433,13 @@ taskqueue_run_locked(struct taskqueue *q TQ_LOCK(queue); tb.tb_running = NULL; wakeup(task); + + TAILQ_REMOVE(&queue->tq_active, &tb, tb_link); + tb_first = TAILQ_FIRST(&queue->tq_active); + if (tb_first != NULL && + tb_first->tb_running == TB_DRAIN_WAITER) + wakeup(tb_first); } - TAILQ_REMOVE(&queue->tq_active, &tb, tb_link); - if (TAILQ_EMPTY(&queue->tq_active)) - wakeup(&queue->tq_active); } void @@ -448,19 +526,13 @@ taskqueue_drain(struct taskqueue *queue, void taskqueue_drain_all(struct taskqueue *queue) { - struct task *task; if (!queue->tq_spin) WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__); TQ_LOCK(queue); - task = STAILQ_LAST(&queue->tq_queue, task, ta_link); - if (task != NULL) - while (task->ta_pending != 0) - TQ_SLEEP(queue, task, &queue->tq_mutex, PWAIT, "-", 0); - taskqueue_drain_running(queue); - KASSERT(STAILQ_EMPTY(&queue->tq_queue), - ("taskqueue queue is not empty after draining")); + taskqueue_drain_tq_queue(queue); + taskqueue_drain_tq_active(queue); TQ_UNLOCK(queue); }