Skip to content

Commit d3a9331

Browse files
ChristianKoenigAMDalexdeucher
ChristianKoenigAMD
authored and
alexdeucher
committed
drm/amdgpu: once more fix the call oder in amdgpu_ttm_move() v2
This reverts drm/amdgpu: fix ftrace event amdgpu_bo_move always move on same heap. The basic problem here is that after the move the old location is simply not available any more. Some fixes were suggested, but essentially we should call the move notification before actually moving things because only this way we have the correct order for DMA-buf and VM move notifications as well. Also rework the statistic handling so that we don't update the eviction counter before the move. v2: add missing NULL check Signed-off-by: Christian König <christian.koenig@amd.com> Fixes: 94aeb41 ("drm/amdgpu: fix ftrace event amdgpu_bo_move always move on same heap") Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3171 Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> CC: stable@vger.kernel.org
1 parent 46fe9cb commit d3a9331

3 files changed

Lines changed: 38 additions & 28 deletions

File tree

‎drivers/gpu/drm/amd/amdgpu/amdgpu_object.c‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,14 +1243,18 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void *buffer,
12431243
* amdgpu_bo_move_notify - notification about a memory move
12441244
* @bo: pointer to a buffer object
12451245
* @evict: if this move is evicting the buffer from the graphics address space
1246+
* @new_mem: new resource for backing the BO
12461247
*
12471248
* Marks the corresponding &amdgpu_bo buffer object as invalid, also performs
12481249
* bookkeeping.
12491250
* TTM driver callback which is called when ttm moves a buffer.
12501251
*/
1251-
void amdgpu_bo_move_notify(struct ttm_buffer_object *bo, bool evict)
1252+
void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
1253+
bool evict,
1254+
struct ttm_resource *new_mem)
12521255
{
12531256
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
1257+
struct ttm_resource *old_mem = bo->resource;
12541258
struct amdgpu_bo *abo;
12551259

12561260
if (!amdgpu_bo_is_amdgpu_bo(bo))
@@ -1262,12 +1266,12 @@ void amdgpu_bo_move_notify(struct ttm_buffer_object *bo, bool evict)
12621266
amdgpu_bo_kunmap(abo);
12631267

12641268
if (abo->tbo.base.dma_buf && !abo->tbo.base.import_attach &&
1265-
bo->resource->mem_type != TTM_PL_SYSTEM)
1269+
old_mem && old_mem->mem_type != TTM_PL_SYSTEM)
12661270
dma_buf_move_notify(abo->tbo.base.dma_buf);
12671271

1268-
/* remember the eviction */
1269-
if (evict)
1270-
atomic64_inc(&adev->num_evictions);
1272+
/* move_notify is called before move happens */
1273+
trace_amdgpu_bo_move(abo, new_mem ? new_mem->mem_type : -1,
1274+
old_mem ? old_mem->mem_type : -1);
12711275
}
12721276

12731277
void amdgpu_bo_get_memory(struct amdgpu_bo *bo,

‎drivers/gpu/drm/amd/amdgpu/amdgpu_object.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,9 @@ int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
328328
int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void *buffer,
329329
size_t buffer_size, uint32_t *metadata_size,
330330
uint64_t *flags);
331-
void amdgpu_bo_move_notify(struct ttm_buffer_object *bo, bool evict);
331+
void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
332+
bool evict,
333+
struct ttm_resource *new_mem);
332334
void amdgpu_bo_release_notify(struct ttm_buffer_object *bo);
333335
vm_fault_t amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo);
334336
void amdgpu_bo_fence(struct amdgpu_bo *bo, struct dma_fence *fence,

‎drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -481,14 +481,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
481481

482482
if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM &&
483483
bo->ttm == NULL)) {
484+
amdgpu_bo_move_notify(bo, evict, new_mem);
484485
ttm_bo_move_null(bo, new_mem);
485-
goto out;
486+
return 0;
486487
}
487488
if (old_mem->mem_type == TTM_PL_SYSTEM &&
488489
(new_mem->mem_type == TTM_PL_TT ||
489490
new_mem->mem_type == AMDGPU_PL_PREEMPT)) {
491+
amdgpu_bo_move_notify(bo, evict, new_mem);
490492
ttm_bo_move_null(bo, new_mem);
491-
goto out;
493+
return 0;
492494
}
493495
if ((old_mem->mem_type == TTM_PL_TT ||
494496
old_mem->mem_type == AMDGPU_PL_PREEMPT) &&
@@ -498,9 +500,10 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
498500
return r;
499501

500502
amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
503+
amdgpu_bo_move_notify(bo, evict, new_mem);
501504
ttm_resource_free(bo, &bo->resource);
502505
ttm_bo_assign_mem(bo, new_mem);
503-
goto out;
506+
return 0;
504507
}
505508

506509
if (old_mem->mem_type == AMDGPU_PL_GDS ||
@@ -512,8 +515,9 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
512515
new_mem->mem_type == AMDGPU_PL_OA ||
513516
new_mem->mem_type == AMDGPU_PL_DOORBELL) {
514517
/* Nothing to save here */
518+
amdgpu_bo_move_notify(bo, evict, new_mem);
515519
ttm_bo_move_null(bo, new_mem);
516-
goto out;
520+
return 0;
517521
}
518522

519523
if (bo->type == ttm_bo_type_device &&
@@ -525,22 +529,23 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
525529
abo->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
526530
}
527531

528-
if (adev->mman.buffer_funcs_enabled) {
529-
if (((old_mem->mem_type == TTM_PL_SYSTEM &&
530-
new_mem->mem_type == TTM_PL_VRAM) ||
531-
(old_mem->mem_type == TTM_PL_VRAM &&
532-
new_mem->mem_type == TTM_PL_SYSTEM))) {
533-
hop->fpfn = 0;
534-
hop->lpfn = 0;
535-
hop->mem_type = TTM_PL_TT;
536-
hop->flags = TTM_PL_FLAG_TEMPORARY;
537-
return -EMULTIHOP;
538-
}
532+
if (adev->mman.buffer_funcs_enabled &&
533+
((old_mem->mem_type == TTM_PL_SYSTEM &&
534+
new_mem->mem_type == TTM_PL_VRAM) ||
535+
(old_mem->mem_type == TTM_PL_VRAM &&
536+
new_mem->mem_type == TTM_PL_SYSTEM))) {
537+
hop->fpfn = 0;
538+
hop->lpfn = 0;
539+
hop->mem_type = TTM_PL_TT;
540+
hop->flags = TTM_PL_FLAG_TEMPORARY;
541+
return -EMULTIHOP;
542+
}
539543

544+
amdgpu_bo_move_notify(bo, evict, new_mem);
545+
if (adev->mman.buffer_funcs_enabled)
540546
r = amdgpu_move_blit(bo, evict, new_mem, old_mem);
541-
} else {
547+
else
542548
r = -ENODEV;
543-
}
544549

545550
if (r) {
546551
/* Check that all memory is CPU accessible */
@@ -555,11 +560,10 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
555560
return r;
556561
}
557562

558-
trace_amdgpu_bo_move(abo, new_mem->mem_type, old_mem->mem_type);
559-
out:
560-
/* update statistics */
563+
/* update statistics after the move */
564+
if (evict)
565+
atomic64_inc(&adev->num_evictions);
561566
atomic64_add(bo->base.size, &adev->num_bytes_moved);
562-
amdgpu_bo_move_notify(bo, evict);
563567
return 0;
564568
}
565569

@@ -1559,7 +1563,7 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
15591563
static void
15601564
amdgpu_bo_delete_mem_notify(struct ttm_buffer_object *bo)
15611565
{
1562-
amdgpu_bo_move_notify(bo, false);
1566+
amdgpu_bo_move_notify(bo, false, NULL);
15631567
}
15641568

15651569
static struct ttm_device_funcs amdgpu_bo_driver = {

0 commit comments

Comments
 (0)