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:[email protected]] 
Sent: Tuesday, September 22, 2015 10:30 AM
To: Pan, Xiuli <[email protected]>
Cc: [email protected]
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 <[email protected]>
> ---
>  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
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/beignet
_______________________________________________
Beignet mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/beignet

Reply via email to