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