On 2017å¹´10æ??31æ?¥ 16:55, Christian König wrote:
> Am 31.10.2017 um 09:51 schrieb Chunming Zhou:
>>
>>
>> On 2017å¹´10æ??31æ?¥ 16:43, Christian König wrote:
>>> 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.
>>>
>>> v2: temporary put the signaled fences at the end of the new container
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>> Â drivers/dma-buf/reservation.c | 36
>>> ++++++++++++++++++++++++++----------
>>> Â 1 file changed, 26 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/reservation.c
>>> b/drivers/dma-buf/reservation.c
>>> index b44d9d7db347..6fc794576997 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 = fobj->shared_max; i < old->shared_count;
>>> ++i) {
>>> Â Â Â Â Â Â Â Â Â struct dma_fence *check;
>>> Â Â Â Â Â Â Â Â Â Â check = rcu_dereference_protected(old->shared[i],
>>> @@ -172,10 +170,18 @@ 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 {
>>> +Â Â Â Â Â Â Â Â Â Â Â /*
>>> +Â Â Â Â Â Â Â Â Â Â Â Â * Temporary save the signaled fences at the end of the
>>> +Â Â Â Â Â Â Â Â Â Â Â Â * new container
>>> +Â Â Â Â Â Â Â Â Â Â Â Â */
>>> +Â Â Â Â Â Â Â Â Â Â Â RCU_INIT_POINTER(fobj->shared[--k], check);
>>> +Â Â Â Â Â Â Â }
>>> Â Â Â Â Â }
>>> +Â Â Â fobj->shared_count = j;
>>> Â Â Â Â Â if (!old_fence) {
>>> RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
>> Seems you still don't address here, how do you sure
>> fobj->shared[fobj->shared_count] is null? if not NULL, the old one
>> will be leak.
>
> I've put them at the end of the container, see above "k =
> fobj->shared_max". Since shared_max >> shared_count (at least twice as
> large in this situation) we should be on the save side.
Ah, oops, Reviewed-by: Chunming Zhou <david1.zhou at amd.com> for the series.
Thanks,
David Zhou
>
> Regards,
> Christian.
>
>>
>> Regards,
>> David Zhou
>>> Â Â Â Â Â Â Â Â Â fobj->shared_count++;
>>> @@ -192,10 +198,20 @@ 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;
>>> +
>>> +Â Â Â /* Drop the references to the signaled fences */
>>> +Â Â Â for (i = k; i < fobj->shared_max; ++i) {
>>> +Â Â Â Â Â Â Â struct dma_fence *f;
>>> +
>>> +Â Â Â Â Â Â Â f = rcu_dereference_protected(fobj->shared[i],
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â reservation_object_held(obj));
>>> +Â Â Â Â Â Â Â dma_fence_put(f);
>>> +Â Â Â }
>>> +Â Â Â kfree_rcu(old, rcu);
>>> Â }
>>> Â Â /**
>>
>
