[PATCH 6/7] amdgpu: add support for memcg integration

[Christian König]

On 5/12/25 08:12, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> This adds the memcg object for any user allocated objects,
> add uses the MEMCG placement flags in the correct places.
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     |  5 ++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 16 +++++++++++-----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  2 ++
>  5 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 82df06a72ee0..1684a7e6d6cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -839,7 +839,10 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>  				union drm_amdgpu_cs *cs)
>  {
>  	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
> -	struct ttm_operation_ctx ctx = { true, false };
> +	struct ttm_operation_ctx ctx = {
> +		.interruptible = true,
> +		.no_wait_gpu = false,
> +	};

In general this change here is very welcomed, but should probably be in a separate patch.

Looks likely a leftover from when the flag was in the context.

Apart from that the patch series looks totally fine to me.

Regards,
Christian.

>  	struct amdgpu_vm *vm = &fpriv->vm;
>  	struct amdgpu_bo_list_entry *e;
>  	struct drm_gem_object *obj;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 69429df09477..bdad9a862ed3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -89,6 +89,7 @@ static void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>  	struct amdgpu_bo *aobj = gem_to_amdgpu_bo(gobj);
>  
>  	amdgpu_hmm_unregister(aobj);
> +	mem_cgroup_put(aobj->tbo.memcg);
>  	ttm_bo_put(&aobj->tbo);
>  }
>  
> @@ -116,6 +117,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>  	bp.domain = initial_domain;
>  	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
>  	bp.xcp_id_plus1 = xcp_id_plus1;
> +	bp.memcg = get_mem_cgroup_from_mm(current->mm);
>  
>  	r = amdgpu_bo_create_user(adev, &bp, &ubo);
>  	if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 0b9987781f76..1d930421354a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -158,7 +158,7 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
>  		places[c].mem_type =
>  			abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ?
>  			AMDGPU_PL_PREEMPT : TTM_PL_TT;
> -		places[c].flags = 0;
> +		places[c].flags = TTM_PL_FLAG_MEMCG;
>  		/*
>  		 * When GTT is just an alternative to VRAM make sure that we
>  		 * only use it as fallback and still try to fill up VRAM first.
> @@ -173,7 +173,7 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
>  		places[c].fpfn = 0;
>  		places[c].lpfn = 0;
>  		places[c].mem_type = TTM_PL_SYSTEM;
> -		places[c].flags = 0;
> +		places[c].flags = TTM_PL_FLAG_MEMCG;
>  		c++;
>  	}
>  
> @@ -657,16 +657,21 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>  		size = ALIGN(size, PAGE_SIZE);
>  	}
>  
> -	if (!amdgpu_bo_validate_size(adev, size, bp->domain))
> +	if (!amdgpu_bo_validate_size(adev, size, bp->domain)) {
> +		mem_cgroup_put(bp->memcg);
>  		return -ENOMEM;
> +	}
>  
>  	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo));
>  
>  	*bo_ptr = NULL;
>  	bo = kvzalloc(bp->bo_ptr_size, GFP_KERNEL);
> -	if (bo == NULL)
> +	if (bo == NULL) {
> +		mem_cgroup_put(bp->memcg);
>  		return -ENOMEM;
> +	}
>  	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
> +	bo->tbo.memcg = bp->memcg;
>  	bo->tbo.base.funcs = &amdgpu_gem_object_funcs;
>  	bo->vm_bo = NULL;
>  	bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain :
> @@ -1341,7 +1346,8 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
>  vm_fault_t amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
>  {
>  	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
> -	struct ttm_operation_ctx ctx = { false, false };
> +	struct ttm_operation_ctx ctx = { .interruptible = false,
> +					 .no_wait_gpu = false };
>  	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
>  	int r;
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 375448627f7b..9a4c506cfb76 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -55,6 +55,7 @@ struct amdgpu_bo_param {
>  	enum ttm_bo_type		type;
>  	bool				no_wait_gpu;
>  	struct dma_resv			*resv;
> +	struct mem_cgroup               *memcg;
>  	void				(*destroy)(struct ttm_buffer_object *bo);
>  	/* xcp partition number plus 1, 0 means any partition */
>  	int8_t				xcp_id_plus1;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 53b71e9d8076..f40b0c0a820b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -151,11 +151,13 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
>  			amdgpu_bo_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_GTT |
>  							AMDGPU_GEM_DOMAIN_CPU);
>  		}
> +		abo->placements[0].flags &= ~TTM_PL_FLAG_MEMCG;
>  		break;
>  	case TTM_PL_TT:
>  	case AMDGPU_PL_PREEMPT:
>  	default:
>  		amdgpu_bo_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_CPU);
> +		abo->placements[0].flags &= ~TTM_PL_FLAG_MEMCG;
>  		break;
>  	}
>  	*placement = abo->placement;