Message ID | 20220222024651.36675-1-qiang.yu@amd.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] drm/amdgpu: check vm ready by amdgpu_vm->evicting flag | expand |
Dear Qiang, Am 22.02.22 um 03:46 schrieb Qiang Yu: > Workstation application ANSA/META v21.1.4 get this error dmesg when > running CI test suite provided by ANSA/META: > [drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16) > > This is caused by: > 1. create a 256MB buffer in invisible VRAM > 2. CPU map the buffer and access it causes vm_fault and try to move > it to visible VRAM > 3. force visible VRAM space and traverse all VRAM bos to check if > evicting this bo is valuable > 4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable() > will set amdgpu_vm->evicting, but latter due to not in visible > VRAM, won't really evict it so not add it to amdgpu_vm->evicted > 5. before next CS to clear the amdgpu_vm->evicting, user VM ops > ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted) > but fail in amdgpu_vm_bo_update_mapping() (check > amdgpu_vm->evicting) and get this error log > > This error won't affect functionality as next CS will finish the > waiting VM ops. But we'd better clear the error log by checking > the amdgpu_vm->evicting flag in amdgpu_vm_ready() to stop calling > amdgpu_vm_bo_update_mapping() latter. later > Another reason is amdgpu_vm->evicted list holds all BOs (both > user buffer and page table), but only page table BOs' eviction > prevent VM ops. amdgpu_vm->evicting flag is set only for page > table BOs, so we should use evicting flag instead of evicted list > in amdgpu_vm_ready(). > > The side effect of This change is: previously blocked VM op (user this > buffer in "evicted" list but no page table in it) gets done > immediately. > > v2: update commit comments. > > Reviewed-by: Christian König <christian.koenig@amd.com> > Signed-off-by: Qiang Yu <qiang.yu@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 37acd8911168..2cd9f1a2e5fa 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, > * Check if all VM PDs/PTs are ready for updates > * > * Returns: > - * True if eviction list is empty. > + * True if VM is not evicting. > */ > bool amdgpu_vm_ready(struct amdgpu_vm *vm) > { > - return list_empty(&vm->evicted); > + bool ret; > + > + amdgpu_vm_eviction_lock(vm); > + ret = !vm->evicting; > + amdgpu_vm_eviction_unlock(vm); > + return ret; > } > > /** Acked-by: Paul Menzel <pmenzel@molgen.mpg.de> Kind regards, Paul
On Wed, Feb 23, 2022 at 3:47 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > Dear Qiang, > > > Am 22.02.22 um 03:46 schrieb Qiang Yu: > > Workstation application ANSA/META v21.1.4 get this error dmesg when > > running CI test suite provided by ANSA/META: > > [drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16) > > > > This is caused by: > > 1. create a 256MB buffer in invisible VRAM > > 2. CPU map the buffer and access it causes vm_fault and try to move > > it to visible VRAM > > 3. force visible VRAM space and traverse all VRAM bos to check if > > evicting this bo is valuable > > 4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable() > > will set amdgpu_vm->evicting, but latter due to not in visible > > VRAM, won't really evict it so not add it to amdgpu_vm->evicted > > 5. before next CS to clear the amdgpu_vm->evicting, user VM ops > > ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted) > > but fail in amdgpu_vm_bo_update_mapping() (check > > amdgpu_vm->evicting) and get this error log > > > > This error won't affect functionality as next CS will finish the > > waiting VM ops. But we'd better clear the error log by checking > > the amdgpu_vm->evicting flag in amdgpu_vm_ready() to stop calling > > amdgpu_vm_bo_update_mapping() latter. > > later > > Another reason is amdgpu_vm->evicted list holds all BOs (both > > user buffer and page table), but only page table BOs' eviction > > prevent VM ops. amdgpu_vm->evicting flag is set only for page > > table BOs, so we should use evicting flag instead of evicted list > > in amdgpu_vm_ready(). > > > > The side effect of This change is: previously blocked VM op (user > > this > > > buffer in "evicted" list but no page table in it) gets done > > immediately. > > > > v2: update commit comments. > > > > Reviewed-by: Christian König <christian.koenig@amd.com> > > Signed-off-by: Qiang Yu <qiang.yu@amd.com> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > index 37acd8911168..2cd9f1a2e5fa 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > @@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, > > * Check if all VM PDs/PTs are ready for updates > > * > > * Returns: > > - * True if eviction list is empty. > > + * True if VM is not evicting. > > */ > > bool amdgpu_vm_ready(struct amdgpu_vm *vm) > > { > > - return list_empty(&vm->evicted); > > + bool ret; > > + > > + amdgpu_vm_eviction_lock(vm); > > + ret = !vm->evicting; > > + amdgpu_vm_eviction_unlock(vm); > > + return ret; > > } > > > > /** > > Acked-by: Paul Menzel <pmenzel@molgen.mpg.de> > Thanks, will submit with the typo fixed. Regards, Qiang > > Kind regards, > > Paul
this patch cause suspend to disk resume amdgpu hang, [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=8330, emitted seq=8332 https://gitlab.freedesktop.org/drm/amd/-/issues/1915
https://gitlab.freedesktop.org/drm/amd/-/issues/1922 2022-03-01 1:26 GMT+08:00, youling257 <youling257@gmail.com>: > this patch cause suspend to disk resume amdgpu hang, > [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled > seq=8330, emitted seq=8332 > > https://gitlab.freedesktop.org/drm/amd/-/issues/1915 >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 37acd8911168..2cd9f1a2e5fa 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, * Check if all VM PDs/PTs are ready for updates * * Returns: - * True if eviction list is empty. + * True if VM is not evicting. */ bool amdgpu_vm_ready(struct amdgpu_vm *vm) { - return list_empty(&vm->evicted); + bool ret; + + amdgpu_vm_eviction_lock(vm); + ret = !vm->evicting; + amdgpu_vm_eviction_unlock(vm); + return ret; } /**