Re: [PATCH 10/11] drm/gem: Use dynamic allocation for GEM object dma_resv

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Sep 03, 2026 at 03:28:05PM +0200, Christian König wrote:
> Remove the embedded dma_resv (_resv) from struct drm_gem_object and use
> dynamically allocated reservations instead. This change aligns with the
> new reference counted dma_resv infrastructure and reduces the size of
> drm_gem_object.
> 
> In drm_gem_private_object_init(), when no external resv is provided,
> allocate a new dma_resv using dma_resv_alloc() instead of initializing
> the embedded _resv field. This allows proper error handling if the
> allocation fails, returning -ENOMEM to the caller.
> 
> The corresponding cleanup in drm_gem_private_object_fini() is simplified
> to only put the resv pointer, removing the redundant put on _resv.
> 
> For TTM buffer objects, add an individual_resv field to track the
> dynamically allocated reservation object for proper cleanup during
> destruction. This ensures we can distinguish between shared and
> individual reservations when freeing resources.
> 
> This change builds on the previous work to make drm_gem_private_object_init()
> return an error code, now utilizing that capability to handle allocation
> failures properly.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Assisted-by: Claude:Sonnet 4
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 12 ++---
>  drivers/gpu/drm/drm_gem.c                     |  9 ++--
>  drivers/gpu/drm/i915/gem/i915_gem_object.c    |  1 -
>  drivers/gpu/drm/nouveau/nouveau_bo.c          |  3 --
>  .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  |  2 +-
>  drivers/gpu/drm/ttm/ttm_bo.c                  | 26 ++++++-----
>  drivers/gpu/drm/ttm/ttm_bo_util.c             | 44 +++++++++++++------
>  drivers/gpu/drm/xe/xe_bo.c                    |  8 ++--
>  include/drm/drm_gem.h                         |  9 ----
>  include/drm/ttm/ttm_bo.h                      |  2 +
>  11 files changed, 65 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 20831dbebc31..a168083edab2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -397,7 +397,7 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo,
>   */
>  void amdgpu_amdkfd_remove_all_eviction_fences(struct amdgpu_bo *bo)
>  {
> -	struct dma_resv *resv = &bo->tbo.base._resv;
> +	struct dma_resv *resv = bo->tbo.individual_resv;
>  	struct dma_fence *fence, *stub;
>  	struct dma_resv_iter cursor;
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 6c5182d54f7d..026821f6da7c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1319,7 +1319,7 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
>  	 * So when this locking here fails something is wrong with the reference
>  	 * counting.
>  	 */
> -	if (WARN_ON_ONCE(!dma_resv_trylock(&bo->base._resv)))
> +	if (WARN_ON_ONCE(!dma_resv_trylock(bo->individual_resv)))
>  		return;
>  
>  	amdgpu_amdkfd_remove_all_eviction_fences(abo);
> @@ -1329,22 +1329,22 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
>  	    adev->in_suspend || drm_dev_is_unplugged(adev_to_drm(adev)))
>  		goto out;
>  
> -	r = dma_resv_reserve_fences(&bo->base._resv, 1);
> +	r = dma_resv_reserve_fences(bo->individual_resv, 1);
>  	if (r)
>  		goto out;
>  
>  	r = amdgpu_ttm_clear_buffer(amdgpu_ttm_next_clear_entity(adev),
> -				    abo, &bo->base._resv, &fence,
> -				    false, AMDGPU_KERNEL_JOB_ID_CLEAR_ON_RELEASE);
> +				    abo, bo->individual_resv, &fence, false,
> +				    AMDGPU_KERNEL_JOB_ID_CLEAR_ON_RELEASE);
>  	if (WARN_ON(r))
>  		goto out;
>  
>  	amdgpu_vram_mgr_set_cleared(bo->resource);
> -	dma_resv_add_fence(&bo->base._resv, fence, DMA_RESV_USAGE_KERNEL);
> +	dma_resv_add_fence(bo->individual_resv, fence, DMA_RESV_USAGE_KERNEL);
>  	dma_fence_put(fence);
>  
>  out:
> -	dma_resv_unlock(&bo->base._resv);
> +	dma_resv_unlock(bo->individual_resv);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index d2ce18bbddea..480391df556d 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -235,9 +235,11 @@ int drm_gem_private_object_init(struct drm_device *dev,
>  	obj->handle_count = 0;
>  	obj->size = size;
>  	mutex_init(&obj->gpuva.lock);
> -	dma_resv_init(&obj->_resv);
> -	if (!obj->resv)
> -		obj->resv = dma_resv_get(&obj->_resv);
> +	if (!obj->resv) {
> +		obj->resv = dma_resv_alloc();
> +		if (!obj->resv)
> +			return -ENOMEM;
> +	}
>  
>  	drm_gem_gpuva_init(obj);
>  
> @@ -259,7 +261,6 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
>  	WARN_ON(obj->dma_buf);
>  
>  	dma_resv_put(obj->resv);
> -	dma_resv_put(&obj->_resv);
>  	mutex_destroy(&obj->gpuva.lock);
>  }
>  EXPORT_SYMBOL(drm_gem_private_object_fini);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 3f37b0cbb70f..1c73f15d0315 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -90,7 +90,6 @@ struct drm_i915_gem_object *i915_gem_object_alloc(void)
>  void i915_gem_object_free(struct drm_i915_gem_object *obj)
>  {
>  	dma_resv_put(obj->base.resv);
> -	dma_resv_put(&obj->base._resv);
>  	return kmem_cache_free(slab_objects, obj);
>  }
>  
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 67c9d32f4f27..65e1f0c2f984 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -159,8 +159,6 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo)
>  			drm_gem_object_put(nvbo->r_obj);
>  
>  		drm_gem_object_release(&bo->base);
> -	} else {
> -		dma_resv_put(&bo->base._resv);
>  	}
>  
>  	kfree(nvbo);
> @@ -385,7 +383,6 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align,
>  		return PTR_ERR(nvbo);
>  
>  	nvbo->bo.base.size = size;
> -	dma_resv_init(&nvbo->bo.base._resv);
>  	drm_vma_node_reset(&nvbo->bo.base.vma_node);
>  
>  	/* This must be called before ttm_bo_init_reserved(). Subsequent
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> index e0ecadccccf8..4559352e7115 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> @@ -640,7 +640,7 @@ static void ttm_bo_validate_no_placement_not_signaled(struct kunit *test)
>  	KUNIT_EXPECT_EQ(test, ctx.bytes_moved, 0);
>  
>  	if (bo->type != ttm_bo_type_sg)
> -		KUNIT_ASSERT_PTR_EQ(test, bo->base.resv, &bo->base._resv);
> +		KUNIT_ASSERT_PTR_EQ(test, bo->base.resv, &bo->individual_resv);
>  
>  	/* Make sure we have an idle object at this point */
>  	dma_resv_wait_timeout(bo->base.resv, usage, false, MAX_SCHEDULE_TIMEOUT);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 999c24251fb7..ecb5f55a0dd1 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -193,13 +193,13 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo)
>  {
>  	int r;
>  
> -	if (bo->base.resv == &bo->base._resv)
> +	if (bo->base.resv == bo->individual_resv)
>  		return 0;
>  
> -	BUG_ON(!dma_resv_trylock(&bo->base._resv));
> +	BUG_ON(!dma_resv_trylock(bo->individual_resv));
>  
> -	r = dma_resv_copy_fences(&bo->base._resv, bo->base.resv);
> -	dma_resv_unlock(&bo->base._resv);
> +	r = dma_resv_copy_fences(bo->individual_resv, bo->base.resv);
> +	dma_resv_unlock(bo->individual_resv);
>  	if (r)
>  		return r;
>  
> @@ -209,7 +209,7 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo)
>  		 * the resv object while holding the lru_lock.
>  		 */
>  		spin_lock(&bo->bdev->lru_lock);
> -		drm_gem_object_set_resv(&bo->base, &bo->base._resv);
> +		drm_gem_object_set_resv(&bo->base, bo->individual_resv);
>  		spin_unlock(&bo->bdev->lru_lock);
>  	}
>  
> @@ -218,7 +218,7 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo)
>  
>  static void ttm_bo_flush_all_fences(struct ttm_buffer_object *bo)
>  {
> -	struct dma_resv *resv = &bo->base._resv;
> +	struct dma_resv *resv = bo->individual_resv;
>  	struct dma_resv_iter cursor;
>  	struct dma_fence *fence;
>  
> @@ -238,8 +238,8 @@ static void ttm_bo_delayed_delete(struct work_struct *work)
>  
>  	bo = container_of(work, typeof(*bo), delayed_delete);
>  
> -	dma_resv_wait_timeout(&bo->base._resv, DMA_RESV_USAGE_BOOKKEEP, false,
> -			      MAX_SCHEDULE_TIMEOUT);
> +	dma_resv_wait_timeout(bo->individual_resv, DMA_RESV_USAGE_BOOKKEEP,
> +			      false, MAX_SCHEDULE_TIMEOUT);
>  	dma_resv_lock(bo->base.resv, NULL);
>  	ttm_bo_cleanup_memtype_use(bo);
>  	dma_resv_unlock(bo->base.resv);
> @@ -273,7 +273,7 @@ static void ttm_bo_release(struct kref *kref)
>  		drm_vma_offset_remove(bdev->vma_manager, &bo->base.vma_node);
>  		ttm_mem_io_free(bdev, bo->resource);
>  
> -		if (!dma_resv_test_signaled(&bo->base._resv,
> +		if (!dma_resv_test_signaled(bo->individual_resv,
>  					    DMA_RESV_USAGE_BOOKKEEP) ||
>  		    (want_init_on_free() && (bo->ttm != NULL)) ||
>  		    bo->type == ttm_bo_type_sg ||
> @@ -316,6 +316,8 @@ static void ttm_bo_release(struct kref *kref)
>  	}
>  
>  	atomic_dec(&ttm_glob.bo_count);
> +	dma_resv_put(bo->individual_resv);
> +	bo->individual_resv = NULL;

individual_resv is set to NULL.

>  	bo->destroy(bo);

vfunc ->destroy() called and this the only call site.

>  }
>  
> @@ -1197,7 +1199,11 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo,
>  	bo->pin_count = 0;
>  	bo->sg = sg;
>  	bo->bulk_move = NULL;
> -	drm_gem_object_set_resv(&bo->base, resv ?: &bo->base._resv);
> +
> +	/* Save the original resv object before overwriting it */
> +	bo->individual_resv = dma_resv_get(bo->base.resv);
> +	if (resv)
> +		drm_gem_object_set_resv(&bo->base, resv);
>  	atomic_inc(&ttm_glob.bo_count);
>  
>  	/*
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index e080ce60933e..e39448a5015e 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -208,7 +208,7 @@ static void ttm_transfered_destroy(struct ttm_buffer_object *bo)

This is the vfunc ->destroy() for transfer destroys.

>  
>  	fbo = container_of(bo, struct ttm_transfer_obj, base);
>  	dma_resv_put(fbo->base.base.resv);
> -	dma_resv_put(&fbo->base.base._resv);
> +	dma_resv_put(fbo->base.individual_resv);

fbo->base.individual_resv should always be NULL per above comments.

Not functionally incorrect as dma_resv_put has a NULL check, but this
confused me in my analysis of the refcounting.

Can we either delete this or WARN_ON_ONCE(fbo->base.individual_resv) to
future proof this?

Otherwise Xe, TTM, and DRM changes LGTM.

Matt

>  	ttm_bo_put(fbo->bo);
>  	kfree(fbo);
>  }
> @@ -238,12 +238,23 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
>  	if (!fbo)
>  		return -ENOMEM;
>  
> -	fbo->base = *bo;
>  
>  	/**
>  	 * Fix up members that we shouldn't copy directly:
>  	 * TODO: Explicit member copy would probably be better here.
>  	 */
> +	fbo->base = *bo;
> +
> +	fbo->base.individual_resv = dma_resv_alloc();
> +	if (!fbo->base.individual_resv) {
> +		ret = -ENOMEM;
> +		goto error_free;
> +	}
> +
> +	if (bo->type != ttm_bo_type_sg)
> +		fbo->base.base.resv = dma_resv_get(fbo->base.individual_resv);
> +	else
> +		dma_resv_get(fbo->base.base.resv);
>  
>  	atomic_inc(&ttm_glob.bo_count);
>  	drm_vma_node_reset(&fbo->base.base.vma_node);
> @@ -251,19 +262,16 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
>  	kref_init(&fbo->base.kref);
>  	fbo->base.destroy = &ttm_transfered_destroy;
>  	fbo->base.pin_count = 0;
> -	if (bo->type != ttm_bo_type_sg)
> -		fbo->base.base.resv = &fbo->base.base._resv;
>  
> -	dma_resv_init(&fbo->base.base._resv);
>  	fbo->base.base.dev = NULL;
> -	ret = dma_resv_trylock(&fbo->base.base._resv);
> +	ret = dma_resv_trylock(fbo->base.individual_resv);
>  	WARN_ON(!ret);
>  
> -	ret = dma_resv_reserve_fences(&fbo->base.base._resv, TTM_NUM_MOVE_FENCES);
> +	ret = dma_resv_reserve_fences(fbo->base.individual_resv,
> +				      TTM_NUM_MOVE_FENCES);
>  	if (ret) {
> -		dma_resv_unlock(&fbo->base.base._resv);
> -		kfree(fbo);
> -		return ret;
> +		dma_resv_unlock(fbo->base.individual_resv);
> +		goto error_unref;
>  	}
>  
>  	if (fbo->base.resource) {
> @@ -281,6 +289,14 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
>  
>  	*new_obj = &fbo->base;
>  	return 0;
> +
> +error_unref:
> +	dma_resv_put(fbo->base.individual_resv);
> +	dma_resv_put(fbo->base.base.resv);
> +
> +error_free:
> +	kfree(fbo);
> +	return ret;
>  }
>  
>  /**
> @@ -618,7 +634,7 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo,
>  	if (ret)
>  		return ret;
>  
> -	dma_resv_add_fence(&ghost_obj->base._resv, fence,
> +	dma_resv_add_fence(ghost_obj->individual_resv, fence,
>  			   DMA_RESV_USAGE_KERNEL);
>  
>  	/**
> @@ -632,7 +648,7 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo,
>  	else
>  		bo->ttm = NULL;
>  
> -	dma_resv_unlock(&ghost_obj->base._resv);
> +	dma_resv_unlock(ghost_obj->individual_resv);
>  	ttm_bo_put(ghost_obj);
>  	return 0;
>  }
> @@ -802,14 +818,14 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
>  	if (ret)
>  		goto error_destroy_tt;
>  
> -	ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv);
> +	ret = dma_resv_copy_fences(ghost->individual_resv, bo->base.resv);
>  	/* Last resort, wait for the BO to be idle when we are OOM */
>  	if (ret) {
>  		dma_resv_wait_timeout(bo->base.resv, DMA_RESV_USAGE_BOOKKEEP,
>  				      false, MAX_SCHEDULE_TIMEOUT);
>  	}
>  
> -	dma_resv_unlock(&ghost->base._resv);
> +	dma_resv_unlock(ghost->individual_resv);
>  	ttm_bo_put(ghost);
>  	bo->ttm = ttm;
>  	return 0;
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 6df7ef07ec0b..422fe4bddf00 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1660,7 +1660,7 @@ static bool xe_ttm_bo_lock_in_destructor(struct ttm_buffer_object *ttm_bo)
>  	 * always succeed here, as long as we hold the lru lock.
>  	 */
>  	spin_lock(&ttm_bo->bdev->lru_lock);
> -	locked = dma_resv_trylock(&ttm_bo->base._resv);
> +	locked = dma_resv_trylock(ttm_bo->individual_resv);
>  	spin_unlock(&ttm_bo->bdev->lru_lock);
>  	xe_assert(xe, locked);
>  
> @@ -1689,14 +1689,14 @@ static void xe_ttm_bo_release_notify(struct ttm_buffer_object *ttm_bo)
>  	 * TODO: Don't do this for external bos once we scrub them after
>  	 * unbind.
>  	 */
> -	dma_resv_for_each_fence(&cursor, &ttm_bo->base._resv,
> +	dma_resv_for_each_fence(&cursor, ttm_bo->individual_resv,
>  				DMA_RESV_USAGE_BOOKKEEP, fence) {
>  		if (xe_fence_is_xe_preempt(fence) &&
>  		    !dma_fence_is_signaled(fence)) {
>  			if (!replacement)
>  				replacement = dma_fence_get_stub();
>  
> -			dma_resv_replace_fences(&ttm_bo->base._resv,
> +			dma_resv_replace_fences(ttm_bo->individual_resv,
>  						fence->context,
>  						replacement,
>  						DMA_RESV_USAGE_BOOKKEEP);
> @@ -1704,7 +1704,7 @@ static void xe_ttm_bo_release_notify(struct ttm_buffer_object *ttm_bo)
>  	}
>  	dma_fence_put(replacement);
>  
> -	dma_resv_unlock(&ttm_bo->base._resv);
> +	dma_resv_unlock(ttm_bo->individual_resv);
>  }
>  
>  static void xe_ttm_bo_delete_mem_notify(struct ttm_buffer_object *ttm_bo)
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index f889232e8525..8aed680bfca2 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -385,15 +385,6 @@ struct drm_gem_object {
>  	 */
>  	struct dma_resv *resv;
>  
> -	/**
> -	 * @_resv:
> -	 *
> -	 * A reservation object for this GEM object.
> -	 *
> -	 * This is unused for imported GEM objects.
> -	 */
> -	struct dma_resv _resv;
> -
>  	/**
>  	 * @gpuva: Fields used by GPUVM to manage mappings pointing to this GEM object.
>  	 *
> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
> index 32791c4db2a9..f132caa9647f 100644
> --- a/include/drm/ttm/ttm_bo.h
> +++ b/include/drm/ttm/ttm_bo.h
> @@ -77,6 +77,7 @@ enum ttm_bo_type {
>   * @bdev: Pointer to the buffer object device structure.
>   * @type: The bo type.
>   * @page_alignment: Page alignment.
> + * @individual_resv: Individual resv for destruction
>   * @destroy: Destruction function. If NULL, kfree is used.
>   * @kref: Reference count of this buffer object. When this refcount reaches
>   * zero, the object is destroyed or put on the delayed delete list.
> @@ -107,6 +108,7 @@ struct ttm_buffer_object {
>  	struct ttm_device *bdev;
>  	enum ttm_bo_type type;
>  	uint32_t page_alignment;
> +	struct dma_resv *individual_resv;
>  	void (*destroy) (struct ttm_buffer_object *);
>  
>  	/*
> -- 
> 2.43.0
>