On 2026/1/15 12:15, Amos Jianjun Kong wrote: > On Thu, Jan 15, 2026 at 11:02 AM Jiqian Chen <[email protected]> wrote: >> >> If drm_sched_job_init fails, hw_vm_fence is not freed currently, >> then cause memory leak. >> >> Signed-off-by: Jiqian Chen <[email protected]> >> --- >> v1->v2 changes: >> * assign the return code of drm_sched_job_init and check that instead. >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c >> index 7f5d01164897..1daa9145b217 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c >> @@ -219,8 +219,11 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct >> amdgpu_vm *vm, >> if (!entity) >> return 0; > > I have a question that may not be related to the PATCH. > > Why not check 'entity' at the beginning of function amdgpu_job_alloc()? > Currently if the 'entity' is invalid, the allocated 3 structs won't be > released. What I can see is not all jobs need to be associated with an entity when they are allocated. It allows entity to be NULL when allocate a new job, then return the job, like amdgpu_amdkfd_submit_ib().
> > >> - return drm_sched_job_init(&(*job)->base, entity, 1, owner, >> - drm_client_id); >> + r = drm_sched_job_init(&(*job)->base, entity, 1, owner, >> drm_client_id); >> + if (!r) >> + return 0; >> + >> + kfree((*job)->hw_vm_fence); > > The V2 looks good, it's already addressed the problem mentioned by Christian. > > Reviewed-by: Amos Kong <[email protected]> Thanks. > >> err_fence: >> kfree((*job)->hw_fence); >> -- >> 2.34.1 >> -- Best regards, Jiqian Chen.
