On 28.09.2017 20:39, Andres Rodriguez wrote:


On 2017-09-28 10:55 AM, Nicolai Hähnle wrote:
From: Nicolai Hähnle <nicolai.haeh...@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.haeh...@amd.com>
Acked-by: Christian König <christian.koe...@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?

Sure, I can add a comment. (It's amd_sched_process_job, which is called directly or indirectly in all the branches of the following if-statement.)


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

Thanks.


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

You may be running into this Mesa issue:

https://patchwork.freedesktop.org/patch/179535/

Cheers,
Nicolai



+
          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

Reply via email to