On 08/07/2019 14:32, Chris Wilson wrote:
Quoting Chris Wilson (2019-07-08 13:22:01)
Quoting Tvrtko Ursulin (2019-07-08 13:18:02)
On 08/07/2019 12:39, Chris Wilson wrote:
v2: Though it doesn't affect the current users, contexts may share
timelines so check if we already hold the right mutex.
+int intel_context_prepare_remote_request(struct intel_context *ce,
+ struct i915_request *rq)
+{
+ struct intel_timeline *tl = ce->ring->timeline;
+ int err;
+
+ /* Only suitable for use in remotely modifying this context */
+ GEM_BUG_ON(rq->hw_context == ce);
+
+ if (rq->timeline != tl) { /* beware timeline sharing */
+ err = mutex_lock_interruptible_nested(&tl->mutex,
+ SINGLE_DEPTH_NESTING);
Which caller is holding tl->mutex?
None today, but it is conceivable. So rather than have a mysterious
deadlock not reported by lockdep in the future, nip the problem in the
bud.
Just in case, creating a request takes the timeline->mutex and holds it
until submitted. So the scenario I considered was if one user context
wanted to modify another one (which could work), but they happen to be
on a common timeline.
Maybe my forward vision is too limited. :) But since code consolidation
is worth it, how about you keep that aspect in this patch, and leave the
tl->mutex taking for later?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
