On 12/20/2016 11:59 AM, Jan Vesely wrote: > On Fri, 2016-12-16 at 13:43 -0800, Francisco Jerez wrote: >> Vedran Miletić <ved...@miletic.net> writes: >> >>> Current implementation of event handling keeps an extra reference to >>> the hardware event, in addition to the reference returned via the OpenCL >>> API. This additional reference is internal and should not be counted >>> when queried via the clGetEventInfo() function. >>> >>> Fixes Piglit's cl/api/retain_release-event test. >>> >>> Signed-off-by: Vedran Miletić <ved...@miletic.net> >>> --- >>> src/gallium/state_trackers/clover/api/event.cpp | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/gallium/state_trackers/clover/api/event.cpp >>> b/src/gallium/state_trackers/clover/api/event.cpp >>> index 5d1a0e5..74bc4d9 100644 >>> --- a/src/gallium/state_trackers/clover/api/event.cpp >>> +++ b/src/gallium/state_trackers/clover/api/event.cpp >>> @@ -107,7 +107,9 @@ clGetEventInfo(cl_event d_ev, cl_event_info param, >>> break; >>> >>> case CL_EVENT_REFERENCE_COUNT: >>> - buf.as_scalar<cl_uint>() = ev.ref_count(); >>> + // Current implementation of event handling keeps an extra reference >>> to >>> + // the hardware event, which is internal and should not be counted. >>> + buf.as_scalar<cl_uint>() = ev.ref_count() - 1; >> >> I don't think this is correct. There is an internal event reference >> held by the command queue object, but only for as long as the event >> remains in the queue until the next flush. In other cases the above >> would give you a reference count which is off by one. That said: >> >>> The reference count returned should be considered immediately >>> stale. It is unsuitable for general use in applications. This feature >>> is provided for identifying memory leaks. > > > I found only a generic description that mentions reference count == 1 > wrt. events (in Glossary). Even there it says that it's an internal > counter. > The only object that seems to require reference count == 1 is root > device. Contexts, queues, mem, samplers, programs, kernels, events, all > include the above footnote. > I think the piglit test should be changed to check for non-zero value > instead of 1. > > Jan >
Thank you both for your inputs. Let's try changing the Piglit test. Vedran -- Vedran Miletić vedran.miletic.net _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev