[Intel-gfx] [PATCH] drm/i915: Mark up racy read of rq->engine

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-04-23 15:53:44)
> 
> On 23/04/2020 12:58, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > index 22635bbabf06..e9fd20242438 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -1660,7 +1660,7 @@ long i915_request_wait(struct i915_request *rq,
> >                       break;
> >               }
> >   
> > -             intel_engine_flush_submission(rq->engine);
> > +             intel_engine_flush_submission(READ_ONCE(rq->engine));
> >   
> >               if (signal_pending_state(state, current)) {
> >                       timeout = -ERESTARTSYS;
> > 
> 
> What with the mutex_acquire/release in this case? No practical effect 
> but they are also dereferencing rq->engine... Take a copy of engine for 
> lockdep at start and another read for engine flushing in the loop?

No practical difference [today], since the lockmap is on the gt.

Well that will be interesting in the future. Hmm, we could replace it
with a static [global] lockmap and then link all gt->reset.mutex to.
The only danger then is that we link a wait on one gt with a wait on
another -- except that if we allow it, that will happen naturally.
So I don't see a loss in generality in using a global lockmap.

Make a note for the future.
-Chris