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

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

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.

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

>
>> +
>> +/**
>> + * 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.

Thanks,
Christian.

> Regards,
>
> Tvrtko