[Beignet] [PATCH 2/2] Fix DRM Memory leak BUG

[Zhigang Gong]

One suggestion, you can use the conformance test suite's event
test cases to verify your modification.

On Tue, Sep 22, 2015 at 05:10:45AM +0000, Pan, Xiuli wrote:
> I have looked into the clWaitForEvents function and read about ocl spec, maybe the uncompleted evnet in the last_event should not be there at all. It should be finished in the waitforevent function and be deleted and  freed in the clReleaseEvent. My patch may cause unexpectable user function behavior for the event's actually finished time is random. I will look into the WaitForEvnets function and make some patches there. Thank you!
> 
> -----Original Message-----
> From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
> Sent: Tuesday, September 22, 2015 10:30 AM
> To: Pan, Xiuli <xiuli.pan at intel.com>
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH 2/2] Fix DRM Memory leak BUG
> 
> Nice catch! But may not be a correct fix.
> We don't need to do the blocking event updating all the time.
> We only need to do that when there is potential possibility to leak a event. If a event has a user call back function registered is such a case, and my best guessing here is:
> one event in the wait list of the last event has user call back function registered and has been missed.
> 
> We may need to check all the wait list of the last event before we do a locking event updating here.
> 
> Thanks,
> Zhigang Gong.
> 
> On Mon, Sep 21, 2015 at 04:41:52PM +0800, Pan Xiuli wrote:
> > This bug is cased by event flush, we should not only run usr event but 
> > also event made by enqueue functions.
> > If the event haven't been completed before it is been overwite in the 
> > last_event, the related gpgpu buffer will not be unreference. And will 
> > cause all related drm buffers unreference and thenw leak.
> > 
> > Signed-off-by: Pan Xiuli <xiuli.pan at intel.com>
> > ---
> >  src/cl_command_queue.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/cl_command_queue.c b/src/cl_command_queue.c index 
> > 4b92311..fd1d613 100644
> > --- a/src/cl_command_queue.c
> > +++ b/src/cl_command_queue.c
> > @@ -261,7 +261,7 @@ cl_command_queue_flush(cl_command_queue queue)
> >    // 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)
> > +  if (last_event)
> >      cl_event_update_status(last_event, 1);
> >    cl_event current_event = get_current_event(queue);
> >    if (current_event && err == CL_SUCCESS) {
> > --
> > 2.1.4
> > 
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet