The patchset LGTM, pushed, thanks.
> -----Original Message----- > From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of > Zhigang Gong > Sent: Wednesday, July 15, 2015 08:58 > To: beignet@lists.freedesktop.org > Cc: Zhigang Gong > Subject: [Beignet] [Patch v2 1/2] Fixed a thread safe bug. > > From: Zhigang Gong <zhigang.g...@linux.intel.com> > > last_event and current_event should be thread private data. > > Signed-off-by: Zhigang Gong <zhigang.g...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list Beignet@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet
