On Tue, Oct 05, 2021 at 01:37:27PM +0200, Christian König wrote:
> No need to actually allocate an array of fences here.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 26 +++++---------------------
> 1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 6b15cad78de9..e42dd79ed6f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2090,30 +2090,14 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device *adev,
> static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> {
> struct dma_resv *resv = vm->root.bo->tbo.base.resv;
> - struct dma_fence *excl, **shared;
> - unsigned i, shared_count;
> - int r;
> + struct dma_resv_iter cursor;
> + struct dma_fence *fence;
>
> - r = dma_resv_get_fences(resv, &excl, &shared_count, &shared);
> - if (r) {
> - /* Not enough memory to grab the fence list, as last resort
> - * block for all the fences to complete.
> - */
> - dma_resv_wait_timeout(resv, true, false,
> - MAX_SCHEDULE_TIMEOUT);
> - return;
> - }
> -
> - /* Add a callback for each fence in the reservation object */
> - amdgpu_vm_prt_get(adev);
I was confused for a bit why the old code wouldn't leak a refcount for
!excl case, but it's all handled.
Not sure amdgpu_vm_add_prt_cb still needs to handle the !fence case, it's
a bit a gotcha but I guess can happen?
Either way, looks correct.
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> - amdgpu_vm_add_prt_cb(adev, excl);
> -
> - for (i = 0; i < shared_count; ++i) {
> + dma_resv_for_each_fence(&cursor, resv, true, fence) {
> + /* Add a callback for each fence in the reservation object */
> amdgpu_vm_prt_get(adev);
> - amdgpu_vm_add_prt_cb(adev, shared[i]);
> + amdgpu_vm_add_prt_cb(adev, fence);
> }
> -
> - kfree(shared);
> }
>
> /**
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
