| Message ID | 20190725131447.27515-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/gt: Add to timeline requires the timeline mutex | expand |
On 25/07/2019 14:14, Chris Wilson wrote: > Modifying a remote context requires careful serialisation with requests > on that context, and that serialisation requires us to take their > timeline->mutex. Make it so. > > Note that while struct_mutex rules, we can't create more than one > request in parallel, but that age is soon coming to an end. > > v2: Though it doesn't affect the current users, contexts may share > timelines so check if we already hold the right mutex. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_context.c | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c > index 9292b6ca5e9c..d64b45f7ec6d 100644 > --- a/drivers/gpu/drm/i915/gt/intel_context.c > +++ b/drivers/gpu/drm/i915/gt/intel_context.c > @@ -254,10 +254,18 @@ int intel_context_prepare_remote_request(struct intel_context *ce, > /* Only suitable for use in remotely modifying this context */ > GEM_BUG_ON(rq->hw_context == ce); > > - /* Queue this switch after all other activity by this context. */ > - err = i915_active_request_set(&tl->last_request, rq); > - if (err) > - return err; > + if (rq->timeline != tl) { /* beware timeline sharing */ > + err = mutex_lock_interruptible_nested(&tl->mutex, > + SINGLE_DEPTH_NESTING); > + if (err) > + return err; > + > + /* Queue this switch after current activity by this context. */ > + err = i915_active_request_set(&tl->last_request, rq); > + if (err) > + goto unlock; > + } > + lockdep_assert_held(&tl->mutex); > > /* > * Guarantee context image and the timeline remains pinned until the > @@ -267,7 +275,12 @@ int intel_context_prepare_remote_request(struct intel_context *ce, > * words transfer the pinned ce object to tracked active request. > */ > GEM_BUG_ON(i915_active_is_idle(&ce->active)); > - return i915_active_ref(&ce->active, rq->fence.context, rq); > + err = i915_active_ref(&ce->active, rq->fence.context, rq); > + > +unlock: > + if (rq->timeline != tl) > + mutex_unlock(&tl->mutex); > + return err; > } > > struct i915_request *intel_context_create_request(struct intel_context *ce) > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 9292b6ca5e9c..d64b45f7ec6d 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -254,10 +254,18 @@ int intel_context_prepare_remote_request(struct intel_context *ce, /* Only suitable for use in remotely modifying this context */ GEM_BUG_ON(rq->hw_context == ce); - /* Queue this switch after all other activity by this context. */ - err = i915_active_request_set(&tl->last_request, rq); - if (err) - return err; + if (rq->timeline != tl) { /* beware timeline sharing */ + err = mutex_lock_interruptible_nested(&tl->mutex, + SINGLE_DEPTH_NESTING); + if (err) + return err; + + /* Queue this switch after current activity by this context. */ + err = i915_active_request_set(&tl->last_request, rq); + if (err) + goto unlock; + } + lockdep_assert_held(&tl->mutex); /* * Guarantee context image and the timeline remains pinned until the @@ -267,7 +275,12 @@ int intel_context_prepare_remote_request(struct intel_context *ce, * words transfer the pinned ce object to tracked active request. */ GEM_BUG_ON(i915_active_is_idle(&ce->active)); - return i915_active_ref(&ce->active, rq->fence.context, rq); + err = i915_active_ref(&ce->active, rq->fence.context, rq); + +unlock: + if (rq->timeline != tl) + mutex_unlock(&tl->mutex); + return err; } struct i915_request *intel_context_create_request(struct intel_context *ce)
Modifying a remote context requires careful serialisation with requests on that context, and that serialisation requires us to take their timeline->mutex. Make it so. Note that while struct_mutex rules, we can't create more than one request in parallel, but that age is soon coming to an end. v2: Though it doesn't affect the current users, contexts may share timelines so check if we already hold the right mutex. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/gt/intel_context.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)