Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/external/bsd/common Rework Linux workqueue synchronizati...



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

description:
Rework Linux workqueue synchronization yet again.

- Use a low bit in the pointer to the queue, rather than whether the
  pointer is null or not, to determine whether the work item is
  queued/scheduled.

- _Preserve_ the pointer to the queue after we release the work.

- Release the work _before_ executing the function, not after.

This simplifies some things: we no longer have to distinguish whether
the work is queued or running in the functions where we are trying to
modify it.  The pointer has to be preserved even after the work is
released so that we can flush the workqueue after the work has been
released.

diffstat:

 sys/external/bsd/common/include/linux/workqueue.h |    4 +-
 sys/external/bsd/common/linux/linux_work.c        |  586 +++++++++------------
 2 files changed, 257 insertions(+), 333 deletions(-)

diffs (truncated from 1014 to 300 lines):

diff -r 93d718ebd1bd -r b85d8051b452 sys/external/bsd/common/include/linux/workqueue.h
--- a/sys/external/bsd/common/include/linux/workqueue.h Mon Aug 27 15:05:44 2018 +0000
+++ b/sys/external/bsd/common/include/linux/workqueue.h Mon Aug 27 15:06:02 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: workqueue.h,v 1.12 2018/08/27 15:05:01 riastradh Exp $ */
+/*     $NetBSD: workqueue.h,v 1.13 2018/08/27 15:06:02 riastradh Exp $ */
 
 /*-
  * Copyright (c) 2013, 2018 The NetBSD Foundation, Inc.
@@ -63,7 +63,7 @@
 struct workqueue_struct;
 
 struct work_struct {
-       struct workqueue_struct *volatile work_queue;
+       volatile uintptr_t              work_owner;
        TAILQ_ENTRY(work_struct)        work_entry;
        void    (*func)(struct work_struct *); /* Linux API name */
 };
diff -r 93d718ebd1bd -r b85d8051b452 sys/external/bsd/common/linux/linux_work.c
--- a/sys/external/bsd/common/linux/linux_work.c        Mon Aug 27 15:05:44 2018 +0000
+++ b/sys/external/bsd/common/linux/linux_work.c        Mon Aug 27 15:06:02 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: linux_work.c,v 1.38 2018/08/27 15:05:44 riastradh Exp $        */
+/*     $NetBSD: linux_work.c,v 1.39 2018/08/27 15:06:02 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.38 2018/08/27 15:05:44 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: linux_work.c,v 1.39 2018/08/27 15:06:02 riastradh Exp $");
 
 #include <sys/types.h>
 #include <sys/atomic.h>
@@ -45,23 +45,29 @@
 
 #include <linux/workqueue.h>
 
+TAILQ_HEAD(work_head, work_struct);
+TAILQ_HEAD(dwork_head, delayed_work);
+
 struct workqueue_struct {
-       kmutex_t                        wq_lock;
-       kcondvar_t                      wq_cv;
-       TAILQ_HEAD(, delayed_work)      wq_delayed;
-       TAILQ_HEAD(, work_struct)       wq_queue;
-       struct work_struct              *wq_current_work;
-       int                             wq_flags;
-       bool                            wq_requeued;
-       bool                            wq_dying;
-       uint64_t                        wq_gen;
-       struct lwp                      *wq_lwp;
+       kmutex_t                wq_lock;
+       kcondvar_t              wq_cv;
+       struct dwork_head       wq_delayed; /* delayed work scheduled */
+       struct work_head        wq_queue;   /* work to run */
+       struct work_head        wq_dqueue;  /* delayed work to run now */
+       struct work_struct      *wq_current_work;
+       int                     wq_flags;
+       bool                    wq_dying;
+       uint64_t                wq_gen;
+       struct lwp              *wq_lwp;
 };
 
 static void __dead     linux_workqueue_thread(void *);
 static void            linux_workqueue_timeout(void *);
+static bool            work_claimed(struct work_struct *,
+                           struct workqueue_struct *);
 static struct workqueue_struct *
-                       acquire_work(struct work_struct *,
+                       work_queue(struct work_struct *);
+static bool            acquire_work(struct work_struct *,
                            struct workqueue_struct *);
 static void            release_work(struct work_struct *,
                            struct workqueue_struct *);
@@ -80,6 +86,13 @@
 struct workqueue_struct        *system_long_wq __read_mostly;
 struct workqueue_struct        *system_power_efficient_wq __read_mostly;
 
+static inline uintptr_t
+atomic_cas_uintptr(volatile uintptr_t *p, uintptr_t old, uintptr_t new)
+{
+
+       return (uintptr_t)atomic_cas_ptr(p, (void *)old, (void *)new);
+}
+
 /*
  * linux_workqueue_init()
  *
@@ -164,9 +177,9 @@
        cv_init(&wq->wq_cv, name);
        TAILQ_INIT(&wq->wq_delayed);
        TAILQ_INIT(&wq->wq_queue);
+       TAILQ_INIT(&wq->wq_dqueue);
        wq->wq_current_work = NULL;
        wq->wq_flags = 0;
-       wq->wq_requeued = false;
        wq->wq_dying = false;
        wq->wq_gen = 0;
        wq->wq_lwp = NULL;
@@ -179,7 +192,8 @@
 
        return wq;
 
-fail0: KASSERT(TAILQ_EMPTY(&wq->wq_queue));
+fail0: KASSERT(TAILQ_EMPTY(&wq->wq_dqueue));
+       KASSERT(TAILQ_EMPTY(&wq->wq_queue));
        KASSERT(TAILQ_EMPTY(&wq->wq_delayed));
        cv_destroy(&wq->wq_cv);
        mutex_destroy(&wq->wq_lock);
@@ -208,7 +222,7 @@
        while (!TAILQ_EMPTY(&wq->wq_delayed)) {
                struct delayed_work *const dw = TAILQ_FIRST(&wq->wq_delayed);
 
-               KASSERT(dw->work.work_queue == wq);
+               KASSERT(work_queue(&dw->work) == wq);
                KASSERTMSG((dw->dw_state == DELAYED_WORK_SCHEDULED ||
                        dw->dw_state == DELAYED_WORK_RESCHEDULED ||
                        dw->dw_state == DELAYED_WORK_CANCELLED),
@@ -248,9 +262,9 @@
        (void)kthread_join(wq->wq_lwp);
 
        KASSERT(wq->wq_dying);
-       KASSERT(!wq->wq_requeued);
        KASSERT(wq->wq_flags == 0);
        KASSERT(wq->wq_current_work == NULL);
+       KASSERT(TAILQ_EMPTY(&wq->wq_dqueue));
        KASSERT(TAILQ_EMPTY(&wq->wq_queue));
        KASSERT(TAILQ_EMPTY(&wq->wq_delayed));
        cv_destroy(&wq->wq_cv);
@@ -275,7 +289,9 @@
 linux_workqueue_thread(void *cookie)
 {
        struct workqueue_struct *const wq = cookie;
-       TAILQ_HEAD(, work_struct) tmp;
+       struct work_head queue, dqueue;
+       struct work_head *const q[2] = { &queue, &dqueue };
+       unsigned i;
 
        lwp_setspecific(workqueue_key, wq);
 
@@ -285,39 +301,52 @@
                 * 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)
+               while (TAILQ_EMPTY(&wq->wq_queue) &&
+                   TAILQ_EMPTY(&wq->wq_dqueue) &&
+                   !wq->wq_dying)
                        cv_wait(&wq->wq_cv, &wq->wq_lock);
-               if (TAILQ_EMPTY(&wq->wq_queue)) {
-                       KASSERT(wq->wq_dying);
+               if (wq->wq_dying) {
+                       KASSERT(TAILQ_EMPTY(&wq->wq_queue));
+                       KASSERT(TAILQ_EMPTY(&wq->wq_dqueue));
                        break;
                }
 
                /* Grab a batch of work off the queue.  */
-               KASSERT(!TAILQ_EMPTY(&wq->wq_queue));
-               TAILQ_INIT(&tmp);
-               TAILQ_CONCAT(&tmp, &wq->wq_queue, work_entry);
+               TAILQ_INIT(&queue);
+               TAILQ_INIT(&dqueue);
+               TAILQ_CONCAT(&queue, &wq->wq_queue, work_entry);
+               TAILQ_CONCAT(&dqueue, &wq->wq_dqueue, work_entry);
 
                /* Process each work item in the batch.  */
-               while (!TAILQ_EMPTY(&tmp)) {
-                       struct work_struct *const work = TAILQ_FIRST(&tmp);
+               for (i = 0; i < 2; i++) {
+                       while (!TAILQ_EMPTY(q[i])) {
+                               struct work_struct *work = TAILQ_FIRST(q[i]);
+                               void (*func)(struct work_struct *);
 
-                       KASSERT(work->work_queue == wq);
-                       TAILQ_REMOVE(&tmp, work, work_entry);
-                       KASSERT(wq->wq_current_work == NULL);
-                       wq->wq_current_work = work;
+                               KASSERT(work_queue(work) == wq);
+                               KASSERT(work_claimed(work, wq));
+                               KASSERTMSG((q[i] != &dqueue ||
+                                       container_of(work, struct delayed_work,
+                                           work)->dw_state ==
+                                       DELAYED_WORK_IDLE),
+                                   "delayed work %p queued and scheduled",
+                                   work);
 
-                       mutex_exit(&wq->wq_lock);
-                       (*work->func)(work);
-                       mutex_enter(&wq->wq_lock);
+                               TAILQ_REMOVE(q[i], work, work_entry);
+                               KASSERT(wq->wq_current_work == NULL);
+                               wq->wq_current_work = work;
+                               func = work->func;
+                               release_work(work, wq);
+                               /* Can't dereference work after this point.  */
 
-                       KASSERT(wq->wq_current_work == work);
-                       KASSERT(work->work_queue == wq);
-                       if (wq->wq_requeued)
-                               wq->wq_requeued = false;
-                       else
-                               release_work(work, wq);
-                       wq->wq_current_work = NULL;
-                       cv_broadcast(&wq->wq_cv);
+                               mutex_exit(&wq->wq_lock);
+                               (*func)(work);
+                               mutex_enter(&wq->wq_lock);
+
+                               KASSERT(wq->wq_current_work == work);
+                               wq->wq_current_work = NULL;
+                               cv_broadcast(&wq->wq_cv);
+                       }
                }
 
                /* Notify flush that we've completed a batch of work.  */
@@ -343,18 +372,20 @@
 linux_workqueue_timeout(void *cookie)
 {
        struct delayed_work *const dw = cookie;
-       struct workqueue_struct *const wq = dw->work.work_queue;
+       struct workqueue_struct *const wq = work_queue(&dw->work);
 
-       KASSERT(wq != NULL);
+       KASSERTMSG(wq != NULL,
+           "delayed work %p state %d resched %d",
+           dw, dw->dw_state, dw->dw_resched);
 
        mutex_enter(&wq->wq_lock);
-       KASSERT(dw->work.work_queue == wq);
+       KASSERT(work_queue(&dw->work) == wq);
        switch (dw->dw_state) {
        case DELAYED_WORK_IDLE:
                panic("delayed work callout uninitialized: %p", dw);
        case DELAYED_WORK_SCHEDULED:
                dw_callout_destroy(wq, dw);
-               TAILQ_INSERT_TAIL(&wq->wq_queue, &dw->work, work_entry);
+               TAILQ_INSERT_TAIL(&wq->wq_dqueue, &dw->work, work_entry);
                cv_broadcast(&wq->wq_cv);
                break;
        case DELAYED_WORK_RESCHEDULED:
@@ -365,7 +396,7 @@
                break;
        case DELAYED_WORK_CANCELLED:
                cancel_delayed_work_done(wq, dw);
-               /* Can't touch dw any more.  */
+               /* Can't dereference dw after this point.  */
                goto out;
        default:
                panic("delayed work callout in bad state: %p", dw);
@@ -412,34 +443,72 @@
 INIT_WORK(struct work_struct *work, void (*fn)(struct work_struct *))
 {
 
-       work->work_queue = NULL;
+       work->work_owner = 0;
        work->func = fn;
 }
 
 /*
+ * work_claimed(work, wq)
+ *
+ *     True if work is currently claimed by a workqueue, meaning it is
+ *     either on the queue or scheduled in a callout.  The workqueue
+ *     must be wq, and caller must hold wq's lock.
+ */
+static bool
+work_claimed(struct work_struct *work, struct workqueue_struct *wq)
+{
+
+       KASSERT(work_queue(work) == wq);
+       KASSERT(mutex_owned(&wq->wq_lock));
+
+       return work->work_owner & 1;
+}
+
+/*
+ * work_queue(work)
+ *
+ *     Return the last queue that work was queued on, or NULL if it
+ *     was never queued.
+ */
+static struct workqueue_struct *
+work_queue(struct work_struct *work)
+{
+
+       return (struct workqueue_struct *)(work->work_owner & ~(uintptr_t)1);
+}
+
+/*
  * acquire_work(work, wq)
  *
- *     Try to associate work with wq.  If work is already on a
- *     workqueue, return that workqueue.  Otherwise, set work's queue
- *     to wq, issue a memory barrier to match any prior release_work,
- *     and return NULL.
+ *     Try to claim work for wq.  If work is already claimed, it must



Home | Main Index | Thread Index | Old Index