[PATCH v2] drm/ttm: fixup ttm_bo_add_move_fence

[PATCH v2] drm/ttm: fixup ttm_bo_add_move_fence

[Matthew Auld]

It looks like we still need to call dma_fence_put() on the man->move,
otherwise we just end up leaking it, leading to fireworks later.

v2(Daniel):
  - Simplify the function tail

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5689
Fixes: 8bb31587820a ("drm/ttm: remove bo->moving")
Cc: Christian König <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 015a94f766de..f7fbf162ce41 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -740,11 +740,9 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo,
 	dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL);
 
 	ret = dma_resv_reserve_fences(bo->base.resv, 1);
-	if (unlikely(ret)) {
-		dma_fence_put(fence);
-		return ret;
-	}
-	return 0;
+
+	dma_fence_put(fence);
+	return ret;
 }
 
 /*
-- 
2.34.1

[Intel-gfx] [PATCH v2] drm/ttm: fixup ttm_bo_add_move_fence

[Matthew Auld]

It looks like we still need to call dma_fence_put() on the man->move,
otherwise we just end up leaking it, leading to fireworks later.

v2(Daniel):
  - Simplify the function tail

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5689
Fixes: 8bb31587820a ("drm/ttm: remove bo->moving")
Cc: Christian König <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 015a94f766de..f7fbf162ce41 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -740,11 +740,9 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo,
 	dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL);
 
 	ret = dma_resv_reserve_fences(bo->base.resv, 1);
-	if (unlikely(ret)) {
-		dma_fence_put(fence);
-		return ret;
-	}
-	return 0;
+
+	dma_fence_put(fence);
+	return ret;
 }
 
 /*
-- 
2.34.1

Re: [PATCH v2] drm/ttm: fixup ttm_bo_add_move_fence

[Christian König]

Am 13.04.22 um 10:51 schrieb Matthew Auld:
> It looks like we still need to call dma_fence_put() on the man->move,
> otherwise we just end up leaking it, leading to fireworks later.
>
> v2(Daniel):
>    - Simplify the function tail
>
> Closes: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Fintel%2F-%2Fissues%2F5689&data=04%7C01%7Cchristian.koenig%40amd.com%7C65a22791ec204cccae9408da1d2ad7cd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637854367089944852%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rGZ4P%2FDUmHXObodJ9QIRynBWoxL2F%2FreypYI8JkiJoI%3D&reserved=0
> Fixes: 8bb31587820a ("drm/ttm: remove bo->moving")
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I've just pushed this version here to drm-misc-next.

Thanks,
Christian.

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 015a94f766de..f7fbf162ce41 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -740,11 +740,9 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo,
>   	dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL);
>   
>   	ret = dma_resv_reserve_fences(bo->base.resv, 1);
> -	if (unlikely(ret)) {
> -		dma_fence_put(fence);
> -		return ret;
> -	}
> -	return 0;
> +
> +	dma_fence_put(fence);
> +	return ret;
>   }
>   
>   /*

Re: [Intel-gfx] [PATCH v2] drm/ttm: fixup ttm_bo_add_move_fence

[Christian König]

Am 13.04.22 um 10:51 schrieb Matthew Auld:
> It looks like we still need to call dma_fence_put() on the man->move,
> otherwise we just end up leaking it, leading to fireworks later.
>
> v2(Daniel):
>    - Simplify the function tail
>
> Closes: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Fintel%2F-%2Fissues%2F5689&data=04%7C01%7Cchristian.koenig%40amd.com%7C65a22791ec204cccae9408da1d2ad7cd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637854367089944852%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rGZ4P%2FDUmHXObodJ9QIRynBWoxL2F%2FreypYI8JkiJoI%3D&reserved=0
> Fixes: 8bb31587820a ("drm/ttm: remove bo->moving")
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I've just pushed this version here to drm-misc-next.

Thanks,
Christian.

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 015a94f766de..f7fbf162ce41 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -740,11 +740,9 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo,
>   	dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL);
>   
>   	ret = dma_resv_reserve_fences(bo->base.resv, 1);
> -	if (unlikely(ret)) {
> -		dma_fence_put(fence);
> -		return ret;
> -	}
> -	return 0;
> +
> +	dma_fence_put(fence);
> +	return ret;
>   }
>   
>   /*