Re: [PATCH 4/5] drm/amd/sched: NULL out the s_fence field after run_job

[Andres Rodriguez <andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 2017-09-28 10:55 AM, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
> 
> amd_sched_process_job drops the fence reference, so NULL out the s_fence
> field before adding it as a callback to guard against accidentally using
> s_fence after it may have be freed.
> 
> Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
> Acked-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index e793312e351c..54eb77cffd9b 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -604,20 +604,23 @@ static int amd_sched_main(void *param)
>   		if (!sched_job)
>   			continue;
>   
>   		s_fence = sched_job->s_fence;
>   
>   		atomic_inc(&sched->hw_rq_count);
>   		amd_sched_job_begin(sched_job);
>   
>   		fence = sched->ops->run_job(sched_job);
>   		amd_sched_fence_scheduled(s_fence);
> +
> +		sched_job->s_fence = NULL;

Minor optional nitpick here. Could this be moved somewhere closer to 
where the fence reference is actually dropped? Alternatively, could a 
comment be added to specify which function call results in the reference 
ownership transfer?

Whether a change is made or not, this series is
Reviewed-by: Andres Rodriguez <andresx7@gmail.com>

Currently running piglit to check if this fixes the occasional soft 
hangs I was getting where all tests complete except one.

> +
>   		if (fence) {
>   			s_fence->parent = dma_fence_get(fence);
>   			r = dma_fence_add_callback(fence, &s_fence->cb,
>   						   amd_sched_process_job);
>   			if (r == -ENOENT)
>   				amd_sched_process_job(fence, &s_fence->cb);
>   			else if (r)
>   				DRM_ERROR("fence add callback failed (%d)\n",
>   					  r);
>   			dma_fence_put(fence);
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx