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

Christian König
2021-11-11T11:37:00Z

Am 01.11.21 um 10:41 schrieb 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@amd.com>
>>>>>>>
>>>>>>> Simplifying the code a bit.
>>>>>>>
>>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>>> [mlankhorst: Handle timeout = 0 correctly, use new
>>>>>>> i915_request_wait_timeout.]
>>>>>>> Signed-off-by: Maarten Lankhorst
>>>>>>> <maarten.lankhorst@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?

At least I think so, yes. But Daniel needs to answer that.

>
> 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.

Can we move forward with those patches? I still don't see them in
drm-misc-next.

I you want I can also pick Maartens modified version here up and send it
out with the reset of the i915 patches once more.

Everything else is already pushed.

Thanks,
Christian.

>> -Daniel
>>