Re: [Intel-gfx] [PATCH 18/26] drm/i915: use new iterator in i915_gem_object_last_write_engine

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 14/09/2021 13:32, Christian König wrote:
> Am 14.09.21 um 14:27 schrieb Tvrtko Ursulin:
>>
>> On 13/09/2021 14:16, Christian König wrote:
>>> This is maybe even a fix since the RCU usage here looks incorrect.
>>
>> What you think is incorrect? Pointless extra rcu locking?
> 
> Yeah, exactly that. I also wondered for a second if rcu_read_lock() can 
> nest or not. But obviously it either works or lockdep hasn't complained 
> yet.
> 
> But I've made a mistake here and at a couple of other places to remove 
> to many rcu_read_lock() calls. Thanks for pointing that out, going to 
> fix it as well.

Ack.

>> Also, FWIW, I submitted a patch to remove this function altogether 
>> since its IMO pretty useless, just failed in getting anyone to ack it 
>> so far.
> 
> I was on the edge of suggesting that as well since it's only debugfs 
> usage looked quite pointless to me.
> 
> Feel free to CC me on the patch and you can have my acked-by.

Patch is here 
https://patchwork.freedesktop.org/patch/451864/?series=94202&rev=1, thanks!

Regards,

Tvrtko

> Thanks,
> Christian.
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/i915/gem/i915_gem_object.h | 15 +++++++--------
>>>   1 file changed, 7 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h 
>>> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
>>> index e9eecebf5c9d..3343922af4d6 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
>>> @@ -500,16 +500,15 @@ static inline struct intel_engine_cs *
>>>   i915_gem_object_last_write_engine(struct drm_i915_gem_object *obj)
>>>   {
>>>       struct intel_engine_cs *engine = NULL;
>>> +    struct dma_resv_cursor cursor;
>>>       struct dma_fence *fence;
>>>   -    rcu_read_lock();
>>> -    fence = dma_resv_get_excl_unlocked(obj->base.resv);
>>> -    rcu_read_unlock();
>>> -
>>> -    if (fence && dma_fence_is_i915(fence) && 
>>> !dma_fence_is_signaled(fence))
>>> -        engine = to_request(fence)->engine;
>>> -    dma_fence_put(fence);
>>> -
>>> +    dma_resv_for_each_fence_unlocked(obj->base.resv, &cursor, false,
>>> +                     fence) {
>>> +        if (fence && dma_fence_is_i915(fence) &&
>>> +            !dma_fence_is_signaled(fence))
>>> +            engine = to_request(fence)->engine;
>>> +    }
>>>       return engine;
>>>   }
>>>
>