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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
