[Beignet] [PATCH 2/4] runtime: refine the last_event in queue to a list

Pan Xiuli xiuli.pan at intel.com
Thu Sep 24 02:13:25 PDT 2015


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 <xiuli.pan at intel.com>
---
 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



More information about the Beignet mailing list