Am 26.04.24 um 18:43 schrieb Tvrtko Ursulin:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>
When commit b453e42a6e8b ("drm/amdgpu: Add new placement for preemptible
SG BOs") added a new TTM region it missed to notice the conceptual
imbalance in GART pin size accounting as done in amdgpu_bo_pin/unpin.
That imbalance leads to such objects getting accounted against the
resource, but are not un-accounted when unpinned.
Fix by extending the accounting criteria in amdgpu_bo_unpin.
What also aappears needs fixing is not reporting their size from the
amdgpu_bo_get_memory, which is used to implement fdinfo stats, so they are
not mixed with the regular userspace created and driver owned objects.
And also amdgpu_bo_print_info for debugfs reporting.
Note that the patch depends on the previous one which broke down the
relevant checks from the domain based to placement based.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>
Fixes: b453e42a6e8b ("drm/amdgpu: Add new placement for preemptible SG BOs")
Cc: Felix Kuehling <Felix.Kuehling@xxxxxxx>
Cc: Christian König <christian.koenig@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index fb984669fc3a..5a2bbc793953 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1032,7 +1032,8 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
atomic64_sub(amdgpu_bo_size(bo), &adev->vram_pin_size);
atomic64_sub(amdgpu_vram_mgr_bo_visible_size(bo),
&adev->visible_pin_size);
- } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT ||
+ bo->tbo.resource->mem_type == AMDGPU_PL_PREEMPT) {
Good catch, but please separate that one from the other changes since we
probably want to backport it.
atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
}
@@ -1298,7 +1299,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
stats->vram_shared += size;
break;
case TTM_PL_TT:
- case AMDGPU_PL_PREEMPT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
@@ -1599,7 +1599,6 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
placement = "VRAM";
break;
case TTM_PL_TT:
- case AMDGPU_PL_PREEMPT:
Yeah, that makes sense as well. But we need a case for AMDGPU_PL_PREEMPT
here as well then.
Regards,
Christian.
placement = "GTT";
break;
case TTM_PL_SYSTEM:
