[Mesa-dev] [PATCH] util/u_queue: fix a deadlock in util_queue_finish

[Marek Olšák]

From: Marek Olšák <marek.olsak at amd.com>

Cc: 18.0 18.1 <mesa-stable at lists.freedesktop.org>
---
 src/util/u_queue.c | 9 +++++++++
 src/util/u_queue.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/src/util/u_queue.c b/src/util/u_queue.c
index dba23f96456..da513fd9cc5 100644
--- a/src/util/u_queue.c
+++ b/src/util/u_queue.c
@@ -304,20 +304,21 @@ util_queue_init(struct util_queue *queue,
    queue->flags = flags;
    queue->num_threads = num_threads;
    queue->max_jobs = max_jobs;
 
    queue->jobs = (struct util_queue_job*)
                  calloc(max_jobs, sizeof(struct util_queue_job));
    if (!queue->jobs)
       goto fail;
 
    (void) mtx_init(&queue->lock, mtx_plain);
+   (void) mtx_init(&queue->finish_lock, mtx_plain);
 
    queue->num_queued = 0;
    cnd_init(&queue->has_queued_cond);
    cnd_init(&queue->has_space_cond);
 
    queue->threads = (thrd_t*) calloc(num_threads, sizeof(thrd_t));
    if (!queue->threads)
       goto fail;
 
    /* start threads */
@@ -391,20 +392,21 @@ util_queue_killall_and_wait(struct util_queue *queue)
 }
 
 void
 util_queue_destroy(struct util_queue *queue)
 {
    util_queue_killall_and_wait(queue);
    remove_from_atexit_list(queue);
 
    cnd_destroy(&queue->has_space_cond);
    cnd_destroy(&queue->has_queued_cond);
+   mtx_destroy(&queue->finish_lock);
    mtx_destroy(&queue->lock);
    free(queue->jobs);
    free(queue->threads);
 }
 
 void
 util_queue_add_job(struct util_queue *queue,
                    void *job,
                    struct util_queue_fence *fence,
                    util_queue_execute_func execute,
@@ -522,29 +524,36 @@ util_queue_finish_execute(void *data, int num_thread)
  * Wait until all previously added jobs have completed.
  */
 void
 util_queue_finish(struct util_queue *queue)
 {
    util_barrier barrier;
    struct util_queue_fence *fences = malloc(queue->num_threads * sizeof(*fences));
 
    util_barrier_init(&barrier, queue->num_threads);
 
+   /* If 2 threads were adding jobs for 2 different barries at the same time,
+    * a deadlock would happen, because 1 barrier requires that all threads
+    * wait for it exclusively.
+    */
+   mtx_lock(&queue->finish_lock);
+
    for (unsigned i = 0; i < queue->num_threads; ++i) {
       util_queue_fence_init(&fences[i]);
       util_queue_add_job(queue, &barrier, &fences[i], util_queue_finish_execute, NULL);
    }
 
    for (unsigned i = 0; i < queue->num_threads; ++i) {
       util_queue_fence_wait(&fences[i]);
       util_queue_fence_destroy(&fences[i]);
    }
+   mtx_unlock(&queue->finish_lock);
 
    util_barrier_destroy(&barrier);
 
    free(fences);
 }
 
 int64_t
 util_queue_get_thread_time_nano(struct util_queue *queue, unsigned thread_index)
 {
    /* Allow some flexibility by not raising an error. */
diff --git a/src/util/u_queue.h b/src/util/u_queue.h
index d49f713e6ad..d702c4bce8d 100644
--- a/src/util/u_queue.h
+++ b/src/util/u_queue.h
@@ -193,20 +193,21 @@ typedef void (*util_queue_execute_func)(void *job, int thread_index);
 struct util_queue_job {
    void *job;
    struct util_queue_fence *fence;
    util_queue_execute_func execute;
    util_queue_execute_func cleanup;
 };
 
 /* Put this into your context. */
 struct util_queue {
    const char *name;
+   mtx_t finish_lock; /* only for util_queue_finish */
    mtx_t lock;
    cnd_t has_queued_cond;
    cnd_t has_space_cond;
    thrd_t *threads;
    unsigned flags;
    int num_queued;
    unsigned num_threads;
    int kill_threads;
    int max_jobs;
    int write_idx, read_idx; /* ring buffer pointers */
-- 
2.17.0