Re: [PATCH v3] drm/amdgpu: skip BOs being torn down during GTT recovery

On 8/5/26 10:58, Yifan Zhang wrote:
> A GPU reset can race with BO teardown after the BO's GTT resource has
> been marked for deletion but before its drm_mm node is removed. In this
> window, amdgpu_gtt_mgr_recover() can treat the node as a live BO and try
> to restore its GART mapping while its TT backing is being destroyed.
> 
> Recolor the GTT node from amdgpu_bo_delete_mem_notify() so that recovery
> skips it, reusing the existing color for ranges without a BO. The range
> stays allocated until the resource is freed.
> 
> This prevents reset recovery from accessing a BO whose backing storage
> is no longer valid.
> 
> v2: refine commit message. (David Francis)
> v3: Remove new BO color. (Christian)
> 
> Signed-off-by: Yifan Zhang <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 23 +++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
>  3 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 0ea32561c4bc..6219233ff712 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -102,6 +102,29 @@ bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *res)
>  	return drm_mm_node_allocated(&node->mm_nodes[0]);
>  }
>  
> +/**
> + * amdgpu_gtt_mgr_mark_bo_teardown - exclude a BO from GART recovery
> + *
> + * @tbo: TTM BO whose TT backing is about to be destroyed
> + *
> + * Keep the GART range allocated until the resource is freed, but make recovery
> + * treat it like a range without a BO so it isn't touched after TT teardown has
> + * started.
> + */
> +void amdgpu_gtt_mgr_mark_bo_teardown(struct ttm_buffer_object *tbo)

Avoiding the ttm_buffer object and giving ttm_resource_manager and ttm_resource separately would be better, but not a must have.

> +{
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
> +	struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(tbo->resource);
> +	struct amdgpu_gtt_mgr *mgr = &adev->mman.gtt_mgr;
> +
> +	dma_resv_assert_held(tbo->base.resv);
> +
> +	spin_lock(&mgr->lock);
> +	if (drm_mm_node_allocated(&node->mm_nodes[0]))
> +		node->mm_nodes[0].color = GART_ENTRY_WITHOUT_BO_COLOR;

We could rename GART_ENTRY_WITHOUT_BO_COLOR to something else, but also not a must have that should work for now.

Feel free to add Acked-by: Christian König <[email protected]> to the patch.

Regards,
Christian.


> +	spin_unlock(&mgr->lock);
> +}
> +
>  /**
>   * amdgpu_gtt_mgr_new - allocate a new node
>   *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index c4094edbdff1..a9067305b26d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1694,6 +1694,9 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>  static void
>  amdgpu_bo_delete_mem_notify(struct ttm_buffer_object *bo)
>  {
> +	if (bo->resource && bo->resource->mem_type == TTM_PL_TT)
> +		amdgpu_gtt_mgr_mark_bo_teardown(bo);
> +
>  	amdgpu_bo_move_notify(bo, false, NULL);
>  }
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index ff9e2e346609..af1e7fcc7175 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -145,6 +145,7 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev);
>  void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
>  
>  bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
> +void amdgpu_gtt_mgr_mark_bo_teardown(struct ttm_buffer_object *tbo);
>  void amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
>  
>  int amdgpu_gtt_mgr_alloc_entries(struct amdgpu_gtt_mgr *mgr,