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