On Thu, Aug 3, 2017 at 10:00 AM, Chris Wilson <ch...@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 <ja...@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 unused > cliprects > > 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 waits > on > > the dma_fence that was in the syncobj and then immediately replaces > it > > 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.ekstr...@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?
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
