[src/trunk]: src/sys/external/bsd/common/linux Fix bugs in workqueue destruct...

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

details:   https://anonhg.NetBSD.org/src/rev/1430380a3262
branches:  trunk
changeset: 366356:1430380a3262
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Mon Aug 27 15:02:52 2018 +0000

description:
Fix bugs in workqueue destruction.

diffstat:

 sys/external/bsd/common/linux/linux_work.c |  57 +++++++++++++++++++++--------
 1 files changed, 40 insertions(+), 17 deletions(-)

diffs (90 lines):

diff -r 1a0206474d8f -r 1430380a3262 sys/external/bsd/common/linux/linux_work.c
--- a/sys/external/bsd/common/linux/linux_work.c        Mon Aug 27 15:02:38 2018 +0000
+++ b/sys/external/bsd/common/linux/linux_work.c        Mon Aug 27 15:02:52 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: linux_work.c,v 1.25 2018/08/27 15:02:38 riastradh Exp $        */
+/*     $NetBSD: linux_work.c,v 1.26 2018/08/27 15:02:52 riastradh Exp $        */
 
 /*-
  * Copyright (c) 2018 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: linux_work.c,v 1.25 2018/08/27 15:02:38 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: linux_work.c,v 1.26 2018/08/27 15:02:52 riastradh Exp $");
 
 #include <sys/types.h>
 #include <sys/atomic.h>
@@ -174,21 +174,39 @@
         * delayed work that that has already timed out, which we can't
         * cancel, may have queued new work.
         */
-       for (;;) {
-               struct delayed_work *dw = NULL;
+       mutex_enter(&wq->wq_lock);
+       while (!TAILQ_EMPTY(&wq->wq_delayed)) {
+               struct delayed_work *const dw = TAILQ_FIRST(&wq->wq_delayed);
+
+               KASSERT(dw->work.work_queue == wq);
+               KASSERTMSG((dw->dw_state == DELAYED_WORK_SCHEDULED ||
+                       dw->dw_state == DELAYED_WORK_RESCHEDULED ||
+                       dw->dw_state == DELAYED_WORK_CANCELLED),
+                   "delayed work %p in bad state: %d",
+                   dw, dw->dw_state);
 
-               mutex_enter(&wq->wq_lock);
-               if (!TAILQ_EMPTY(&wq->wq_delayed)) {
-                       dw = TAILQ_FIRST(&wq->wq_delayed);
-                       if (!callout_halt(&dw->dw_callout, &wq->wq_lock))
-                               TAILQ_REMOVE(&wq->wq_delayed, dw, dw_entry);
-               }
-               mutex_exit(&wq->wq_lock);
+               /*
+                * Mark it cancelled and try to stop the callout before
+                * it starts.
+                *
+                * If it's too late and the callout has already begun
+                * to execute, then it will notice that we asked to
+                * cancel it and remove itself from the queue before
+                * returning.
+                *
+                * If we stopped the callout before it started,
+                * however, then we can safely destroy the callout and
+                * dissociate it from the workqueue ourselves.
+                */
+               dw->dw_state = DELAYED_WORK_CANCELLED;
+               if (!callout_halt(&dw->dw_callout, &wq->wq_lock))
+                       cancel_delayed_work_done(wq, dw);
+       }
+       mutex_exit(&wq->wq_lock);
 
-               if (dw == NULL)
-                       break;
-               cancel_delayed_work_sync(dw);
-       }
+       /*
+        * At this point, no new work can be put on the queue.
+        */
 
        /* Tell the thread to exit.  */
        mutex_enter(&wq->wq_lock);
@@ -225,11 +243,16 @@
 
        mutex_enter(&wq->wq_lock);
        for (;;) {
-               /* Wait until there's activity.  If we're dying, stop.  */
+               /*
+                * Wait until there's activity.  If there's no work and
+                * we're dying, stop here.
+                */
                while (TAILQ_EMPTY(&wq->wq_queue) && !wq->wq_dying)
                        cv_wait(&wq->wq_cv, &wq->wq_lock);
-               if (wq->wq_dying)
+               if (TAILQ_EMPTY(&wq->wq_queue)) {
+                       KASSERT(wq->wq_dying);
                        break;
+               }
 
                /* Grab a batch of work off the queue.  */
                KASSERT(!TAILQ_EMPTY(&wq->wq_queue));