[PATCH] drm/amdgpu: Fix lifetime of struct amdgpu_task_info after ring reset

[PATCH] drm/amdgpu: Fix lifetime of struct amdgpu_task_info after ring reset

[André Almeida]

When a ring reset happens, amdgpu calls drm_dev_wedged_event() using
struct amdgpu_task_info *ti as one of the arguments. After using *ti, a
call to amdgpu_vm_put_task_info(ti) is required to correctly track its
lifetime.

However, it's called from a place that the ring reset path never reaches
due to a goto after drm_dev_wedged_event() is called. Move
amdgpu_vm_put_task_info() bellow the exit label to make sure that it's
called regardless of the code path.

amdgpu_vm_put_task_info() can only accept a valid address or NULL as
argument, so initialise *ti to make sure we can call this function if
*ti isn't used.

Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
Reported-by: Dave Airlie <airlied@gmail.com>
Closes: https://lore.kernel.org/dri-devel/CAPM=9tz0rQP8VZWKWyuF8kUMqRScxqoa6aVdwWw9=5yYxyYQ2Q@mail.gmail.com/
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 1e24590ae144..e69366401f6b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -90,7 +90,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
 	struct amdgpu_ring *ring = to_amdgpu_ring(s_job->sched);
 	struct amdgpu_job *job = to_amdgpu_job(s_job);
 	struct drm_wedge_task_info *info = NULL;
-	struct amdgpu_task_info *ti;
+	struct amdgpu_task_info *ti = NULL;
 	struct amdgpu_device *adev = ring->adev;
 	int idx;
 	int r;
@@ -172,8 +172,6 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
 	}
 	dma_fence_set_error(&s_job->s_fence->finished, -ETIME);
 
-	amdgpu_vm_put_task_info(ti);
-
 	if (amdgpu_device_should_recover_gpu(ring->adev)) {
 		struct amdgpu_reset_context reset_context;
 		memset(&reset_context, 0, sizeof(reset_context));
@@ -199,6 +197,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
 	}
 
 exit:
+	amdgpu_vm_put_task_info(ti);
 	drm_dev_exit(idx);
 	return DRM_GPU_SCHED_STAT_NOMINAL;
 }
-- 
2.49.0

Re: [PATCH] drm/amdgpu: Fix lifetime of struct amdgpu_task_info after ring reset

[Christian König]

On 04.07.25 05:06, André Almeida wrote:
> When a ring reset happens, amdgpu calls drm_dev_wedged_event() using
> struct amdgpu_task_info *ti as one of the arguments. After using *ti, a
> call to amdgpu_vm_put_task_info(ti) is required to correctly track its
> lifetime.
> 
> However, it's called from a place that the ring reset path never reaches
> due to a goto after drm_dev_wedged_event() is called. Move
> amdgpu_vm_put_task_info() bellow the exit label to make sure that it's
> called regardless of the code path.
> 
> amdgpu_vm_put_task_info() can only accept a valid address or NULL as
> argument, so initialise *ti to make sure we can call this function if
> *ti isn't used.
> 
> Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
> Reported-by: Dave Airlie <airlied@gmail.com>
> Closes: https://lore.kernel.org/dri-devel/CAPM=9tz0rQP8VZWKWyuF8kUMqRScxqoa6aVdwWw9=5yYxyYQ2Q@mail.gmail.com/
> Signed-off-by: André Almeida <andrealmeid@igalia.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 1e24590ae144..e69366401f6b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -90,7 +90,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>  	struct amdgpu_ring *ring = to_amdgpu_ring(s_job->sched);
>  	struct amdgpu_job *job = to_amdgpu_job(s_job);
>  	struct drm_wedge_task_info *info = NULL;
> -	struct amdgpu_task_info *ti;
> +	struct amdgpu_task_info *ti = NULL;
>  	struct amdgpu_device *adev = ring->adev;
>  	int idx;
>  	int r;
> @@ -172,8 +172,6 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>  	}
>  	dma_fence_set_error(&s_job->s_fence->finished, -ETIME);
>  
> -	amdgpu_vm_put_task_info(ti);
> -
>  	if (amdgpu_device_should_recover_gpu(ring->adev)) {
>  		struct amdgpu_reset_context reset_context;
>  		memset(&reset_context, 0, sizeof(reset_context));
> @@ -199,6 +197,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>  	}
>  
>  exit:
> +	amdgpu_vm_put_task_info(ti);
>  	drm_dev_exit(idx);
>  	return DRM_GPU_SCHED_STAT_NOMINAL;
>  }

Re: [PATCH] drm/amdgpu: Fix lifetime of struct amdgpu_task_info after ring reset

[André Almeida]

Em 04/07/2025 00:06, André Almeida escreveu:
> When a ring reset happens, amdgpu calls drm_dev_wedged_event() using
> struct amdgpu_task_info *ti as one of the arguments. After using *ti, a
> call to amdgpu_vm_put_task_info(ti) is required to correctly track its
> lifetime.
> 
> However, it's called from a place that the ring reset path never reaches
> due to a goto after drm_dev_wedged_event() is called. Move
> amdgpu_vm_put_task_info() bellow the exit label to make sure that it's
> called regardless of the code path.
> 
> amdgpu_vm_put_task_info() can only accept a valid address or NULL as
> argument, so initialise *ti to make sure we can call this function if
> *ti isn't used.
> 
> Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
> Reported-by: Dave Airlie <airlied@gmail.com>
> Closes: https://lore.kernel.org/dri-devel/CAPM=9tz0rQP8VZWKWyuF8kUMqRScxqoa6aVdwWw9=5yYxyYQ2Q@mail.gmail.com/
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---

This patch is now merged at drm-misc-next, thanks!