amdgpu_vm_update_stats_locked() recomputed the vm->stats[] bucket index (bo_memtype) fresh from bo->preferred_domains on every call, for both add and remove. bo->preferred_domains is a mutable property of the shared amdgpu_bo, not of the individual amdgpu_vm_bo_base attachment, so if it changes between when an attachment is added and when it is later removed (e.g. amdgpu_amdkfd_gpuvm_pin_bo() pinning a BO into the DOORBELL/MMIO_REMAP aperture after it was already attached as CPU), the add credits one bucket and the matching remove debits a different one. This permanently corrupts both buckets even though the aggregate add/remove counts balance, and eventually trips the "VM memory stats ... is non-zero when fini" warning.
Cache the bucket on amdgpu_vm_bo_base at add time and reuse it on removal instead of recomputing it, so an attachment's remove always targets the same bucket its add credited. amdgpu_vm_bo_move()'s remove-then-add pair for a relocated BO still works correctly: the remove reuses the cache from the prior placement, and the add recomputes and refreshes the cache for the new placement. Signed-off-by: Eric Huang <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 +++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 10 ++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 94b1cfe05e3e..45c25be72c6c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -358,11 +358,18 @@ static void amdgpu_vm_update_stats_locked(struct amdgpu_vm_bo_base *base, struct amdgpu_vm *vm = base->vm; struct amdgpu_bo *bo = base->bo; int64_t size = sign * amdgpu_bo_size(bo); - uint32_t bo_memtype = amdgpu_bo_mem_stats_placement(bo); + uint32_t bo_memtype; /* For drm-total- and drm-shared-, BO are accounted by their preferred - * placement, see also amdgpu_bo_mem_stats_placement. + * placement, see also amdgpu_bo_mem_stats_placement. Cache the bucket + * on @base when adding so a later remove reuses the same bucket even + * if bo->preferred_domains changed in the meantime, instead of + * recomputing it and leaking vm->stats in two mismatched buckets. */ + if (sign > 0) + base->bo_memtype = amdgpu_bo_mem_stats_placement(bo); + bo_memtype = base->bo_memtype; + if (base->shared) vm->stats[bo_memtype].drm.shared += size; else diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 7a51ca284b99..e37fb238e3d0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -221,6 +221,16 @@ struct amdgpu_vm_bo_base { * protected by vm BO being reserved */ bool shared; + /* vm->stats[] bucket this base's size is currently counted under, + * cached from bo->preferred_domains at the last amdgpu_vm_update_stats + * add. bo->preferred_domains can change after a BO is attached (e.g. + * KFD pinning a BO to the DOORBELL/MMIO aperture), so a later removal + * must reuse this cached bucket instead of recomputing it, or the + * add and the matching remove can land in different buckets and leak + * vm->stats permanently. + */ + uint32_t bo_memtype; + /* if the BO was moved and all mappings are invalid * protected by the BO being reserved */ bool moved; -- 2.43.0
