[Mesa-dev] [PATCH] i965: store reference to the context within struct brw_fence

[Chad Versace]

On Mon 28 Sep 2015, Marek Olšák wrote:
> On Mon, Sep 28, 2015 at 1:29 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > As the spec allows for {server,client}_wait_sync to be called without
> > currently bound context, while our implementation requires context
> > pointer.
> >
> > UNTESTED.
> >
> > Cc: Chad Versace <chad.versace at intel.com>
> > Cc: Marek Olšák <marek.olsak at amd.com>
> > Cc: Chih-Wei Huang <cwhuang at android-x86.org>
> > Cc: Mauro Rossi <issor.oruam at gmail.com>
> > Cc: 10.6 11.0 <mesa-stable at lists.freedesktop.org>
> > Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> > ---
> >
> > Upon second thought I'm leaning that we should move this to the API
> > specific library (i.e. libEGL) rather than keeping it here. It will give
> > us remove API specific from the DRI extension, plus it'll give us better
> > mix'n'match (loader+dri module) compatibility.
> >
> > How do you guys feel on the topic ?
> 
> I don't think there is anything to move into libEGL. st/dri doesn't
> add a context pointer to the fence, instead it adds a screen pointer,
> which is a different thing.

I agree with Mark here.  Even if there was something you could move into
libEGL, I don't it could be done safely. It needs to remain in the
driver.

> This i965 fix is probably not thread-safe, because eglClientWaitSync
> can be called from another thread and contexts typically contain no
> locks.

Emil, the fix is not thread-safe because drm_intel_gem_bo_wait() mutates
the bo. Thread-safety matters because some Chromium Ozone code that's
cooking does call eglCreateSync and eglClientWaitSync from different
threads.

If you add a mutex to struct brw_fence, and lock it for the duration of
brw_fence_client_wait() and brw_fence_is_completed(), then I think
that's sufficient for ensuring thread-safety.