[Intel-gfx] [PATCH v3] drm/i915: Add to timeline requires the timeline mutex

[Tvrtko Ursulin]

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