Op 14-10-2021 om 10:37 schreef Tvrtko Ursulin:
>
> On 13/10/2021 11:41, Maarten Lankhorst wrote:
>> No memory should be allocated when calling i915_gem_object_wait,
>> because it may be called to idle a BO when evicting memory.
>>
>> Fix this by using dma_resv_iter helpers to call
>> i915_gem_object_wait_fence() on each fence, which cleans up the code a lot.
>> Also remove dma_resv_prune, it's questionably.
>>
>> This will result in the following lockdep splat.
>
> <snip>
>
>> @@ -37,56 +36,17 @@ i915_gem_object_wait_reservation(struct dma_resv *resv,
>> unsigned int flags,
>> long timeout)
>> {
>> - struct dma_fence *excl;
>> - bool prune_fences = false;
>> -
>> - if (flags & I915_WAIT_ALL) {
>> - struct dma_fence **shared;
>> - unsigned int count, i;
>> - int ret;
>> + struct dma_resv_iter cursor;
>> + struct dma_fence *fence;
>> - ret = dma_resv_get_fences(resv, &excl, &count, &shared);
>> - if (ret)
>> - return ret;
>> -
>> - for (i = 0; i < count; i++) {
>> - timeout = i915_gem_object_wait_fence(shared[i],
>> - flags, timeout);
>> - if (timeout < 0)
>> - break;
>> + dma_resv_iter_begin(&cursor, resv, flags & I915_WAIT_ALL);
>> + dma_resv_for_each_fence_unlocked(&cursor, fence) {
>> - dma_fence_put(shared[i]);
>> - }
>> -
>> - for (; i < count; i++)
>> - dma_fence_put(shared[i]);
>> - kfree(shared);
>> -
>> - /*
>> - * If both shared fences and an exclusive fence exist,
>> - * then by construction the shared fences must be later
>> - * than the exclusive fence. If we successfully wait for
>> - * all the shared fences, we know that the exclusive fence
>> - * must all be signaled. If all the shared fences are
>> - * signaled, we can prune the array and recover the
>> - * floating references on the fences/requests.
>> - */
>> - prune_fences = count && timeout >= 0;
>> - } else {
>> - excl = dma_resv_get_excl_unlocked(resv);
>> + timeout = i915_gem_object_wait_fence(fence, flags, timeout);
>> + if (timeout <= 0)
>> + break;
>
> You have another change in behaviour here, well a bug really. When userspace passes in zero timeout you fail to report activity in other than the first fence.
Hmm, not necessarily, passing 0 to i915_gem_object_wait_fence timeout = 0 is a special case and means test only. It will return 1 on success.
Of course it is still broken, I sent a reply to könig about it, hope it will get fixed and respun. :)
~Maarten
