[Mesa-dev] [PATCH 01/11] util/u_queue: add a way to remove a job when we just want to destroy it

Nicolai Hähnle nhaehnle at gmail.com
Wed Jun 7 11:06:54 UTC 2017


On 01.06.2017 20:18, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> ---
>   src/util/u_queue.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
>   src/util/u_queue.h |  2 ++
>   2 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/src/util/u_queue.c b/src/util/u_queue.c
> index 8db09b0..3834b6f 100644
> --- a/src/util/u_queue.c
> +++ b/src/util/u_queue.c
> @@ -173,27 +173,29 @@ util_queue_thread_func(void *input)
>         if (job.job) {
>            job.execute(job.job, thread_index);
>            util_queue_fence_signal(job.fence);
>            if (job.cleanup)
>               job.cleanup(job.job, thread_index);
>         }
>      }
>   
>      /* signal remaining jobs before terminating */
>      mtx_lock(&queue->lock);
> -   while (queue->jobs[queue->read_idx].job) {
> -      util_queue_fence_signal(queue->jobs[queue->read_idx].fence);
> -
> -      queue->jobs[queue->read_idx].job = NULL;
> -      queue->read_idx = (queue->read_idx + 1) % queue->max_jobs;
> +   for (unsigned i = queue->read_idx; i != queue->write_idx;
> +        i = (i + 1) % queue->max_jobs) {
> +      if (queue->jobs[i].job) {
> +         util_queue_fence_signal(queue->jobs[i].fence);
> +         queue->jobs[i].job = NULL;
> +      }
>      }
> -   queue->num_queued = 0; /* reset this when exiting the thread */
> +   queue->read_idx = (queue->read_idx + queue->num_queued) % queue->max_jobs;

I'd prefer:

    queue->read_idx = queue->write_idx;

which should always be the same thing.


> +   queue->num_queued = 0;
>      mtx_unlock(&queue->lock);
>      return 0;
>   }
>   
>   bool
>   util_queue_init(struct util_queue *queue,
>                   const char *name,
>                   unsigned max_jobs,
>                   unsigned num_threads)
>   {
> @@ -322,19 +324,55 @@ util_queue_add_job(struct util_queue *queue,
>      ptr->fence = fence;
>      ptr->execute = execute;
>      ptr->cleanup = cleanup;
>      queue->write_idx = (queue->write_idx + 1) % queue->max_jobs;
>   
>      queue->num_queued++;
>      cnd_signal(&queue->has_queued_cond);
>      mtx_unlock(&queue->lock);
>   }
>   
> +/**
> + * Remove a queued job. If the job hasn't started execution, it's removed from
> + * the queue. If the job has started execution, the function waits for it to
> + * complete.
> + *
> + * In all cases, the fence is signalled when the function returns.
> + *
> + * The function can be used when destroying an object associated with the job
> + * when you don't care about the job completion state.
> + */
> +void
> +util_queue_drop_job(struct util_queue *queue, struct util_queue_fence *fence)
> +{
> +   bool removed = false;
> +
> +   if (util_queue_fence_is_signalled(fence))
> +      return;
> +
> +   mtx_lock(&queue->lock);
> +   for (unsigned i = queue->read_idx; i != queue->write_idx;
> +        i = (i + 1) % queue->max_jobs) {
> +      if (queue->jobs[i].fence == fence) {

Please move the hunk from patch #2 here to this patch.

Apart from those two points, the series is

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>


> +         /* Just clear it. The threads will drop it. */
> +         memset(&queue->jobs[i], 0, sizeof(queue->jobs[i]));
> +         removed = true;
> +         break;
> +      }
> +   }
> +   mtx_unlock(&queue->lock);
> +
> +   if (removed)
> +      util_queue_fence_signal(fence);
> +   else
> +      util_queue_fence_wait(fence);
> +}
> +
>   int64_t
>   util_queue_get_thread_time_nano(struct util_queue *queue, unsigned thread_index)
>   {
>      /* Allow some flexibility by not raising an error. */
>      if (thread_index >= queue->num_threads)
>         return 0;
>   
>      return u_thread_get_time_nano(queue->threads[thread_index]);
>   }
> diff --git a/src/util/u_queue.h b/src/util/u_queue.h
> index 4aec1f2..9876865 100644
> --- a/src/util/u_queue.h
> +++ b/src/util/u_queue.h
> @@ -85,20 +85,22 @@ bool util_queue_init(struct util_queue *queue,
>   void util_queue_destroy(struct util_queue *queue);
>   void util_queue_fence_init(struct util_queue_fence *fence);
>   void util_queue_fence_destroy(struct util_queue_fence *fence);
>   
>   /* optional cleanup callback is called after fence is signaled: */
>   void util_queue_add_job(struct util_queue *queue,
>                           void *job,
>                           struct util_queue_fence *fence,
>                           util_queue_execute_func execute,
>                           util_queue_execute_func cleanup);
> +void util_queue_drop_job(struct util_queue *queue,
> +                         struct util_queue_fence *fence);
>   
>   void util_queue_fence_wait(struct util_queue_fence *fence);
>   int64_t util_queue_get_thread_time_nano(struct util_queue *queue,
>                                           unsigned thread_index);
>   
>   /* util_queue needs to be cleared to zeroes for this to work */
>   static inline bool
>   util_queue_is_initialized(struct util_queue *queue)
>   {
>      return queue->threads != NULL;
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.


More information about the mesa-dev mailing list