On Thu, Aug 3, 2017 at 10:00 AM, Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Jason Ekstrand (2017-07-05 22:15:09)
On Wed, Jul 5, 2017 at 2:13 PM, Jason Ekstrand jason@jlekstrand.net
wrote:
This commit adds support for waiting on or signaling DRM syncobjs as part of execbuf. It does so by hijacking the currently unusedcliprects
pointer to instead point to an array of i915_gem_exec_fence structs which containe a DRM syncobj and a flags parameter which specifies whether to wait on it or to signal it. This implementation theoretically allows for both flags to be set in which case it waitson
the dma_fence that was in the syncobj and then immediately replacesit
with the dma_fence from the current execbuf. v2: - Rebase on new syncobj API v3: - Pull everything out into helpers - Do all allocation in gem_execbuffer2 - Pack the flags in the bottom 2 bits of the drm_syncobj*Just noticed, no sign off. Could you please check https://developercertificate.org/ and apply your Signed-off-by, just a reply will do.
Sorry, not familiar with the kernel...
Signed-off-by: Jason Ekstrand jason.ekstrand@intel.com
+static int await_fence_array(struct i915_execbuffer *eb, + struct drm_syncobj **fences) +{ + const unsigned int nfences = eb->args->num_cliprects; + unsigned int n; + int err; + + for (n = 0; n < nfences; n++) { + struct drm_syncobj *syncobj; + unsigned int flags; + + syncobj = ptr_unpack_bits(fences[n], &flags, 2); + if (!(flags & I915_EXEC_FENCE_WAIT)) + continue; + + err = i915_gem_request_await_dma_fence(eb->request, +syncobj->fence);
Is there a race here? What happens if some other process replaces the
fence
between the syncobj->fence lookup and gem_request_await_dma_fence taking
its
reference?
Yes. It's inherently racy be via from objects, dmabuf or explicit fences. We obtain a snapshot of fences, and the only condition we must impose is that they are not cyclic - i.e. we must complete the snapshot of all fences before exposing our new &request->fence to the world.
As to the race where the fence pointed to may change whilst we inspect it, there is nothing we can do to prevent that nor define what the right behaviour is. It is up to userspace to apply its own execution barriers if it requires strict control over the ordering of fences, i.e. what does if mean if client B changed the fence whilst client A was executing? Should client A use the original fence or the new fence? If it matters, the two clients need to coordinate usage of their shared fence.
I'm not concerned about what happens to racy clients. They get what they get. What concerns me is what happens if somehow the fence is replaced and deleted before i915_gem_request_await_dma_fence takes it's reference. Can this cause the kernel to segfault?