On 1/15/26 05: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.
That you don't have an entity is not an error but normal condition. It just means that the job is pushed directly to the HW without going through the scheduler. Regards, Christian. > > >> - 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]> > >> err_fence: >> kfree((*job)->hw_fence); >> -- >> 2.34.1 >>
