Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 12 Nov 2010 18:10:27 +0000 (UTC)
From:      Matthew D Fleming <mdf@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org
Subject:   svn commit: r215191 - stable/7/sys/kern
Message-ID:  <201011121810.oACIAR1x018652@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mdf
Date: Fri Nov 12 18:10:27 2010
New Revision: 215191
URL: http://svn.freebsd.org/changeset/base/215191

Log:
  MFC r213813.  This is a direct commit because the diff is against
  several non-MFC'd changes.
  
  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.

Modified:
  stable/7/sys/kern/subr_taskqueue.c

Modified: stable/7/sys/kern/subr_taskqueue.c
==============================================================================
--- stable/7/sys/kern/subr_taskqueue.c	Fri Nov 12 18:09:06 2010	(r215190)
+++ stable/7/sys/kern/subr_taskqueue.c	Fri Nov 12 18:10:27 2010	(r215191)
@@ -48,13 +48,18 @@ static void	*taskqueue_ih;
 static STAILQ_HEAD(taskqueue_list, taskqueue) taskqueue_queues;
 static struct mtx taskqueue_queues_mutex;
 
+struct taskqueue_busy {
+	struct task	*tb_running;
+	TAILQ_ENTRY(taskqueue_busy) tb_link;
+};
+
 struct taskqueue {
 	STAILQ_ENTRY(taskqueue)	tq_link;
 	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 proc		**tq_pproc;
 	int			tq_pcount;
@@ -66,6 +71,8 @@ struct taskqueue {
 #define	TQ_FLAGS_BLOCKED	(1 << 1)
 #define	TQ_FLAGS_PENDING	(1 << 2)
 
+static void	taskqueue_run_locked(struct taskqueue *);
+
 static __inline void
 TQ_LOCK(struct taskqueue *tq)
 {
@@ -117,6 +124,7 @@ _taskqueue_create(const char *name, int 
 		return 0;
 
 	STAILQ_INIT(&queue->tq_queue);
+	TAILQ_INIT(&queue->tq_active);
 	queue->tq_name = name;
 	queue->tq_enqueue = enqueue;
 	queue->tq_context = context;
@@ -162,8 +170,9 @@ taskqueue_free(struct taskqueue *queue)
 
 	TQ_LOCK(queue);
 	queue->tq_flags &= ~TQ_FLAGS_ACTIVE;
-	taskqueue_run(queue);
+	taskqueue_run_locked(queue);
 	taskqueue_terminate(queue->tq_pproc, queue);
+	KASSERT(TAILQ_EMPTY(&queue->tq_active), ("Tasks still running?"));
 	mtx_destroy(&queue->tq_mutex);
 	free(queue->tq_pproc, M_TASKQUEUE);
 	free(queue, M_TASKQUEUE);
@@ -258,15 +267,17 @@ taskqueue_unblock(struct taskqueue *queu
 	TQ_UNLOCK(queue);
 }
 
-void
-taskqueue_run(struct taskqueue *queue)
+static void
+taskqueue_run_locked(struct taskqueue *queue)
 {
+	struct taskqueue_busy tb;
 	struct task *task;
-	int owned, pending;
+	int pending;
+
+	mtx_assert(&queue->tq_mutex, MA_OWNED);
+	tb.tb_running = NULL;
+	TAILQ_INSERT_TAIL(&queue->tq_active, &tb, tb_link);
 
-	owned = mtx_owned(&queue->tq_mutex);
-	if (!owned)
-		TQ_LOCK(queue);
 	while (STAILQ_FIRST(&queue->tq_queue)) {
 		/*
 		 * Carefully remove the first task from the queue and
@@ -276,22 +287,38 @@ 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;
+		tb.tb_running = task;
 		TQ_UNLOCK(queue);
 
 		task->ta_func(task->ta_context, pending);
 
 		TQ_LOCK(queue);
-		queue->tq_running = NULL;
+		tb.tb_running = NULL;
 		wakeup(task);
 	}
+	TAILQ_REMOVE(&queue->tq_active, &tb, tb_link);
+}
 
-	/*
-	 * For compatibility, unlock on return if the queue was not locked
-	 * on entry, although this opens a race window.
-	 */
-	if (!owned)
-		TQ_UNLOCK(queue);
+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
@@ -299,14 +326,14 @@ taskqueue_drain(struct taskqueue *queue,
 {
 	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_is_running(queue, task))
 			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_is_running(queue, task))
 			msleep(task, &queue->tq_mutex, PWAIT, "-", 0);
 		mtx_unlock(&queue->tq_mutex);
 	}
@@ -398,7 +425,7 @@ taskqueue_thread_loop(void *arg)
 	tq = *tqp;
 	TQ_LOCK(tq);
 	while ((tq->tq_flags & TQ_FLAGS_ACTIVE) != 0) {
-		taskqueue_run(tq);
+		taskqueue_run_locked(tq);
 		TQ_SLEEP(tq, tq, &tq->tq_mutex, 0, "-", 0);
 	}

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201011121810.oACIAR1x018652>