2023-06-26 09:28:31

by Thomas Hellström

Subject: [PATCH v2 3/4] drm/ttm: Don't leak a resource on eviction error

On eviction errors other than -EMULTIHOP we were leaking a resource.
Fix.

v2:
- Avoid yet another goto (Andi Shyti)

Fixes: 403797925768 ("drm/ttm: Fix multihop assert on eviction.")
Cc: Andrey Grodzovsky <[email protected]>
Cc: Christian König <[email protected]>
Cc: Christian Koenig <[email protected]>
Cc: Huang Rui <[email protected]>
Cc: [email protected]
Cc: <[email protected]> # v5.15+
Signed-off-by: Thomas Hellström <[email protected]>
Reviewed-by: Nirmoy Das <[email protected]> #v1
---
drivers/gpu/drm/ttm/ttm_bo.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 615d30c4262d..c0e3bbd21d3d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -458,18 +458,18 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
goto out;
}

-bounce:
- ret = ttm_bo_handle_move_mem(bo, evict_mem, true, ctx, &hop);
- if (ret == -EMULTIHOP) {
+ do {
+ ret = ttm_bo_handle_move_mem(bo, evict_mem, true, ctx, &hop);
+ if (ret != -EMULTIHOP)
+ break;
+
ret = ttm_bo_bounce_temp_buffer(bo, &evict_mem, ctx, &hop);
- if (ret) {
- if (ret != -ERESTARTSYS && ret != -EINTR)
- pr_err("Buffer eviction failed\n");
- ttm_resource_free(bo, &evict_mem);
- goto out;
- }
- /* try and move to final place now. */
- goto bounce;
+ } while (!ret);
+
+ if (ret) {
+ ttm_resource_free(bo, &evict_mem);
+ if (ret != -ERESTARTSYS && ret != -EINTR)
+ pr_err("Buffer eviction failed\n");
}
out:
return ret;
--
2.40.1

2023-06-26 11:37:00

by Andi Shyti

Subject: Re: [PATCH v2 3/4] drm/ttm: Don't leak a resource on eviction error

Hi Thomas,

On Mon, Jun 26, 2023 at 11:14:49AM +0200, Thomas Hellstr?m wrote:
> On eviction errors other than -EMULTIHOP we were leaking a resource.
> Fix.
>
> v2:
> - Avoid yet another goto (Andi Shyti)
>
> Fixes: 403797925768 ("drm/ttm: Fix multihop assert on eviction.")
> Cc: Andrey Grodzovsky <[email protected]>
> Cc: Christian K?nig <[email protected]>
> Cc: Christian Koenig <[email protected]>
> Cc: Huang Rui <[email protected]>
> Cc: [email protected]
> Cc: <[email protected]> # v5.15+
> Signed-off-by: Thomas Hellstr?m <[email protected]>
> Reviewed-by: Nirmoy Das <[email protected]> #v1

Reviewed-by: Andi Shyti <[email protected]>

Andi

2023-06-26 12:01:08

by Christian König

Subject: Re: [PATCH v2 3/4] drm/ttm: Don't leak a resource on eviction error

Am 26.06.23 um 11:14 schrieb Thomas Hellström:
> On eviction errors other than -EMULTIHOP we were leaking a resource.
> Fix.
>
> v2:
> - Avoid yet another goto (Andi Shyti)
>
> Fixes: 403797925768 ("drm/ttm: Fix multihop assert on eviction.")
> Cc: Andrey Grodzovsky <[email protected]>
> Cc: Christian König <[email protected]>
> Cc: Christian Koenig <[email protected]>
> Cc: Huang Rui <[email protected]>
> Cc: [email protected]
> Cc: <[email protected]> # v5.15+
> Signed-off-by: Thomas Hellström <[email protected]>
> Reviewed-by: Nirmoy Das <[email protected]> #v1

Reviewed-by: Christian König <[email protected]>

> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 615d30c4262d..c0e3bbd21d3d 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -458,18 +458,18 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
> goto out;
> }
>
> -bounce:
> - ret = ttm_bo_handle_move_mem(bo, evict_mem, true, ctx, &hop);
> - if (ret == -EMULTIHOP) {
> + do {
> + ret = ttm_bo_handle_move_mem(bo, evict_mem, true, ctx, &hop);
> + if (ret != -EMULTIHOP)
> + break;
> +
> ret = ttm_bo_bounce_temp_buffer(bo, &evict_mem, ctx, &hop);
> - if (ret) {
> - if (ret != -ERESTARTSYS && ret != -EINTR)
> - pr_err("Buffer eviction failed\n");
> - ttm_resource_free(bo, &evict_mem);
> - goto out;
> - }
> - /* try and move to final place now. */
> - goto bounce;
> + } while (!ret);
> +
> + if (ret) {
> + ttm_resource_free(bo, &evict_mem);
> + if (ret != -ERESTARTSYS && ret != -EINTR)
> + pr_err("Buffer eviction failed\n");
> }
> out:
> return ret;