[Qemu-devel] [PATCH] thread-pool: fix deadlock when callbacks depends on each other

[Marcin Gibuła]

When two coroutines submit I/O and first coroutine depends on second to 
complete (by calling bdrv_drain_all), deadlock may occur.

This is because both requests may have completed before thread pool 
notifier got called. Then, when notifier gets executed and first 
coroutine calls aio_pool() to make progress, it will hang forever, as 
notifier's descriptor has been already marked clear.

This patch fixes this, by rearming thread pool notifier if there are 
more than one completed requests on list.

Without this patch, I could reproduce this bug with snapshot-commit with 
about 1 per 10 tries. With this patch, I couldn't reproduce it any more.

Signed-off-by: Marcin Gibula <address@hidden>
---

--- thread-pool.c       2014-04-17 15:44:45.000000000 +0200
+++ thread-pool.c       2014-05-31 20:20:26.083011514 +0200
@@ -76,6 +76,8 @@ struct ThreadPool {
     int new_threads;     /* backlog of threads we need to create */
     int pending_threads; /* threads created but not running yet */
     int pending_cancellations; /* whether we need a cond_broadcast */
+    int pending_completions; /* whether we need to rearm notifier when
+                                executing callback */
     bool stopping;
 };

@@ -110,6 +112,10 @@ static void *worker_thread(void *opaque)
         ret = req->func(req->arg);

         req->ret = ret;
+        if (req->common.cb) {
+            pool->pending_completions++;
+        }
+
         /* Write ret before state.  */
         smp_wmb();
         req->state = THREAD_DONE;
@@ -185,6 +191,14 @@ restart:
         }
         if (elem->state == THREAD_DONE && elem->common.cb) {
             QLIST_REMOVE(elem, all);
+            /* If more completed requests are waiting, notifier needs
+             * to be rearmed so callback can progress with aio_pool().
+             */
+            pool->pending_completions--;
+            if (pool->pending_completions) {
+                event_notifier_set(notifier);
+            }
+
             /* Read state before ret.  */
             smp_rmb();
             elem->common.cb(elem->common.opaque, elem->ret);