Am 19.10.2016 um 19:48 schrieb Gustavo Padovan:
> From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>
> When creating fence arrays we were not holding references to the fences
> in the array, however when destroy the array we were putting away a
> reference to these fences.
>
> This patch hold the ref for all fences in the array when creating the
> array.
>
> It then removes the code that was holding the fences on both amdgpu_vm and
> sync_file. For sync_file, specially, we worked on small referencing
> refactor for sync_file_merge().
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
I would prefer it to keep it like it is, cause this is a bit inconsistent.
With this change the fence array takes the ownership of the array, but
not of the fences inside it.
> ---
> drivers/dma-buf/fence-array.c | 8 ++++----
> drivers/dma-buf/sync_file.c | 14 +++-----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ---
> 3 files changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/dma-buf/fence-array.c b/drivers/dma-buf/fence-array.c
> index f1989fc..598737f 100644
> --- a/drivers/dma-buf/fence-array.c
> +++ b/drivers/dma-buf/fence-array.c
> @@ -112,10 +112,6 @@ EXPORT_SYMBOL(fence_array_ops);
> * Allocate a fence_array object and initialize the base fence with
> fence_init().
> * In case of error it returns NULL.
> *
> - * The caller should allocate the fences array with num_fences size
> - * and fill it with the fences it wants to add to the object. Ownership of
> this
> - * array is taken and fence_put() is used on each fence on release.
> - *
At bare minimum you should keep this comment, cause ownership of the
array is still taken and so it is released in the destructor.
Christian.
> * If @signal_on_any is true the fence array signals if any fence in the
> array
> * signals, otherwise it signals when all fences in the array signal.
> */
> @@ -125,6 +121,7 @@ struct fence_array *fence_array_create(int num_fences,
> struct fence **fences,
> {
> struct fence_array *array;
> size_t size = sizeof(*array);
> + int i;
>
> /* Allocate the callback structures behind the array. */
> size += num_fences * sizeof(struct fence_array_cb);
> @@ -140,6 +137,9 @@ struct fence_array *fence_array_create(int num_fences,
> struct fence **fences,
> atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences);
> array->fences = fences;
>
> + for (i = 0; i < array->num_fences; ++i)
> + fence_get(array->fences[i]);
> +
> return array;
> }
> EXPORT_SYMBOL(fence_array_create);
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 235f8ac..678baaf 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -142,14 +142,8 @@ static int sync_file_set_fence(struct sync_file
> *sync_file,
> {
> struct fence_array *array;
>
> - /*
> - * The reference for the fences in the new sync_file and held
> - * in add_fence() during the merge procedure, so for num_fences == 1
> - * we already own a new reference to the fence. For num_fence > 1
> - * we own the reference of the fence_array creation.
> - */
> if (num_fences == 1) {
> - sync_file->fence = fences[0];
> + sync_file->fence = fence_get(fences[0]);
> kfree(fences);
> } else {
> array = fence_array_create(num_fences, fences,
> @@ -180,10 +174,8 @@ static void add_fence(struct fence **fences, int *i,
> struct fence *fence)
> {
> fences[*i] = fence;
>
> - if (!fence_is_signaled(fence)) {
> - fence_get(fence);
> + if (!fence_is_signaled(fence))
> (*i)++;
> - }
> }
>
> /**
> @@ -255,7 +247,7 @@ static struct sync_file *sync_file_merge(const char
> *name, struct sync_file *a,
> add_fence(fences, &i, b_fences[i_b]);
>
> if (i == 0)
> - fences[i++] = fence_get(a_fences[0]);
> + fences[i++] = a_fences[0];
>
> if (num_fences > i) {
> nfences = krealloc(fences, i * sizeof(*fences),
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index bc4b22c..4ee7988 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -228,9 +228,6 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct
> amdgpu_ring *ring,
> struct fence_array *array;
> unsigned j;
>
> - for (j = 0; j < i; ++j)
> - fence_get(fences[j]);
> -
> array = fence_array_create(i, fences, fence_context,
> seqno, true);
> if (!array) {
