[Intel-gfx] [PATCH 01/26] dma-buf: add dma_resv_for_each_fence_unlocked v4

[Tvrtko Ursulin]

On 22/09/2021 15:50, Christian König wrote:
> Am 22.09.21 um 16:36 schrieb Tvrtko Ursulin:
>>> +
>>> +/**
>>> + * dma_resv_iter_first_unlocked - first fence in an unlocked 
>>> dma_resv obj.
>>> + * @cursor: the cursor with the current position
>>> + *
>>> + * Returns the first fence from an unlocked dma_resv obj.
>>> + */
>>> +struct dma_fence *dma_resv_iter_first_unlocked(struct dma_resv_iter 
>>> *cursor)
>>> +{
>>> +    rcu_read_lock();
>>> +    do {
>>> +        dma_resv_iter_restart_unlocked(cursor);
>>> +        dma_resv_iter_walk_unlocked(cursor);
>>> +    } while (read_seqcount_retry(&cursor->obj->seq, cursor->seq));
>>> +    rcu_read_unlock();
>>> +
>>> +    return cursor->fence;
>>> +}
>>> +EXPORT_SYMBOL(dma_resv_iter_first_unlocked);
>>
>> Why is this one split from dma_resv_iter_begin and even exported?
> 
> I've split it to be able to use dma_resv_iter_begin in both the unlocked 
> and locked iterator.

Ok.

> 
>> I couldn't find any users in the series.
> 
> This is used in the dma_resv_for_each_fence_unlocked() macro to return 
> the first fence.

Doh!

>>> +
>>> +/**
>>> + * dma_resv_iter_next_unlocked - next fence in an unlocked dma_resv 
>>> obj.
>>> + * @cursor: the cursor with the current position
>>> + *
>>> + * Returns the next fence from an unlocked dma_resv obj.
>>> + */
>>> +struct dma_fence *dma_resv_iter_next_unlocked(struct dma_resv_iter 
>>> *cursor)
>>> +{
>>> +    bool restart;
>>> +
>>> +    rcu_read_lock();
>>> +    cursor->is_restarted = false;
>>> +    restart = read_seqcount_retry(&cursor->obj->seq, cursor->seq);
>>> +    do {
>>> +        if (restart)
>>> +            dma_resv_iter_restart_unlocked(cursor);
>>> +        dma_resv_iter_walk_unlocked(cursor);
>>> +        restart = true;
>>> +    } while (read_seqcount_retry(&cursor->obj->seq, cursor->seq));
>>> +    rcu_read_unlock();
>>> +
>>> +    return cursor->fence;
>>> +}
>>> +EXPORT_SYMBOL(dma_resv_iter_next_unlocked);
>>
>> Couldn't dma_resv_iter_first_unlocked and dma_resv_iter_next_unlocked 
>> share the same implementation? Especially if you are able to replace 
>> cursor->is_restarted with cursor->index == -1.
> 
> That's what I had initially, but Daniel disliked it for some reason. You 
> then need a centralized walk function instead of first/next.

I had some ideas to only consolidate "first" and "next" helpers but never mind, yours is fine as well.

Regards,

Tvrtko

> 
> Thanks,
> Christian.
> 
>> Regards,
>>
>> Tvrtko
>