[Beignet] [PATCH] Don't leak memory on long chains of events

Rebecca N. Palmer rebecca_palmer at zoho.com
Sat Jul 21 18:19:38 UTC 2018


Delete event->depend_events when it is no longer needed, to allow
the event objects it refers to to be freed.

This avoids out-of-memory hangs in large dependency trees
(e.g. long iterative calculations):
https://launchpad.net/bugs/1354086

Signed-off-by: Rebecca N. Palmer <rebecca_palmer at zoho.com>
---
*Possibly* also https://bugs.freedesktop.org/show_bug.cgi?id=102509

pocl does something similar:
https://sources.debian.org/src/pocl/1.1-5/lib/CL/devices/common.c/?hl=722#L714
Neo instead has the pointers go the other way (from an event to
the events waiting for it), but that would be harder and riskier
to convert existing code to.

--- a/src/cl_event.c
+++ b/src/cl_event.c
@@ -183,6 +183,25 @@ cl_event_new(cl_context ctx, cl_command_
   return e;
 }
 
+/* This exists to prevent long chains of events from filling up memory (https://bugs.launchpad.net/ubuntu/+source/beignet/+bug/1354086).  Call only after the dependencies are complete, or failed and marked as such in this event's status, or when this event is being destroyed */
+LOCAL void
+cl_event_delete_depslist(cl_event event)
+{
+  CL_OBJECT_LOCK(event);
+  cl_event *old_depend_events = event->depend_events;
+  int depend_count = event->depend_event_num;
+  event->depend_event_num = 0;
+  event->depend_events = NULL;
+  CL_OBJECT_UNLOCK(event);
+  if (old_depend_events) {
+    assert(depend_count);
+    for (int i = 0; i < depend_count; i++) {
+      cl_event_delete(old_depend_events[i]);
+    }
+    cl_free(old_depend_events);
+  }
+}
+
 LOCAL void
 cl_event_delete(cl_event event)
 {
@@ -199,13 +218,7 @@ cl_event_delete(cl_event event)
 
   assert(list_node_out_of_list(&event->enqueue_node));
 
-  if (event->depend_events) {
-    assert(event->depend_event_num);
-    for (i = 0; i < event->depend_event_num; i++) {
-      cl_event_delete(event->depend_events[i]);
-    }
-    cl_free(event->depend_events);
-  }
+  cl_event_delete_depslist(event);
 
   /* Free all the callbacks. Last ref, no need to lock. */
   while (!list_empty(&event->callbacks)) {
@@ -565,8 +578,12 @@ cl_event_exec(cl_event event, cl_int exe
   assert(depend_status <= CL_COMPLETE || ignore_depends || exec_to_status == CL_QUEUED);
   if (depend_status < CL_COMPLETE) { // Error happend, cancel exec.
     ret = cl_event_set_status(event, depend_status);
+    cl_event_delete_depslist(event);
     return depend_status;
   }
+  if (depend_status == CL_COMPLETE) { // Avoid memory leak
+    cl_event_delete_depslist(event);
+  }
 
   if (cur_status <= exec_to_status) {
     return ret;
--- a/src/cl_event.h
+++ b/src/cl_event.h
@@ -44,7 +44,7 @@ typedef struct _cl_event {
   cl_command_type event_type; /* Event type. */
   cl_bool is_barrier;         /* Is this event a barrier */
   cl_int status;              /* The execution status */
-  cl_event *depend_events;    /* The events must complete before this. */
+  cl_event *depend_events;    /* The events must complete before this. May disappear after they have completed - see cl_event_delete_depslist*/
   cl_uint depend_event_num;   /* The depend events number. */
   list_head callbacks;        /* The events The event callback functions */
   list_node enqueue_node;     /* The node in the enqueue list. */



More information about the Beignet mailing list