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