[Beignet] [PATCH v2] GetEventProfilingInfo could query all event status.

[Zhigang Gong]

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Yang, Rong R
> Sent: Wednesday, August 5, 2015 1:05 PM
> To: Zhigang Gong; Luo, Xionghu
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH v2] GetEventProfilingInfo could query all event
> status.
> 
> 
> 
> > -----Original Message-----
> > From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
> > Sent: Wednesday, August 5, 2015 12:49
> > To: Yang, Rong R; Luo, Xionghu
> > Cc: beignet at lists.freedesktop.org
> > Subject: RE: [Beignet] [PATCH v2] GetEventProfilingInfo could query
> > all event status.
> >
> > For this special case, the user application has an serious bug which
> > is forgetting to call clReleaseEvent() and will leak one event.
> 
> Suppose the application call clReleaseEvent and clReleaseCommandQueue in
> event's callback, and it is the last enqueue event, so beignet has no chance to
> call the callback.

Right. Put the clReleaseEvent() in a callback function is a normal case.
I'm ok you push both of the patches. But please be aware, these fixes
are only work around for some cases. If the user doesn't call clFinish() or
will call clFinish after the callback function be called, application will
be stalled.

Thanks,
Zhigang Gong.

> 
> >
> > For beignet, the real problem is if an application rely on event
> > driven mechanism, then it may never get a chance to move forward, here
> > is a simple example:
> >
> > main()
> > {
> >   ...
> >   clEnqueueNDRange(queue,...,event);
> >   clSetEventCallback(event, .., callbackFn);
> >   ...
> >   pthread_create(pFn);//creat a child thread.
> >   pthread_wait() // wait for the last thread to terminate.
> >   clReleaseEvent(event);
> >   clFinish(queue);
> > }
> >
> > callbackFn() {
> >   pthread_cond_signal(); // notify the child thread to do some thing.
> >   ...
> > }
> >
> > pFn() {
> >   pthread_cond_wait()
> >   ...//do something on CPU;
> >   clEnqueueNDRange(queue, ..., event2);
> >   ...
> > }
> >
> > The callbackFn will never be called and the child thread will always
> > wait for the condition variable even the actual event has been
> > finished soon after the first enqueuer().
> >
> > So add event status update in clFinish() will not really fix this type of problem.
> > The real fix should be to rework the whole event handling mechanism
> > and add one daemon thread to maintain event status.
> > I really can't think of one method without an additional thread to
> > solve this problem.
> [Yang, Rong R] Argee with it.
> >
> >
> > Thanks,
> > Zhigang Gong.
> >
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet