Re: [PATCH 1/5] drm/amdgpu: Fix possible refcount leak for release of external_hw_fence

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2022-06-22 05:00, Christian König wrote:
Am 21.06.22 um 21:34 schrieb Andrey Grodzovsky:
On 2022-06-21 03:19, Christian König wrote:

Am 21.06.22 um 00:02 schrieb Andrey Grodzovsky:
Problem:
In amdgpu_job_submit_direct - The refcount should drop by 2
but it drops only by 1.

amdgpu_ib_sched->emit -> refcount 1 from first fence init
dma_fence_get -> refcount 2
dme_fence_put -> refcount 1

Fix:
Add put for external_hw_fence in amdgpu_job_free/free_cb

Well what is the external_hw_fence good for in this construct?


As far as I understand for direct submissions you don't want to pass a job pointer to ib_schedule and so u can't use the embedded fence for this case.

Can you please look a bit deeper into this, we now have a couple of fields in the job structure which have no obvious use.

I think we could pass a job structure to ib_schedule even for direct submit now.


Are you sure  ? I see a lot of activities in amdgpu_ib_schedule depend on presence of  vm and fence_ctx which are set if the job pointer argument != NULL, might this have a negative impact on direct submit ?

Andrey



Regards,
Christian.


Andrey




Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 10aa073600d4..58568fdde2d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -152,8 +152,10 @@ static void amdgpu_job_free_cb(struct drm_sched_job *s_job)
      /* only put the hw fence if has embedded fence */
      if (job->hw_fence.ops != NULL)
          dma_fence_put(&job->hw_fence);
-    else
+    else {

When one side of the if uses {} the other side should use {} as well, e.g. use } else { here.

Christian.

+ dma_fence_put(job->external_hw_fence);
          kfree(job);
+    }
  }
    void amdgpu_job_free(struct amdgpu_job *job)
@@ -165,8 +167,10 @@ void amdgpu_job_free(struct amdgpu_job *job)
      /* only put the hw fence if has embedded fence */
      if (job->hw_fence.ops != NULL)
          dma_fence_put(&job->hw_fence);
-    else
+    else {
+        dma_fence_put(job->external_hw_fence);
          kfree(job);
+    }
  }
    int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux