[Beignet] [PATCH] Fix a event notify bug.

junyan.he at inbox.com junyan.he at inbox.com
Thu Jan 5 12:53:56 UTC 2017


From: Junyan He <junyan.he at intel.com>

When a event complete, we need to notify all the command_queue
within the same context. But sometime, some command_queue in
the context is already invalid.
Modify to ensure all the command_queue to be notified are
valid.

Signed-off-by: Junyan He <junyan.he at intel.com>
---
 src/cl_command_queue.c | 12 ++----------
 src/cl_context.c       | 11 ++++++++---
 src/cl_context.h       |  2 +-
 src/cl_event.c         | 50 ++++++++++++++++----------------------------------
 4 files changed, 27 insertions(+), 48 deletions(-)

diff --git a/src/cl_command_queue.c b/src/cl_command_queue.c
index 2d66550..aa371d0 100644
--- a/src/cl_command_queue.c
+++ b/src/cl_command_queue.c
@@ -81,19 +81,11 @@ cl_command_queue_delete(cl_command_queue queue)
   if (CL_OBJECT_DEC_REF(queue) > 1)
     return;
 
-  cl_command_queue_destroy_enqueue(queue);
-
-#ifdef HAS_CMRT
-  if (queue->cmrt_event != NULL)
-    cmrt_destroy_event(queue);
-#endif
+  cl_context_remove_queue(queue->ctx, queue);
 
-  // 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);
+  cl_command_queue_destroy_enqueue(queue);
 
   cl_mem_delete(queue->perf);
-  cl_context_remove_queue(queue->ctx, queue);
   if (queue->barrier_events) {
     cl_free(queue->barrier_events);
   }
diff --git a/src/cl_context.c b/src/cl_context.c
index 65e7909..3f2e757 100644
--- a/src/cl_context.c
+++ b/src/cl_context.c
@@ -46,9 +46,11 @@ cl_context_add_queue(cl_context ctx, cl_command_queue queue) {
   cl_context_add_ref(ctx);
 
   CL_OBJECT_LOCK(ctx);
+  while (ctx->queue_modify_disable) {
+    CL_OBJECT_WAIT_ON_COND(ctx);
+  }
   list_add_tail(&ctx->queues, &queue->base.node);
   ctx->queue_num++;
-  ctx->queue_cookie++;
   CL_OBJECT_UNLOCK(ctx);
 
   queue->ctx = ctx;
@@ -57,10 +59,13 @@ cl_context_add_queue(cl_context ctx, cl_command_queue queue) {
 LOCAL void
 cl_context_remove_queue(cl_context ctx, cl_command_queue queue) {
   assert(queue->ctx == ctx);
+
   CL_OBJECT_LOCK(ctx);
+  while (ctx->queue_modify_disable) {
+    CL_OBJECT_WAIT_ON_COND(ctx);
+  }
   list_node_del(&queue->base.node);
   ctx->queue_num--;
-  ctx->queue_cookie++;
   CL_OBJECT_UNLOCK(ctx);
 
   cl_context_delete(ctx);
@@ -333,7 +338,7 @@ cl_context_new(struct _cl_context_prop *props, cl_uint dev_num, cl_device_id* al
   list_init(&ctx->samplers);
   list_init(&ctx->events);
   list_init(&ctx->programs);
-  ctx->queue_cookie = 1;
+  ctx->queue_modify_disable = CL_FALSE;
   TRY_ALLOC_NO_ERR (ctx->drv, cl_driver_new(props));
   ctx->props = *props;
   ctx->ver = cl_driver_get_ver(ctx->drv);
diff --git a/src/cl_context.h b/src/cl_context.h
index 9820b6e..4812afd 100644
--- a/src/cl_context.h
+++ b/src/cl_context.h
@@ -105,7 +105,7 @@ struct _cl_context {
   cl_uint device_num;               /* Devices number of this context */
   list_head queues;                 /* All command queues currently allocated */
   cl_uint queue_num;                /* All queue number currently allocated */
-  cl_uint queue_cookie;             /* Cookie will change every time we change queue list. */
+  cl_uint queue_modify_disable;     /* Temp disable queue list change. */
   list_head mem_objects;            /* All memory object currently allocated */
   cl_uint mem_object_num;           /* All memory number currently allocated */
   list_head samplers;               /* All sampler object currently allocated */
diff --git a/src/cl_event.c b/src/cl_event.c
index 4dcc728..3e1dc22 100644
--- a/src/cl_event.c
+++ b/src/cl_event.c
@@ -428,10 +428,7 @@ cl_event_set_status(cl_event event, cl_int status)
 
   /* Need to notify all the command queue within the same context. */
   if (notify_queue) {
-    cl_command_queue *q_list = NULL;
-    cl_uint queue_num = 0;
-    int i = 0;
-    int cookie = 0;
+    cl_command_queue queue = NULL;
 
     /*First, we need to remove it from queue's barrier list. */
     if (CL_EVENT_IS_BARRIER(event)) {
@@ -441,37 +438,22 @@ cl_event_set_status(cl_event event, cl_int status)
 
     /* Then, notify all the queues within the same context. */
     CL_OBJECT_LOCK(event->ctx);
-    do {
-      queue_num = event->ctx->queue_num;
-      cookie = event->ctx->queue_cookie;
-
-      if (queue_num > 0) {
-        q_list = cl_calloc(queue_num, sizeof(cl_command_queue));
-        assert(q_list);
-        i = 0;
-        list_for_each(pos, &event->ctx->queues)
-        {
-          q_list[i] = (cl_command_queue)(list_entry(pos, _cl_base_object, node));
-          assert(i < queue_num);
-          i++;
-        }
-
-        CL_OBJECT_UNLOCK(event->ctx); // Update status without context lock.
-
-        for (i = 0; i < queue_num; i++) {
-          cl_command_queue_notify(q_list[i]);
-        }
-
-        CL_OBJECT_LOCK(event->ctx); // Lock again.
-      } else {
-        /* No queue? Just do nothing. */
-      }
-
-    } while (cookie != event->ctx->queue_cookie); // Some queue may be added when we unlock.
+    /* Disable remove and add queue to the context temporary. We need to
+       make sure all the queues in the context currently are valid. */
+    event->ctx->queue_modify_disable++;
+    CL_OBJECT_UNLOCK(event->ctx);
+    list_for_each(pos, &event->ctx->queues)
+    {
+      queue = (cl_command_queue)(list_entry(pos, _cl_base_object, node));
+      assert(queue != NULL);
+      cl_command_queue_notify(queue);
+    }
+    CL_OBJECT_LOCK(event->ctx);
+    /* Disable remove and add queue to the context temporary. We need to
+       make sure all the queues in the context currently are valid. */
+    event->ctx->queue_modify_disable--;
+    CL_OBJECT_NOTIFY_COND(event->ctx);
     CL_OBJECT_UNLOCK(event->ctx);
-
-    if (q_list)
-      cl_free(q_list);
   }
 
   return CL_SUCCESS;
-- 
2.7.4





More information about the Beignet mailing list