Refine the evetn struct to make last_event become a list to store all uncompelete and update them every queue flush. This can make sure all events created in the runtime have a chance for updating status and upcoming callback functions and delete. We will also need not to worry about other memory leak casued by leaked events. This is also a bugfix for https://bugs.freedesktop.org/show_bug.cgi?id=91710 the memory leak of these cases is caused by unrefenced drm buffer which is still reffered by leaked uncompleted events.
Signed-off-by: Pan Xiuli <[email protected]> --- src/cl_command_queue.c | 25 +++++++------------ src/cl_event.c | 66 +++++++++++++++++++++++++++++++++++++++++--------- src/cl_event.h | 9 +++++-- 3 files changed, 71 insertions(+), 29 deletions(-) diff --git a/src/cl_command_queue.c b/src/cl_command_queue.c index 4b92311..50436fc 100644 --- a/src/cl_command_queue.c +++ b/src/cl_command_queue.c @@ -76,11 +76,9 @@ cl_command_queue_delete(cl_command_queue queue) assert(queue); if (atomic_dec(&queue->ref_n) != 1) return; - // If there is a valid last event, we need to give it a chance to - // call the call-back function. - cl_event last_event = get_last_event(queue); - if (last_event && last_event->user_cb) - cl_event_update_status(last_event, 1); + // If there is a list of valid events, we need to give them + // a chance to call the call-back function. + cl_event_update_last_events(queue,1); /* Remove it from the list */ assert(queue->ctx); pthread_mutex_lock(&queue->ctx->queue_lock); @@ -255,14 +253,11 @@ cl_command_queue_flush(cl_command_queue queue) int err; GET_QUEUE_THREAD_GPGPU(queue); err = cl_command_queue_flush_gpgpu(queue, gpgpu); - // As we don't have a deadicate timer thread to take care the possible - // event which has a call back function registerred and the event will - // 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. - cl_event last_event = get_last_event(queue); - if (last_event && last_event->user_cb) - cl_event_update_status(last_event, 1); + // We now keep a list of uncompleted events and check if they compelte + // every flush. This can make sure all events created have chance to be + // update status, so the callback functions or reference can be handled. + cl_event_update_last_events(queue,0); + cl_event current_event = get_current_event(queue); if (current_event && err == CL_SUCCESS) { err = cl_event_flush(current_event); @@ -276,9 +271,7 @@ LOCAL cl_int cl_command_queue_finish(cl_command_queue queue) { cl_gpgpu_sync(cl_get_thread_batch_buf(queue)); - cl_event last_event = get_last_event(queue); - if (last_event) - cl_event_update_status(last_event, 1); + cl_event_update_last_events(queue,1); return CL_SUCCESS; } diff --git a/src/cl_event.c b/src/cl_event.c index bf44197..74c1700 100644 --- a/src/cl_event.c +++ b/src/cl_event.c @@ -28,6 +28,46 @@ #include <assert.h> #include <stdio.h> +void cl_event_update_last_events(cl_command_queue queue, int wait) +{ + cl_event last_event = get_last_event(queue); + if(!last_event) return; + cl_event prev, next, now , ret; + ret = last_event; + now = last_event; + while(now){ + next = now->last_next;//get prev and next first + prev = now->last_prev;//in case now is freed + /* check if completed event delete did maintain for us, or we do */ + if(cl_event_update_status(now,wait)){ + if(!prev){ + ret = next; + } + else if(prev->last_next != next){ + prev->last_next = next; + } + if (next) + next->last_prev = prev; + } + now = next; + } + set_last_event(queue,ret); +} + +void cl_event_insert_last_events(cl_command_queue queue,cl_event event) +{ + if(!event) return; + cl_event last_event = get_last_event(queue); + if(last_event){ + cl_event now = last_event; + while(now->last_next) + now = now->last_next; + now->last_next = event; + event->last_prev = now; + } + else set_last_event(queue,event); +} + inline cl_bool cl_event_is_gpu_command_type(cl_command_type type) { @@ -56,7 +96,7 @@ int cl_event_flush(cl_event event) event->gpgpu = NULL; } cl_gpgpu_event_flush(event->gpgpu_event); - set_last_event(event->queue, event); + cl_event_insert_last_events(event->queue,event); return err; } @@ -117,8 +157,13 @@ void cl_event_delete(cl_event event) if (atomic_dec(&event->ref_n) > 1) return; + /* Maintain the last_list before freed */ + if (event->last_prev) + event->last_prev->last_next = event->last_next; + if (event->last_next) + event->last_next->last_prev = event->last_prev; if(event->queue && get_last_event(event->queue) == event) - set_last_event(event->queue, NULL); + set_last_event(event->queue, event->last_next); /* 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 @@ -529,13 +574,16 @@ void cl_event_set_status(cl_event event, cl_int status) cl_event_delete(event); } -void cl_event_update_status(cl_event event, int wait) +int cl_event_update_status(cl_event event, int wait) { if(event->status <= CL_COMPLETE) - return; + return 1; if((event->gpgpu_event) && - (cl_gpgpu_event_update_status(event->gpgpu_event, wait) == command_complete)) + (cl_gpgpu_event_update_status(event->gpgpu_event, wait) == command_complete)){ cl_event_set_status(event, CL_COMPLETE); + return 1; + } + return 0; } cl_int cl_event_marker_with_wait_list(cl_command_queue queue, @@ -568,9 +616,7 @@ cl_int cl_event_marker_with_wait_list(cl_command_queue queue, return CL_SUCCESS; } - 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_update_last_events(queue,1); cl_event_set_status(e, CL_COMPLETE); return CL_SUCCESS; @@ -605,9 +651,7 @@ cl_int cl_event_barrier_with_wait_list(cl_command_queue queue, return CL_SUCCESS; } - 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_update_last_events(queue,1); cl_event_set_status(e, CL_COMPLETE); return CL_SUCCESS; diff --git a/src/cl_event.h b/src/cl_event.h index f7bf09f..dcaaeaf 100644 --- a/src/cl_event.h +++ b/src/cl_event.h @@ -71,6 +71,7 @@ struct _cl_event { cl_bool emplict; /* Identify this event whether created by api emplict*/ cl_ulong timestamp[4];/* The time stamps for profiling. */ cl_ulong queued_timestamp; + cl_event last_next, last_prev;/* We need a list to monitor untouchable api event*/ }; /* Create a new event object */ @@ -91,8 +92,8 @@ cl_int cl_event_wait_events(cl_uint, const cl_event *, cl_command_queue); void cl_event_new_enqueue_callback(cl_event, enqueue_data *, cl_uint, const cl_event *); /* Set the event status and call all callbacks */ void cl_event_set_status(cl_event, cl_int); -/* Check and update event status */ -void cl_event_update_status(cl_event, cl_int); +/* Check and update event status, return 1 for complete events */ +int cl_event_update_status(cl_event, cl_int); /* Create the marker event */ cl_int cl_event_marker_with_wait_list(cl_command_queue, cl_uint, const cl_event *, cl_event*); /* Create the barrier event */ @@ -115,5 +116,9 @@ cl_int cl_event_insert_user_event(user_event** p_u_ev, cl_event event); cl_int cl_event_remove_user_event(user_event** p_u_ev, cl_event event); /* flush the event's pending gpgpu batch buffer and notify driver this gpgpu event has been flushed. */ cl_int cl_event_flush(cl_event event); +/* monitor or block wait all events in the last_event list */ +void cl_event_update_last_events(cl_command_queue queuet, int wait); +/* insert the event into the last_event list in queue */ +void cl_event_insert_last_events(cl_command_queue queue, cl_event event); #endif /* __CL_EVENT_H__ */ -- 2.1.4 _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
