add WITNESS support to barriers modelled on the timeout stuff visa did.

if a taskq takes a lock, and something holding that lock calls
taskq_barrier, there's a potential deadlock. detect this as a lock
order problem when witness is enable. task_del conditionally followed
by taskq_barrier is a common pattern, so add a taskq_del_barrier
wrapper for it that unconditionally checks for the deadlock, like
timeout_del_barrier.

ok visa@

3 files changed, 82 insertions, 8 deletions

diff --git a/share/man/man9/task_add.9 b/share/man/man9/task_add.9
index 9874d8398a1..294ee468231 100644
--- a/share/man/man9/task_add.9
+++ b/share/man/man9/task_add.9
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: task_add.9,v 1.20 2019/04/01 06:28:05 jmc Exp $
+.\"	$OpenBSD: task_add.9,v 1.21 2019/04/28 04:20:40 dlg Exp $
 .\"
 .\" Copyright (c) 2013 David Gwynne <dlg@openbsd.org>
 .\"
@@ -14,13 +14,14 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: April 1 2019 $
+.Dd $Mdocdate: April 28 2019 $
 .Dt TASK_ADD 9
 .Os
 .Sh NAME
 .Nm taskq_create ,
 .Nm taskq_destroy ,
 .Nm taskq_barrier ,
+.Nm taskq_del_barrier ,
 .Nm task_set ,
 .Nm task_add ,
 .Nm task_del ,
@@ -41,6 +42,8 @@
 .Ft void
 .Fn taskq_barrier "struct taskq *tq"
 .Ft void
+.Fn taskq_del_barrier "struct taskq *tq" "struct task *t"
+.Ft void
 .Fn task_set "struct task *t" "void (*fn)(void *)" "void *arg"
 .Ft int
 .Fn task_add "struct taskq *tq" "struct task *t"
@@ -101,6 +104,13 @@ returns.
 is only supported on taskqs serviced by 1 thread,
 and may not be called by a task running in the specified taskq.
 .Pp
+.Fn taskq_del_barrier
+either removes
+.Fa t
+from the list of pending tasks on the
+.Fa tq
+taskq, or waits until any running task has completed.
+.Pp
 It is the responsibility of the caller to provide the
 .Fn task_set ,
 .Fn task_add ,
@@ -181,6 +191,8 @@ and
 .Fn taskq_destroy
 can be called during autoconf, or from process context.
 .Fn taskq_barrier
+and
+.Fn taskq_del_barrier
 can be called from process context.
 .Fn task_set ,
 .Fn task_add ,
diff --git a/sys/kern/kern_task.c b/sys/kern/kern_task.c
index c5e824636c9..74f3ea4ab18 100644
--- a/sys/kern/kern_task.c
+++ b/sys/kern/kern_task.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_task.c,v 1.24 2019/04/01 03:23:45 dlg Exp $ */
+/*	$OpenBSD: kern_task.c,v 1.25 2019/04/28 04:20:40 dlg Exp $ */
 
 /*
  * Copyright (c) 2013 David Gwynne <dlg@openbsd.org>
@@ -23,6 +23,18 @@
 #include <sys/kthread.h>
 #include <sys/task.h>
 #include <sys/proc.h>
+#include <sys/witness.h>
+
+#ifdef WITNESS
+
+static struct lock_type taskq_lock_type = {
+	.lt_name = "taskq"
+};
+
+#define TASKQ_LOCK_FLAGS LO_WITNESS | LO_INITIALIZED | LO_SLEEPABLE | \
+    (LO_CLASS_RWLOCK << LO_CLASSSHIFT)
+
+#endif /* WITNESS */
 
 struct taskq {
 	enum {
@@ -37,26 +49,45 @@ struct taskq {
 
 	struct mutex		 tq_mtx;
 	struct task_list	 tq_worklist;
+#ifdef WITNESS
+	struct lock_object	 tq_lock_object;
+#endif
 };
 
+static const char taskq_sys_name[] = "systq";
+
 struct taskq taskq_sys = {
 	TQ_S_CREATED,
 	0,
 	1,
 	0,
-	"systq",
+	taskq_sys_name,
 	MUTEX_INITIALIZER(IPL_HIGH),
-	TAILQ_HEAD_INITIALIZER(taskq_sys.tq_worklist)
+	TAILQ_HEAD_INITIALIZER(taskq_sys.tq_worklist),
+#ifdef WITNESS
+	{
+		.lo_name = taskq_sys_name,
+		.lo_flags = TASKQ_LOCK_FLAGS,
+	},
+#endif
 };
 
+static const char taskq_sys_mp_name[] = "systqmp";
+
 struct taskq taskq_sys_mp = {
 	TQ_S_CREATED,
 	0,
 	1,
 	TASKQ_MPSAFE,
-	"systqmp",
+	taskq_sys_mp_name,
 	MUTEX_INITIALIZER(IPL_HIGH),
-	TAILQ_HEAD_INITIALIZER(taskq_sys_mp.tq_worklist)
+	TAILQ_HEAD_INITIALIZER(taskq_sys_mp.tq_worklist),
+#ifdef WITNESS
+	{
+		.lo_name = taskq_sys_mp_name,
+		.lo_flags = TASKQ_LOCK_FLAGS,
+	},
+#endif
 };
 
 struct taskq *const systq = &taskq_sys;
@@ -73,7 +104,9 @@ void	taskq_thread(void *);
 void
 taskq_init(void)
 {
+	WITNESS_INIT(&systq->tq_lock_object, &taskq_lock_type);
 	kthread_create_deferred(taskq_create_thread, systq);
+	WITNESS_INIT(&systqmp->tq_lock_object, &taskq_lock_type);
 	kthread_create_deferred(taskq_create_thread, systqmp);
 }
 
@@ -96,6 +129,13 @@ taskq_create(const char *name, unsigned int nthreads, int ipl,
 	mtx_init_flags(&tq->tq_mtx, ipl, name, 0);
 	TAILQ_INIT(&tq->tq_worklist);
 
+#ifdef WITNESS
+	memset(&tq->tq_lock_object, 0, sizeof(tq->tq_lock_object));
+	tq->tq_lock_object.lo_name = name;
+	tq->tq_lock_object.lo_flags = TASKQ_LOCK_FLAGS;
+	witness_init(&tq->tq_lock_object, &taskq_lock_type);
+#endif
+
 	/* try to create a thread to guarantee that tasks will be serviced */
 	kthread_create_deferred(taskq_create_thread, tq);
 
@@ -181,8 +221,24 @@ taskq_barrier(struct taskq *tq)
 	struct cond c = COND_INITIALIZER();
 	struct task t = TASK_INITIALIZER(taskq_barrier_task, &c);
 
+	WITNESS_CHECKORDER(&tq->tq_lock_object, LOP_NEWORDER, NULL);
+
 	task_add(tq, &t);
+	cond_wait(&c, "tqbar");
+}
 
+void
+taskq_del_barrier(struct taskq *tq, struct task *del)
+{
+	struct cond c = COND_INITIALIZER();
+	struct task t = TASK_INITIALIZER(taskq_barrier_task, &c);
+
+	WITNESS_CHECKORDER(&tq->tq_lock_object, LOP_NEWORDER, NULL);
+
+	if (task_del(tq, del))
+		return;
+
+	task_add(tq, &t);
 	cond_wait(&c, "tqbar");
 }
 
@@ -281,8 +337,12 @@ taskq_thread(void *xtq)
 	if (ISSET(tq->tq_flags, TASKQ_MPSAFE))
 		KERNEL_UNLOCK();
 
+	WITNESS_CHECKORDER(&tq->tq_lock_object, LOP_NEWORDER, NULL);
+
 	while (taskq_next_work(tq, &work)) {
+		WITNESS_LOCK(&tq->tq_lock_object, 0);
 		(*work.t_func)(work.t_arg);
+		WITNESS_UNLOCK(&tq->tq_lock_object, 0);
 		sched_pause(yield);
 	}
 
diff --git a/sys/sys/task.h b/sys/sys/task.h
index df9270072bc..be83b99f7ca 100644
--- a/sys/sys/task.h
+++ b/sys/sys/task.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: task.h,v 1.14 2019/04/01 03:23:45 dlg Exp $ */
+/*	$OpenBSD: task.h,v 1.15 2019/04/28 04:20:40 dlg Exp $ */
 
 /*
  * Copyright (c) 2013 David Gwynne <dlg@openbsd.org>
@@ -46,6 +46,8 @@ struct taskq	*taskq_create(const char *, unsigned int, int, unsigned int);
 void		 taskq_destroy(struct taskq *);
 void		 taskq_barrier(struct taskq *);
 
+void		 taskq_del_barrier(struct taskq *, struct task *);
+
 void		 task_set(struct task *, void (*)(void *), void *);
 int		 task_add(struct taskq *, struct task *);
 int		 task_del(struct taskq *, struct task *);