[src/trunk]: src/sys/kern workqueue: Lift unnecessary restriction on workqueu...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/1d11fb8a95f4
branches:  trunk
changeset: 1013957:1d11fb8a95f4
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Sep 08 17:02:18 2020 +0000

description:
workqueue: Lift unnecessary restriction on workqueue_wait.

Allow multiple concurrent waits at a time, and allow enqueueing work
at the same time (as long as it's not the work we're waiting for).
This way multiple users can use a shared global workqueue and safely
wait for individual work items concurrently, while the workqueue is
still in use for other items (e.g., wg(4) peers).

This has the side effect of taking away a diagnostic measure, but I
think allowing the diagnostic's false positives instead of rejecting
them is worth it.  We could cheaply add it back with some false
negatives if it's important.

diffstat:

 share/man/man9/workqueue.9 |  12 ++++++------
 sys/kern/subr_workqueue.c  |  22 +++++++---------------
 2 files changed, 13 insertions(+), 21 deletions(-)

diffs (110 lines):

diff -r 5ded9166ee32 -r 1d11fb8a95f4 share/man/man9/workqueue.9
--- a/share/man/man9/workqueue.9        Tue Sep 08 17:02:03 2020 +0000
+++ b/share/man/man9/workqueue.9        Tue Sep 08 17:02:18 2020 +0000
@@ -1,4 +1,4 @@
-.\"    $NetBSD: workqueue.9,v 1.14 2020/08/01 09:51:06 wiz Exp $
+.\"    $NetBSD: workqueue.9,v 1.15 2020/09/08 17:02:18 riastradh Exp $
 .\"
 .\" Copyright (c)2005 YAMAMOTO Takashi,
 .\" All rights reserved.
@@ -133,11 +133,11 @@
 on the workqueue
 .Fa wq
 to finish.
-The caller must ensure that no new work will be enqueued to the workqueue
-beforehand.
-Note that if the workqueue is
-.Dv WQ_PERCPU ,
-the caller can enqueue a new work to another queue other than the waiting queue.
+The caller must ensure that
+.Fa wk
+will not be enqueued to the workqueue again until after
+.Fn workqueue_wait
+returns.
 .Pp
 .\" - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 .Fn workqueue_destroy
diff -r 5ded9166ee32 -r 1d11fb8a95f4 sys/kern/subr_workqueue.c
--- a/sys/kern/subr_workqueue.c Tue Sep 08 17:02:03 2020 +0000
+++ b/sys/kern/subr_workqueue.c Tue Sep 08 17:02:18 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: subr_workqueue.c,v 1.38 2020/08/01 02:14:43 riastradh Exp $    */
+/*     $NetBSD: subr_workqueue.c,v 1.39 2020/09/08 17:02:18 riastradh Exp $    */
 
 /*-
  * Copyright (c)2002, 2005, 2006, 2007 YAMAMOTO Takashi,
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_workqueue.c,v 1.38 2020/08/01 02:14:43 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_workqueue.c,v 1.39 2020/09/08 17:02:18 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/cpu.h>
@@ -52,7 +52,6 @@
        struct workqhead q_queue_pending;
        struct workqhead q_queue_running;
        lwp_t *q_worker;
-       work_impl_t *q_waiter;
 };
 
 struct workqueue {
@@ -138,10 +137,8 @@
                mutex_enter(&q->q_mutex);
                KASSERT(!SIMPLEQ_EMPTY(&q->q_queue_running));
                SIMPLEQ_INIT(&q->q_queue_running);
-               if (__predict_false(q->q_waiter != NULL)) {
-                       /* Wake up workqueue_wait */
-                       cv_signal(&q->q_cv);
-               }
+               /* Wake up workqueue_wait */
+               cv_broadcast(&q->q_cv);
                mutex_exit(&q->q_mutex);
        }
        if (wq->wq_flags & WQ_FPU)
@@ -211,7 +208,7 @@
        KASSERT(SIMPLEQ_EMPTY(&q->q_queue_pending));
        mutex_enter(&q->q_mutex);
        q->q_worker = NULL;
-       cv_signal(&q->q_cv);
+       cv_broadcast(&q->q_cv);
        mutex_exit(&q->q_mutex);
        kthread_exit(0);
 }
@@ -228,7 +225,7 @@
        KASSERT(q->q_worker != NULL);
        mutex_enter(&q->q_mutex);
        SIMPLEQ_INSERT_TAIL(&q->q_queue_pending, &wqe.wqe_wk, wk_entry);
-       cv_signal(&q->q_cv);
+       cv_broadcast(&q->q_cv);
        while (q->q_worker != NULL) {
                cv_wait(&q->q_cv, &q->q_mutex);
        }
@@ -306,13 +303,9 @@
     found:
        if (wk != NULL) {
                found = true;
-               KASSERT(q->q_waiter == NULL);
-               q->q_waiter = wk;
                cv_wait(&q->q_cv, &q->q_mutex);
                goto again;
        }
-       if (q->q_waiter != NULL)
-               q->q_waiter = NULL;
     out:
        mutex_exit(&q->q_mutex);
 
@@ -386,11 +379,10 @@
        q = workqueue_queue_lookup(wq, ci);
 
        mutex_enter(&q->q_mutex);
-       KASSERT(q->q_waiter == NULL);
 #ifdef DEBUG
        workqueue_check_duplication(q, wk);
 #endif
        SIMPLEQ_INSERT_TAIL(&q->q_queue_pending, wk, wk_entry);
-       cv_signal(&q->q_cv);
+       cv_broadcast(&q->q_cv);
        mutex_exit(&q->q_mutex);
 }