Skip to content

Commit f903b85

Browse files
committed
drm/amdgpu: fix possible fence leaks from job structure
If we don't end up initializing the fences, free them when we free the job. We can't set the hw_fence to NULL after emitting it because we need it in the cleanup path for the submit direct case. v2: take a reference to the fences if we emit them v3: handle non-job fence in error paths Fixes: db36632 ("drm/amdgpu: clean up and unify hw fence handling") Reviewed-by: Jesse Zhang <Jesse.Zhang@amd.com> (v1) Reviewed-by: Christian König <christian.koenig@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
1 parent d95ca7f commit f903b85

3 files changed

Lines changed: 35 additions & 4 deletions

File tree

drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,18 +176,21 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned int num_ibs,
176176

177177
if (!ring->sched.ready) {
178178
dev_err(adev->dev, "couldn't schedule ib on ring <%s>\n", ring->name);
179-
return -EINVAL;
179+
r = -EINVAL;
180+
goto free_fence;
180181
}
181182

182183
if (vm && !job->vmid) {
183184
dev_err(adev->dev, "VM IB without ID\n");
184-
return -EINVAL;
185+
r = -EINVAL;
186+
goto free_fence;
185187
}
186188

187189
if ((ib->flags & AMDGPU_IB_FLAGS_SECURE) &&
188190
(!ring->funcs->secure_submission_supported)) {
189191
dev_err(adev->dev, "secure submissions not supported on ring <%s>\n", ring->name);
190-
return -EINVAL;
192+
r = -EINVAL;
193+
goto free_fence;
191194
}
192195

193196
alloc_size = ring->funcs->emit_frame_size + num_ibs *
@@ -196,7 +199,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned int num_ibs,
196199
r = amdgpu_ring_alloc(ring, alloc_size);
197200
if (r) {
198201
dev_err(adev->dev, "scheduling IB failed (%d).\n", r);
199-
return r;
202+
goto free_fence;
200203
}
201204

202205
need_ctx_switch = ring->current_ctx != fence_ctx;
@@ -302,6 +305,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned int num_ibs,
302305
return r;
303306
}
304307
*f = &af->base;
308+
/* get a ref for the job */
309+
if (job)
310+
dma_fence_get(*f);
305311

306312
if (ring->funcs->insert_end)
307313
ring->funcs->insert_end(ring);
@@ -328,6 +334,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned int num_ibs,
328334
amdgpu_ring_commit(ring);
329335

330336
return 0;
337+
338+
free_fence:
339+
if (!job)
340+
kfree(af);
341+
return r;
331342
}
332343

333344
/**

drivers/gpu/drm/amd/amdgpu/amdgpu_job.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,15 @@ static void amdgpu_job_free_cb(struct drm_sched_job *s_job)
293293

294294
amdgpu_sync_free(&job->explicit_sync);
295295

296+
if (job->hw_fence->base.ops)
297+
dma_fence_put(&job->hw_fence->base);
298+
else
299+
kfree(job->hw_fence);
300+
if (job->hw_vm_fence->base.ops)
301+
dma_fence_put(&job->hw_vm_fence->base);
302+
else
303+
kfree(job->hw_vm_fence);
304+
296305
kfree(job);
297306
}
298307

@@ -322,6 +331,15 @@ void amdgpu_job_free(struct amdgpu_job *job)
322331
if (job->gang_submit != &job->base.s_fence->scheduled)
323332
dma_fence_put(job->gang_submit);
324333

334+
if (job->hw_fence->base.ops)
335+
dma_fence_put(&job->hw_fence->base);
336+
else
337+
kfree(job->hw_fence);
338+
if (job->hw_vm_fence->base.ops)
339+
dma_fence_put(&job->hw_vm_fence->base);
340+
else
341+
kfree(job->hw_vm_fence);
342+
325343
kfree(job);
326344
}
327345

drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,8 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job,
849849
if (r)
850850
return r;
851851
fence = &job->hw_vm_fence->base;
852+
/* get a ref for the job */
853+
dma_fence_get(fence);
852854
}
853855

854856
if (vm_flush_needed) {

0 commit comments

Comments
 (0)