Thanks Christian. Since I don't have commit access, I assume that if all looks good the commit will be passed on to be applied?
Thanks, John On Fri, 9 Oct 2026 at 20:00, Christian König <[email protected]> wrote: > > On 10/8/26 22:45, John Fisher wrote: > > amdgpu_vm_make_compute() switches a VM from SDMA to CPU page table > > updates when a KFD process acquires it. It syncs "with the last SDMA > > update/clear" through amdgpu_bo_sync_wait() on the root PD, but that > > walks the reservation with DMA_RESV_USAGE_READ, and the clears of the > > VM's page tables (the root PD from amdgpu_vm_init(), and the PDs/PTs > > the seq64 mapping allocates at the render node's open) are added there > > by amdgpu_vm_sdma_commit() with DMA_RESV_USAGE_BOOKKEEP only: > > amdgpu_vm_pt_clear() commits them with a NULL fence, so they do not > > reach vm->last_update. The CPU path's own waits (amdgpu_vm_cpu_update(), > > amdgpu_bo_kmap()) use DMA_RESV_USAGE_KERNEL. None of them waits for a > > clear that is still queued. > > > > When one is, the CPU writes the first entries of the new compute VM (the > > KFD trap handler's TBA/TMA page at AMDGPU_VA_RESERVED_TRAP_START, mapped > > during ACQUIRE_VM) into a page directory the SDMA then clears. The > > entries are gone, the handler's page is unmapped, and the first wave > > save of any queue of the process faults in the SQC instruction fetch at > > the handler's address (GCVM_L2_PROTECTION_FAULT_STATUS 0x008012B1, > > PERMISSION_FAULTS 0xb). The MES then fails REMOVE_QUEUE and SUSPEND, and > > the device takes a MODE1 reset with VRAM lost on 7.2.9; on 7.3-rc6 the > > queue is reset instead. > > > > The window is the time between the render node's open and ACQUIRE_VM. > > ROCr spends milliseconds there and is practically never hit; a minimal > > KFD client that opens the render node and acquires the VM at once is hit > > most of the time. On a Radeon AI PRO R9700 (gfx1201, Resizable BAR, so > > vm_update_mode defaults to CPU updates for compute VMs), a wave reading > > the trap handler's page right after queue creation found it unmapped in > > 8 of 12 runs; a 5 ms sleep between the open and ACQUIRE_VM made that 0 > > of 9, and the same sleep after queue creation instead 3 of 3. > > > > The wait covered these jobs until commit 926c79ad6ecd ("drm/amdgpu: > > promote the implicit sync to the dependent read fences") narrowed > > amdgpu_sync_resv() from DMA_RESV_USAGE_BOOKKEEP to DMA_RESV_USAGE_READ; > > the VM's page table jobs have been added with BOOKKEEP since commit > > 4247084057cf ("drm/amdgpu: use DMA_RESV_USAGE_BOOKKEEP v2"), so from > > v6.16 on the sync in amdgpu_vm_make_compute() waits for nothing of > > them. > > > > Record the clears in vm->last_update, as amdgpu_vm_update_pdes() does > > with its updates, and wait for it, uninterruptibly, before switching the > > update functions. The clears all go through vm->delayed, so the last one > > covers the earlier ones; immediate clears are not recorded, as before. > > > > With this, on 7.3-rc6 and on 7.2.9, the same client reads the handler > > 10 of 10 times, 8 busy queues on the 4 compute slots complete in 5 of 5 > > runs on 7.3-rc6 and 3 of 3 on 7.2.9 (1 of 6 before), 16 and 32 queues > > complete on 7.3-rc6, and two processes with 8 queues each complete > > together on both, with no fault. > > > > Fixes: 926c79ad6ecd ("drm/amdgpu: promote the implicit sync to the > > dependent read fences") > > Cc: [email protected] # v6.16+ > > Cc: Prike Liang <[email protected]> > > Signed-off-by: John Fisher <[email protected]> > > Reviewed-by: Christian König <[email protected]> > > > --- > > v2: wait for vm->last_update, in which amdgpu_vm_pt_clear() now records > > the clears, instead of every BOOKKEEP fence on the root PD; wait > > uninterruptibly without checking the result; keep the original > > comment (Christian) > > v1: > > https://lore.kernel.org/amd-gfx/[email protected]/ > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 +---- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 2 +- > > 2 files changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > index 29187fabc..757a037ed 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > @@ -2737,10 +2737,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device > > *adev, struct amdgpu_vm *vm) > > > > if (vm->use_cpu_for_update) { > > /* Sync with last SDMA update/clear before switching to CPU */ > > - r = amdgpu_bo_sync_wait(vm->root.bo, > > - AMDGPU_FENCE_OWNER_UNDEFINED, true); > > - if (r) > > - goto unreserve_bo; > > + dma_fence_wait(vm->last_update, false); > > > > vm->update_funcs = &amdgpu_vm_cpu_funcs; > > r = amdgpu_vm_pt_map_tables(adev, vm); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > > index c03327f12..cdbc2e931 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > > @@ -427,7 +427,7 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, > > struct amdgpu_vm *vm, > > if (r) > > goto exit; > > > > - r = vm->update_funcs->commit(¶ms, NULL); > > + r = vm->update_funcs->commit(¶ms, &vm->last_update); > > exit: > > drm_dev_exit(idx); > > return r; >
