diff --git a/sys/dev/drm/include/linux/workqueue.h b/sys/dev/drm/include/linux/workqueue.h --- a/sys/dev/drm/include/linux/workqueue.h +++ b/sys/dev/drm/include/linux/workqueue.h @@ -54,6 +54,7 @@ struct workqueue_worker { STAILQ_HEAD(ws_list, work_struct) ws_list_head; struct thread *worker_thread; + struct work_struct *running_work; /* owner of in-flight callback */ struct lock worker_lock; }; diff --git a/sys/dev/drm/linux_workqueue.c b/sys/dev/drm/linux_workqueue.c --- a/sys/dev/drm/linux_workqueue.c +++ b/sys/dev/drm/linux_workqueue.c @@ -62,6 +62,34 @@ * - unbound high priority */ +/* + * work_is_executing() - authoritative "queued or mid-callback" test. + * + * The worker-owned worker->running_work pointer is authoritative because + * work->running is no longer cleared after the callback returns: a work + * item may legally free itself inside func() (self-freeing works are + * standard Linux semantics, e.g. i915_clflush_work()), and + * process_all_work() must not touch *work once func() has returned. + */ +static bool +work_is_executing(struct work_struct *work) +{ + struct workqueue_worker *worker; + bool executing; + + if (work->on_queue) + return (true); + if (work->running == false) + return (false); + worker = work->worker; + if (worker == NULL) + return (true); /* conservatively: about to start */ + lockmgr(&worker->worker_lock, LK_EXCLUSIVE); + executing = (worker->running_work == work); + lockmgr(&worker->worker_lock, LK_RELEASE); + return (executing); +} + static inline void process_all_work(struct workqueue_worker *worker) { @@ -74,7 +102,7 @@ work->on_queue = false; /* A work shouldn't be executed concurrently on a single cpu */ - if (work->running) + if (work_is_executing(work)) continue; /* Do not run canceled works */ @@ -84,16 +112,32 @@ continue; } + /* + * Snapshot cancel state before the callback: after func() + * returns we may no longer touch *work at all (it may have + * freed itself). running_work is set before running so any + * locked reader that observes running=true also observes + * running_work==work. + */ + didcan = work->canceled; + worker->running_work = work; work->running = true; lockmgr(&worker->worker_lock, LK_RELEASE); work->func(work); lwkt_yield(); lockmgr(&worker->worker_lock, LK_EXCLUSIVE); - if (work->on_queue == false) - work->worker = NULL; - didcan = work->canceled; + + /* + * Completion bookkeeping is worker-owned; never touch *work + * past this point. wakeup() only uses the address as a + * wait-channel key, and the free-after-cancel API contract + * keeps the item alive for any cancel_work_sync() waiter + * until woken (this covers cancels requested before func(); + * cancels during func() are picked up by the waiter's own + * 1-tick retest loop). + */ + worker->running_work = NULL; cpu_sfence(); - work->running = false; if (didcan == true) wakeup(work); } @@ -295,7 +339,7 @@ unsigned int work_busy(struct work_struct *work) { - return (work->on_queue || work->running); + return (work_is_executing(work)); } static inline void @@ -313,7 +357,7 @@ INIT_WORK(&__flush_work, __flush_work_func); queue_work(wq, &__flush_work); - while (__flush_work.on_queue || __flush_work.running) { + while (work_is_executing(&__flush_work)) { tsleep(&__flush_work, 0, "flshwq", 0); } } @@ -328,7 +372,7 @@ int ret = false; /* XXX: probably unreliable */ - while (work->on_queue || work->running) { + while (work_is_executing(work)) { ret = true; /* XXX: use something more intelligent than tsleep() */ tsleep(&flush_work, 0, "flshwrk", 1); @@ -361,14 +405,14 @@ ret = true; lockmgr(&worker->worker_lock, LK_RELEASE); } - if (work->running == false) + if (work_is_executing(work) == false) break; worker = work->worker; if (worker == NULL) continue; lockmgr(&worker->worker_lock, LK_EXCLUSIVE); - if (worker != work->worker || work->running == false) { + if (worker != work->worker || worker->running_work != work) { lockmgr(&worker->worker_lock, LK_RELEASE); continue; }