[Mesa-dev] [PATCH 1/2] clover: fix event handling of buffer operations
greg at chown.ath.cx
Thu Jun 25 10:00:29 PDT 2015
On 2015-06-09 22:52, Francisco Jerez wrote:
>> + if (blocking)
>> + hev().wait();
> hard_event::wait() may fail, so this should probably be done before the
> ret_object() call to avoid leaks.
Alright... C++ exceptions are a minefield. :)
> Is there any reason you didn't make
> the same change in clEnqueueReadBuffer() and clEnqueueWriteBuffer()?
Must be an oversight. I think I did that, or at least I intended to do
> Same comment as above. Also note that this is being more strict than
> the spec requires (which I believe is what Tom was referring to). From
> the CL 1.2 spec:
> | If blocking_write is CL_TRUE, the OpenCL implementation copies the
> | referred to by ptr and enqueues the write operation in the
> | command-queue. The memory pointed to by ptr can be reused by the
> | application after the clEnqueueWriteBufferRect call returns.
> The spec is giving you no guarantee that the write to the actual memory
> object will be complete by the time the clEnqueueWriteBufferRect call
> returns -- Only that your data will have been buffered somewhere and
> memory pointed to by the argument can be reused immediately by the
> application. The reason why I was reluctant to make this change last
> time it came up was that it's likely to hurt performance unnecessarily
> because the wait() call blocks until *all* previous commands in the
> queue have completed execution, even though in the most common case the
> copy is performed synchronously using soft_copy_op(), so the wait()
> is redundant even for blocking copies.
OK, maybe we could drop the wait completely for all of the "write"
> The case with blocking reads is similar, the copy is handled
> synchronously using soft_copy_op() when no user events are present in
> the list of dependencies, so calling wait() on the event is unnecessary
> to guarantee that the execution of the read has completed, and will
> cause a pipe_context flush and wait until the most recent fence is
I think it's reasonable to expect that the event is ready for profile
queries after a blocking read has finished. That was the initial
motivation for this patch. Other implementations behave like that. I
didn't expect wait() to completely flush everything. Won't that cause a
lot of needless flushing with event wait lists?
> Ideally we would have a weaker variant of event::wait()
> (e.g. wait_signalled()) that doesn't flush and just waits for the
> associated action call-back to have been executed without giving any
> guarantees about the corresponding GPU command. The event interface
> doesn't expose such a functionality right now, I'm attaching two
> (completely untested) patches implementing it, you should be able to
> them as starting point to fix blocking transfers.
Thanks, I'll look into that later when I get some free time.
More information about the mesa-dev