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);
>> Â }
>> Â Â /**
>
