[Beignet] [Patch v2 1/2] Fixed a thread safe bug.
Yang, Rong R
rong.r.yang at intel.com
Wed Jul 15 00:13:20 PDT 2015
The patchset LGTM, pushed, thanks.
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Zhigang Gong
> Sent: Wednesday, July 15, 2015 08:58
> To: beignet at lists.freedesktop.org
> Cc: Zhigang Gong
> Subject: [Beignet] [Patch v2 1/2] Fixed a thread safe bug.
>
> From: Zhigang Gong <zhigang.gong at linux.intel.com>
>
> last_event and current_event should be thread private data.
>
> Signed-off-by: Zhigang Gong <zhigang.gong at linux.intel.com>
> ---
> src/cl_api.c | 2 +-
> src/cl_command_queue.c | 17 +++++++++++------
> src/cl_command_queue.h | 2 --
> src/cl_event.c | 18 +++++++++---------
> src/cl_thread.c | 30 ++++++++++++++++++++++++++++++
> src/cl_thread.h | 5 +++++
> 6 files changed, 56 insertions(+), 18 deletions(-)
>
> diff --git a/src/cl_api.c b/src/cl_api.c index 1ba775f..3d79dcd 100644
> --- a/src/cl_api.c
> +++ b/src/cl_api.c
> @@ -85,7 +85,7 @@ handle_events(cl_command_queue queue, cl_int num,
> const cl_event *wait_list,
> cl_event_new_enqueue_callback(e, data, num, wait_list);
> }
> }
> - queue->current_event = e;
> + set_current_event(queue, e);
> return status;
> }
>
> diff --git a/src/cl_command_queue.c b/src/cl_command_queue.c index
> da71fb7..4e4ebfb 100644
> --- a/src/cl_command_queue.c
> +++ b/src/cl_command_queue.c
> @@ -78,8 +78,9 @@ cl_command_queue_delete(cl_command_queue
> queue)
>
> // If there is a valid last event, we need to give it a chance to
> // call the call-back function.
> - if (queue->last_event && queue->last_event->user_cb)
> - cl_event_update_status(queue->last_event, 1);
> + cl_event last_event = get_last_event(queue); if (last_event &&
> + last_event->user_cb)
> + cl_event_update_status(last_event, 1);
> /* Remove it from the list */
> assert(queue->ctx);
> pthread_mutex_lock(&queue->ctx->queue_lock);
> @@ -259,10 +260,14 @@ cl_command_queue_flush(cl_command_queue
> queue)
> // be released at the call back function, no other function will access
> // the event any more. If we don't do this here, we will leak that event
> // and all the corresponding buffers which is really bad.
> - if (queue->last_event && queue->last_event->user_cb)
> - cl_event_update_status(queue->last_event, 1);
> - if (queue->current_event && err == CL_SUCCESS)
> - err = cl_event_flush(queue->current_event);
> + cl_event last_event = get_last_event(queue); if (last_event &&
> + last_event->user_cb)
> + cl_event_update_status(last_event, 1); cl_event current_event =
> + get_current_event(queue); if (current_event && err == CL_SUCCESS) {
> + err = cl_event_flush(current_event);
> + set_current_event(queue, NULL);
> + }
> cl_invalid_thread_gpgpu(queue);
> return err;
> }
> diff --git a/src/cl_command_queue.h b/src/cl_command_queue.h index
> 91c941c..2cd6739 100644
> --- a/src/cl_command_queue.h
> +++ b/src/cl_command_queue.h
> @@ -40,8 +40,6 @@ struct _cl_command_queue {
> cl_event* wait_events; /* Point to array of non-complete user
> events that block this command queue */
> cl_int wait_events_num; /* Number of Non-complete user events
> */
> cl_int wait_events_size; /* The size of array that wait_events point to
> */
> - cl_event last_event; /* The last event in the queue, for enqueue
> mark used */
> - cl_event current_event; /* Current event. */
> cl_command_queue_properties props; /* Queue properties */
> cl_command_queue prev, next; /* We chain the command queues
> together */
> void *thread_data; /* Used to store thread context data */
> diff --git a/src/cl_event.c b/src/cl_event.c index b4734b2..56778ad 100644
> --- a/src/cl_event.c
> +++ b/src/cl_event.c
> @@ -56,7 +56,7 @@ int cl_event_flush(cl_event event)
> event->gpgpu = NULL;
> }
> cl_gpgpu_event_flush(event->gpgpu_event);
> - event->queue->last_event = event;
> + set_last_event(event->queue, event);
> return err;
> }
>
> @@ -117,8 +117,8 @@ void cl_event_delete(cl_event event)
> if (atomic_dec(&event->ref_n) > 1)
> return;
>
> - if(event->queue && event->queue->last_event == event)
> - event->queue->last_event = NULL;
> + if(event->queue && get_last_event(event->queue) == event)
> + set_last_event(event->queue, NULL);
>
> /* Call all user's callback if haven't execute */
> cl_event_call_callback(event, CL_COMPLETE, CL_TRUE); // CL_COMPLETE
> status will force all callbacks that are not executed to run @@ -568,9 +568,9
> @@ cl_int cl_event_marker_with_wait_list(cl_command_queue queue,
> return CL_SUCCESS;
> }
>
> - if(queue->last_event && queue->last_event->gpgpu_event) {
> - cl_gpgpu_event_update_status(queue->last_event->gpgpu_event, 1);
> - }
> + cl_event last_event = get_last_event(queue); if(last_event &&
> + last_event->gpgpu_event)
> + cl_gpgpu_event_update_status(last_event->gpgpu_event, 1);
>
> cl_event_set_status(e, CL_COMPLETE);
> return CL_SUCCESS;
> @@ -605,9 +605,9 @@ cl_int
> cl_event_barrier_with_wait_list(cl_command_queue queue,
> return CL_SUCCESS;
> }
>
> - if(queue->last_event && queue->last_event->gpgpu_event) {
> - cl_gpgpu_event_update_status(queue->last_event->gpgpu_event, 1);
> - }
> + cl_event last_event = get_last_event(queue); if(last_event &&
> + last_event->gpgpu_event)
> + cl_gpgpu_event_update_status(last_event->gpgpu_event, 1);
>
> cl_event_set_status(e, CL_COMPLETE);
> return CL_SUCCESS;
> diff --git a/src/cl_thread.c b/src/cl_thread.c index 0d99574..5e5a351 100644
> --- a/src/cl_thread.c
> +++ b/src/cl_thread.c
> @@ -45,6 +45,8 @@ typedef struct _thread_spec_data {
> cl_gpgpu gpgpu ;
> int valid;
> void* thread_batch_buf;
> + cl_event last_event;
> + cl_event current_event;
> int thread_magic;
> } thread_spec_data;
>
> @@ -106,6 +108,34 @@ static thread_spec_data *
> __create_thread_spec_data(cl_command_queue queue, int
> return spec;
> }
>
> +cl_event get_current_event(cl_command_queue queue) {
> + thread_spec_data* spec = __create_thread_spec_data(queue, 1);
> + assert(spec && spec->thread_magic == thread_magic);
> + return spec->current_event;
> +}
> +
> +cl_event get_last_event(cl_command_queue queue) {
> + thread_spec_data* spec = __create_thread_spec_data(queue, 1);
> + assert(spec && spec->thread_magic == thread_magic);
> + return spec->last_event;
> +}
> +
> +void set_current_event(cl_command_queue queue, cl_event e) {
> + thread_spec_data* spec = __create_thread_spec_data(queue, 1);
> + assert(spec && spec->thread_magic == thread_magic);
> + spec->current_event = e;
> +}
> +
> +void set_last_event(cl_command_queue queue, cl_event e) {
> + thread_spec_data* spec = __create_thread_spec_data(queue, 1);
> + assert(spec && spec->thread_magic == thread_magic);
> + spec->last_event = e;
> +}
> +
> void* cl_thread_data_create(void)
> {
> queue_thread_private* thread_private = CALLOC(queue_thread_private);
> diff --git a/src/cl_thread.h b/src/cl_thread.h index 7b48a26..d77526b 100644
> --- a/src/cl_thread.h
> +++ b/src/cl_thread.h
> @@ -44,4 +44,9 @@ void* cl_get_thread_batch_buf(cl_command_queue
> queue);
> /* take current gpgpu from the thread gpgpu pool. */ cl_gpgpu
> cl_thread_gpgpu_take(cl_command_queue queue);
>
> +cl_event get_current_event(cl_command_queue queue); cl_event
> +get_last_event(cl_command_queue queue); void
> +set_current_event(cl_command_queue queue, cl_event e); void
> +set_last_event(cl_command_queue queue, cl_event e);
> +
> #endif /* __CL_THREAD_H__ */
> --
> 1.9.1
>
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list