On 11/5/26 22:00, Benjamin Cheng wrote: > In the happy path, the job gets freed when the job is submitted. > However, if amdgpu_vm_ptes_update failed, we don't submit the job, so we > would leak the job.
Yeah that was already brought up upstream as well, but that here is the wrong approach to fixing this. Thadeu Lima de Souza Cascardo <[email protected]> came up with a few patches on the public mailing list to tackle this as well. Essentially we need to commit PTE updates before we allocate any new PT, background is not only that allocating PTs can fail but also that we need to drop the lock for this and that can make the PTEs we already constructed in the SDMA IB invalid. I wanted to get that fixed for weeks but couldn't find time for it. Can you guys together look into that? Thanks in advance, Christian. > > Fixes: 009f401fb98b ("drm/amdgpu: move amdgpu_vm_bo_update() call into > correct location") > Signed-off-by: Benjamin Cheng <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 2ba33212d507..f6cb2b5854dd 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1274,8 +1274,11 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, > struct amdgpu_vm *vm, > > tmp = start + num_entries; > r = amdgpu_vm_ptes_update(¶ms, start, tmp, addr, flags); > - if (r) > + if (r) { > + if (params.job) > + amdgpu_job_free(params.job); > goto error_free; > + } > > amdgpu_res_next(&cursor, num_entries * AMDGPU_GPU_PAGE_SIZE); > start = tmp;
