[PATCH 1/2] dma-buf: keep only not signaled fence in reservation_object_add_shared_replace
Christian König
ckoenig.leichtzumerken at gmail.com
Tue Oct 31 08:42:25 UTC 2017
Looks like v2 never made it to the list. I've just send the V2 patches
once more.
Christian.
Am 31.10.2017 um 08:26 schrieb Chunming Zhou:
> Any update?
>
>
> On 2017年10月25日 15:28, Christian König wrote:
>> Am 25.10.2017 um 08:42 schrieb Chunming Zhou:
>>>
>>>
>>> On 2017年10月24日 21:55, Christian König wrote:
>>>> From: Christian König <christian.koenig at amd.com>
>>>>
>>>> The amdgpu issue to also need signaled fences in the reservation
>>>> objects
>>>> should be fixed by now.
>>>>
>>>> Optimize the list by keeping only the not signaled yet fences around.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>> ---
>>>> drivers/dma-buf/reservation.c | 31 +++++++++++++++++++++----------
>>>> 1 file changed, 21 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/dma-buf/reservation.c
>>>> b/drivers/dma-buf/reservation.c
>>>> index b44d9d7db347..4ede77d1bb31 100644
>>>> --- a/drivers/dma-buf/reservation.c
>>>> +++ b/drivers/dma-buf/reservation.c
>>>> @@ -145,8 +145,8 @@ reservation_object_add_shared_replace(struct
>>>> reservation_object *obj,
>>>> struct reservation_object_list *fobj,
>>>> struct dma_fence *fence)
>>>> {
>>>> - unsigned i;
>>>> struct dma_fence *old_fence = NULL;
>>>> + unsigned i, j, k;
>>>> dma_fence_get(fence);
>>>> @@ -162,9 +162,7 @@ reservation_object_add_shared_replace(struct
>>>> reservation_object *obj,
>>>> * references from the old struct are carried over to
>>>> * the new.
>>>> */
>>>> - fobj->shared_count = old->shared_count;
>>>> -
>>>> - for (i = 0; i < old->shared_count; ++i) {
>>>> + for (i = 0, j = 0, k = old->shared_count; i <
>>>> old->shared_count; ++i) {
>>>> struct dma_fence *check;
>>>> check = rcu_dereference_protected(old->shared[i],
>>>> @@ -172,10 +170,14 @@ reservation_object_add_shared_replace(struct
>>>> reservation_object *obj,
>>>> if (!old_fence && check->context == fence->context) {
>>>> old_fence = check;
>>>> - RCU_INIT_POINTER(fobj->shared[i], fence);
>>>> - } else
>>>> - RCU_INIT_POINTER(fobj->shared[i], check);
>>>> + RCU_INIT_POINTER(fobj->shared[j++], fence);
>>>> + } else if (!dma_fence_is_signaled(check)) {
>>>> + RCU_INIT_POINTER(fobj->shared[j++], check);
>>>> + } else {
>>>> + RCU_INIT_POINTER(fobj->shared[--k], check);
>>>> + }
>>>> }
>>>> + fobj->shared_count = j;
>>>> if (!old_fence) {
>>>> RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
>>> Here there is a memory leak for signaled fence slots, since you
>>> re-order the slots, the j'th slot is storing signaled fence, there
>>> is no place to put it when you assign to new one.
>>> you cam move it to end or put it here first.
>>
>> Good point, thanks. Going to respin.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>> David Zhou
>>>> fobj->shared_count++;
>>>> @@ -192,10 +194,19 @@ reservation_object_add_shared_replace(struct
>>>> reservation_object *obj,
>>>> write_seqcount_end(&obj->seq);
>>>> preempt_enable();
>>>> - if (old)
>>>> - kfree_rcu(old, rcu);
>>>> -
>>>> dma_fence_put(old_fence);
>>>> +
>>>> + if (!old)
>>>> + return;
>>>> +
>>>> + for (i = fobj->shared_count; i < old->shared_count; ++i) {
>>>> + struct dma_fence *f;
>>>> +
>>>> + f = rcu_dereference_protected(fobj->shared[i],
>>>> + reservation_object_held(obj));
>>>> + dma_fence_put(f);
>>>> + }
>>>> + kfree_rcu(old, rcu);
>>>> }
>>>> /**
>>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the dri-devel
mailing list