[Mesa-dev] [PATCH] util/u_queue: fix a deadlock in util_queue_finish
Nicolai Hähnle
nhaehnle at gmail.com
Fri Apr 27 08:35:16 UTC 2018
Nice catch!
Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
On 24.04.2018 23:06, Marek Olšák wrote:
> 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 */
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list