[Intel-gfx] [PATCH 02/28] drm/i915: use new iterator in i915_gem_object_wait_reservation

[Tvrtko Ursulin]

On 28/10/2021 16:30, Daniel Vetter wrote:
> On Thu, Oct 28, 2021 at 10:41:38AM +0200, Christian König wrote:
>> Am 21.10.21 um 13:13 schrieb Tvrtko Ursulin:
>>>
>>> On 21/10/2021 12:06, Maarten Lankhorst wrote:
>>>> Op 21-10-2021 om 12:38 schreef Christian König:
>>>>> Am 21.10.21 um 12:35 schrieb Maarten Lankhorst:
>>>>>> From: Christian König <christian.koenig at amd.com>
>>>>>>
>>>>>> Simplifying the code a bit.
>>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>>>> [mlankhorst: Handle timeout = 0 correctly, use new
>>>>>> i915_request_wait_timeout.]
>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>>>
>>>>> LGTM, do you want to push it or should I pick it up into drm-misc-next?
>>>>
>>>> I think it can be applied to drm-intel-gt-next, after a backmerge.
>>>> It needs patch 1 too, which fixes
>>>>
>>>> i915_request_wait semantics when used in dma-fence. It exports a
>>>> dma-fence compatible i915_request_wait_timeout function, used in
>>>> this patch.
>>
>> What about the other i915 patches? I guess you then want to merge them
>> through drm-intel-gt-next as well.
>>
>>> I don't think my open has been resolved, at least I haven't seen a reply
>>> from Daniel on the topic of potential for infinite waits with untrusted
>>> clients after this change. +Daniel
>>
>> Please resolve that internally and let me know the result. I'm fine to use
>> any of the possible approaches, I just need to know which one.
> 
> I thought I explained this in the patch set from Maarten. This isn't an
> issue, since the exact same thing can happen if you get interrupts and
> stuff.

Ah were you trying to point out all this time the infinite wait just got 
moved from inside the "old" dma_resv_get_fences to the new iterator caller?

Regards,

Tvrtko

> 
> The only proper fix for bounding the waits is a) compositor grabs a stable
> set of dma_fence from the dma-buf through the proposed fence export ioctl
> b) compositor waits on that fence (or drm_syncobj).
> 
> Everything else is cargo-culted nonsense, and very much includes that igt
> patch that's floating around internally.
> 
> I can also whack this into drm-next if this is stuck in this silly
> bikeshed.
> -Daniel
>