On 11/6/26 15:18, Benjamin Cheng wrote:
>
> On 2026-11-06 03:55, Christian König wrote:
>> 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?
>
>
> I see, thanks I'll take a look at the previous patches.
>
> Should we consider reverting commit 009f401fb98b ("drm/amdgpu: move
> amdgpu_vm_bo_update() call into correct location") in the meantime then? As I
> wrote in the cover letter it exposes this issue and can now be detected by
> IGT tests, which makes our CI flaky.
>
What is the rational that "drm/amdgpu: move amdgpu_vm_bo_update() call into
correct location" should make this flaky?
As far as I can see that is completely unrelated.
Thanks,
Christian.
>
>> 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;
