The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: "Christian König" <christian.koenig@amd.com>,
	"Natalie Vock" <natalie.vock@gmx.de>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Tvrtko Ursulin" <tursulin@ursulin.net>,
	"Huang Rui" <ray.huang@amd.com>,
	"Matthew Auld" <matthew.auld@intel.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 02/10] drm/ttm: replace TTMs refcount with the DRM refcount v4
Date: Mon, 6 Jul 2026 11:05:53 -0700	[thread overview]
Message-ID: <akvugeeF5zfAauf3@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <akvt1GtqVKVVtmEU@gsse-cloud1.jf.intel.com>

On Mon, Jul 06, 2026 at 11:03:00AM -0700, Matthew Brost wrote:
> On Mon, Jul 06, 2026 at 07:53:12PM +0200, Thomas Hellström wrote:
> > On Mon, 2026-07-06 at 10:51 -0700, Matthew Brost wrote:
> > > On Mon, Jul 06, 2026 at 07:01:27PM +0200, Thomas Hellström wrote:
> > > > On Mon, 2026-07-06 at 16:49 +0200, Christian König wrote:
> > > > > On 7/6/26 15:14, Thomas Hellström wrote:
> > > > > > On Fri, 2026-07-03 at 18:31 +0200, Natalie Vock wrote:
> > > > > > > From: Christian König <christian.koenig@amd.com>
> > > > > > > 
> > > > > > > Instead of keeping a separate reference count for the TTM
> > > > > > > object
> > > > > > > also
> > > > > > > use
> > > > > > > the reference count for DRM GEM objects inside TTM.
> > > > > > > 
> > > > > > > Apart from avoiding two reference counts for one object this
> > > > > > > approach
> > > > > > > has
> > > > > > > the clear advantage of being able to use drm_exec inside TTM.
> > > > > > > 
> > > > > > > v2: adjust XE assert as well and re-enable disabled test
> > > > > > > v3: handle another case in i915
> > > > > > > v4: set GEM driver funcs of transfer BOs to point to the TTM
> > > > > > > free
> > > > > > > callback (Natalie)
> > > > > > 
> > > > > > I think the main review issue from the last time this was on
> > > > > > the
> > > > > > table
> > > > > > was that we shouldn't resurrect the gem refcount. Apart from
> > > > > > the
> > > > > > risc
> > > > > > of getting barriers wrong, both xe and IIRC i915 partly rely on
> > > > > > the
> > > > > > gem
> > > > > > refcount never being resurrected and that callbacks for bos
> > > > > > with
> > > > > > zero
> > > > > > gem refcount means that the gem part of the object is unusable.
> > > > > 
> > > > > I've spend quite some time thinking about that and came to the
> > > > > conclusion that this is actually harmless.
> > > > > 
> > > > > The drivers shouldn't be able to see the resurected BO, except if
> > > > > they go over the LRU list manually (which they shouldn't).
> > > > 
> > > > The shrinker uses the TTM helpers for this. Basically the check
> > > > needs
> > > > to be ported to use the zombie interface but the present change
> > > > also
> > > > widens the window where we can't evict / shrink at all due to zero
> > > > refcounts.
> > > > 
> > > > While it might be made harmless, resurrecting a refcount like this
> > > > is
> > > > IMO not something that should leak into the gem refcount. Nobody
> > > > else
> > > > does this in the kernel tree. The bo in reality becomes a zombie
> > > > once
> > > > the gem refcount reaches zero.
> > > 
> > > When you say resurrecting a refcount - what exactly do you mean by
> > > this?
> > > 
> > > ref -> 0 -> init ref count 1 -> 0 again?
> > 
> > Yes, the destructor re-initializes it to 1 again. Prevously this was
> > confined to the TTM refcount only and had limited visibility.
> > 

Opps, missed pasting in a line here:

Ok, yes, this is also something in the pipeline for Xe related to CPU bindings:

> 
> Put a PT BO on a deferred list -> reinit -> sometime later, do an async
> put. This is all done using the TTM refcount. Of course, we don't have
> to do it this way, though.
> 
> In general, I agree that tricks like this are not the most desirable
> approach. If I recall correctly, I implemented this in the Xe code
> because it followed an existing paradigm. If we want to merge the TTM
> and GEM refcounts, I agree that we should likely move away from this
> paradigm, since the GEM refcount significantly expands its scope.
> 
> Matt
> 
> > /Thomas
> > 
> > > 
> > > Matt
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > For example xe_bo.c:
> > > > > > 
> > > > > > 	if (!xe_bo_is_xe_bo(bo) ||
> > > > > > !xe_bo_get_unless_zero(xe_bo))
> > > > > > 		return xe_bo_shrink_purge(ctx, bo, scanned);
> > > > > > 
> > > > > > So IIRC the conclusion was when removing the ttm refcount we
> > > > > > shouldn't
> > > > > > attempt to resurrect the gem one. If the get_unless_zero()
> > > > > > fails
> > > > > > during
> > > > > > evict walk, we simply find something to wait for. See previous
> > > > > > discussion there.
> > > > > 
> > > > > Yeah, I considered that as well but the problem is we often
> > > > > doesn't
> > > > > have anything to wait on.
> > > > 
> > > > That's not the conclusion of the previous discussion?
> > > > 
> > > > https://lore.kernel.org/dri-devel/20250716160555.20217-2-christian.koenig@amd.com/#r
> > > > 
> > > > > 
> > > > > > 
> > > > > > I fully support removing the ttm refcount, but not if it means
> > > > > > resurrecting the gem refcount.
> > > > > > 
> > > > > > If we want to sidestep that problem, in favour of getting the
> > > > > > proposed
> > > > > > locking functionality in and future proof it, I suggest using
> > > > > > 
> > > > > > https://lore.kernel.org/all/20260605112700.181040-1-thomas.hellstrom@linux.intel.com/
> > > > > > 
> > > > > > And rebase this series on that. This means we can use the ttm
> > > > > > refcount
> > > > > > for the transaction refcounting, and also that if we add a dma-
> > > > > > buf
> > > > > > map
> > > > > > interface with a dma_resv_txn_obj, we could use that to also
> > > > > > have
> > > > > > exhaustive eviction that originates from a dma_buf map.
> > > > > 
> > > > > I don't think that this is a good idea. It just adds another
> > > > > layer of
> > > > > abstraction and doesn't solve the problem in any way possible.
> > > > 
> > > > This comment confuses me. Exactly what problem isn't solved by
> > > > this,
> > > > and which of the stated benefits/use-cases in the cover-letter do
> > > > you
> > > > think aren't worthwhile?
> > > > 
> > > > Also for reference: (Section at the end and follow-up messages)
> > > > https://lore.kernel.org/all/3716d43462188590743060755b37e3d060f7600f.camel@linux.intel.com/
> > > > 
> > > > Thanks,
> > > > Thomas
> > > > 
> > > > 
> > > > > Regards,
> > > > > Christian.
> > > > > 
> > > > > > 
> > > > > > /Thomas
> > > > > > 
> > > > > > > 
> > > > > > > Signed-off-by: tChristian König <christian.koenig@amd.com>
> > > > > > > Signed-off-by: Natalie Vock <natalie.vock@gmx.de>
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/gem/i915_gem_ttm.c          |  35 +++--
> > > > > > > -
> > > > > > >  drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c |   8 +-
> > > > > > >  drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c    |   2 -
> > > > > > >  drivers/gpu/drm/ttm/ttm_bo.c                     | 135
> > > > > > > +++++++++++--
> > > > > > > ----------
> > > > > > >  drivers/gpu/drm/ttm/ttm_bo_internal.h            |  17 ++-
> > > > > > >  drivers/gpu/drm/ttm/ttm_bo_util.c                |  15 ++-
> > > > > > >  drivers/gpu/drm/xe/xe_bo.c                       |   2 +-
> > > > > > >  include/drm/ttm/ttm_bo.h                         |   9 --
> > > > > > >  8 files changed, 111 insertions(+), 112 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > > > > > > b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > > > > > > index df3fcc2b1248e..642296602de69 100644
> > > > > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > > > > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > > > > > > @@ -939,7 +939,7 @@ void i915_ttm_adjust_lru(struct
> > > > > > > drm_i915_gem_object *obj)
> > > > > > >  	 * Don't manipulate the TTM LRUs while in TTM bo
> > > > > > > destruction.
> > > > > > >  	 * We're called through
> > > > > > > i915_ttm_delete_mem_notify().
> > > > > > >  	 */
> > > > > > > -	if (!kref_read(&bo->kref))
> > > > > > > +	if (!kref_read(&bo->base.refcount))
> > > > > > >  		return;
> > > > > > >  
> > > > > > >  	/*
> > > > > > > @@ -957,30 +957,21 @@ void i915_ttm_adjust_lru(struct
> > > > > > > drm_i915_gem_object *obj)
> > > > > > >  	 *
> > > > > > >  	 * TODO: consider maybe also bumping the shrinker
> > > > > > > list
> > > > > > > here
> > > > > > > when we have
> > > > > > >  	 * already unpinned it, which should give us
> > > > > > > something
> > > > > > > more
> > > > > > > like an LRU.
> > > > > > > -	 *
> > > > > > > -	 * TODO: There is a small window of opportunity for
> > > > > > > this
> > > > > > > function to
> > > > > > > -	 * get called from eviction after we've dropped the
> > > > > > > last
> > > > > > > GEM
> > > > > > > refcount,
> > > > > > > -	 * but before the TTM deleted flag is set on the
> > > > > > > object.
> > > > > > > Avoid
> > > > > > > -	 * adjusting the shrinker list in such cases, since
> > > > > > > the
> > > > > > > object is
> > > > > > > -	 * not available to the shrinker anyway due to its
> > > > > > > zero
> > > > > > > refcount.
> > > > > > > -	 * To fix this properly we should move to a TTM
> > > > > > > shrinker
> > > > > > > LRU
> > > > > > > list for
> > > > > > > -	 * these objects.
> > > > > > >  	 */
> > > > > > > -	if (kref_get_unless_zero(&obj->base.refcount)) {
> > > > > > > -		if (shrinkable != obj->mm.ttm_shrinkable) {
> > > > > > > -			if (shrinkable) {
> > > > > > > -				if (obj->mm.madv ==
> > > > > > > I915_MADV_WILLNEED)
> > > > > > > -
> > > > > > > 					__i915_gem_object_ma
> > > > > > > ke_s
> > > > > > > hrinkable(obj);
> > > > > > > -				else
> > > > > > > -
> > > > > > > 					__i915_gem_object_ma
> > > > > > > ke_p
> > > > > > > urgeable(obj);
> > > > > > > -			} else {
> > > > > > > -
> > > > > > > 				i915_gem_object_make_unshrin
> > > > > > > kabl
> > > > > > > e(obj);
> > > > > > > -			}
> > > > > > > -
> > > > > > > -			obj->mm.ttm_shrinkable = shrinkable;
> > > > > > > +	i915_gem_object_get(obj);
> > > > > > > +	if (shrinkable != obj->mm.ttm_shrinkable) {
> > > > > > > +		if (shrinkable) {
> > > > > > > +			if (obj->mm.madv ==
> > > > > > > I915_MADV_WILLNEED)
> > > > > > > +				__i915_gem_object_make_shrin
> > > > > > > kabl
> > > > > > > e(ob
> > > > > > > j);
> > > > > > > +			else
> > > > > > > +				__i915_gem_object_make_purge
> > > > > > > able
> > > > > > > (obj
> > > > > > > );
> > > > > > > +		} else {
> > > > > > > +			i915_gem_object_make_unshrinkable(ob
> > > > > > > j);
> > > > > > >  		}
> > > > > > > -		i915_gem_object_put(obj);
> > > > > > > +
> > > > > > > +		obj->mm.ttm_shrinkable = shrinkable;
> > > > > > >  	}
> > > > > > > +	i915_gem_object_put(obj);
> > > > > > >  
> > > > > > >  	/*
> > > > > > >  	 * Put on the correct LRU list depending on the MADV
> > > > > > > status
> > > > > > > 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 56ad8ef325840..904cb4da6c9b3 100644
> > > > > > > --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > > > > > > +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > > > > > > @@ -127,7 +127,7 @@ static void
> > > > > > > ttm_bo_init_reserved_sys_man(struct
> > > > > > > kunit *test)
> > > > > > >  	dma_resv_unlock(bo->base.resv);
> > > > > > >  
> > > > > > >  	KUNIT_EXPECT_EQ(test, err, 0);
> > > > > > > -	KUNIT_EXPECT_EQ(test, kref_read(&bo->kref), 1);
> > > > > > > +	KUNIT_EXPECT_EQ(test, kref_read(&bo->base.refcount),
> > > > > > > 1);
> > > > > > >  	KUNIT_EXPECT_PTR_EQ(test, bo->bdev, priv->ttm_dev);
> > > > > > >  	KUNIT_EXPECT_EQ(test, bo->type, bo_type);
> > > > > > >  	KUNIT_EXPECT_EQ(test, bo->page_alignment,
> > > > > > > PAGE_SIZE);
> > > > > > > @@ -176,7 +176,7 @@ static void
> > > > > > > ttm_bo_init_reserved_mock_man(struct
> > > > > > > kunit *test)
> > > > > > >  	dma_resv_unlock(bo->base.resv);
> > > > > > >  
> > > > > > >  	KUNIT_EXPECT_EQ(test, err, 0);
> > > > > > > -	KUNIT_EXPECT_EQ(test, kref_read(&bo->kref), 1);
> > > > > > > +	KUNIT_EXPECT_EQ(test, kref_read(&bo->base.refcount),
> > > > > > > 1);
> > > > > > >  	KUNIT_EXPECT_PTR_EQ(test, bo->bdev, priv->ttm_dev);
> > > > > > >  	KUNIT_EXPECT_EQ(test, bo->type, bo_type);
> > > > > > >  	KUNIT_EXPECT_EQ(test, ctx.bytes_moved, size);
> > > > > > > @@ -969,6 +969,8 @@ static void
> > > > > > > ttm_bo_validate_allowed_only_evict(struct kunit *test)
> > > > > > >  	ttm_mock_manager_fini(priv->ttm_dev, mem_multihop);
> > > > > > >  }
> > > > > > >  
> > > > > > > +extern const struct drm_gem_object_funcs
> > > > > > > ttm_deleted_object_funcs;
> > > > > > > +
> > > > > > >  static void ttm_bo_validate_deleted_evict(struct kunit
> > > > > > > *test)
> > > > > > >  {
> > > > > > >  	struct ttm_operation_ctx ctx_init = { }, ctx_val  =
> > > > > > > { };
> > > > > > > @@ -999,7 +1001,7 @@ static void
> > > > > > > ttm_bo_validate_deleted_evict(struct
> > > > > > > kunit *test)
> > > > > > >  	KUNIT_EXPECT_EQ(test,
> > > > > > > ttm_resource_manager_usage(man),
> > > > > > > big);
> > > > > > >  
> > > > > > >  	dma_resv_unlock(bo_big->base.resv);
> > > > > > > -	bo_big->deleted = true;
> > > > > > > +	bo_big->base.funcs = &ttm_deleted_object_funcs;
> > > > > > >  
> > > > > > >  	bo_small = ttm_bo_kunit_init(test, test->priv,
> > > > > > > small,
> > > > > > > NULL);
> > > > > > >  	bo_small->type = bo_type;
> > > > > > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > > > b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > > > index 5cfe8f3f80d75..b7ab19e0e4b2b 100644
> > > > > > > --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > > > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > > > @@ -189,8 +189,6 @@ struct ttm_buffer_object
> > > > > > > *ttm_bo_kunit_init(struct kunit *test,
> > > > > > >  	bo->bdev = devs->ttm_dev;
> > > > > > >  	bo->destroy = dummy_ttm_bo_destroy;
> > > > > > >  
> > > > > > > -	kref_init(&bo->kref);
> > > > > > > -
> > > > > > >  	return bo;
> > > > > > >  }
> > > > > > >  EXPORT_SYMBOL_GPL(ttm_bo_kunit_init);
> > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > > > b/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > > > index 3980f376e3ba4..2b470c1746f60 100644
> > > > > > > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > > > @@ -246,88 +246,84 @@ static void
> > > > > > > ttm_bo_delayed_delete(struct
> > > > > > > work_struct *work)
> > > > > > >  	ttm_bo_put(bo);
> > > > > > >  }
> > > > > > >  
> > > > > > > -static void ttm_bo_release(struct kref *kref)
> > > > > > > +/*
> > > > > > > + * All other callbacks should never ever be called on a
> > > > > > > deleted
> > > > > > > TTM
> > > > > > > object.
> > > > > > > + */
> > > > > > > +const struct drm_gem_object_funcs ttm_deleted_object_funcs =
> > > > > > > {
> > > > > > > +	.free = ttm_bo_free
> > > > > > > +};
> > > > > > > +EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_deleted_object_funcs);
> > > > > > > +
> > > > > > > +/* Returns true if the BO is about to get deleted */
> > > > > > > +static bool ttm_bo_is_zombie(struct ttm_buffer_object *bo)
> > > > > > > +{
> > > > > > > +	return bo->base.funcs == &ttm_deleted_object_funcs;
> > > > > > > +}
> > > > > > > +
> > > > > > > +void ttm_bo_fini(struct ttm_buffer_object *bo)
> > > > > > >  {
> > > > > > > -	struct ttm_buffer_object *bo =
> > > > > > > -	    container_of(kref, struct ttm_buffer_object,
> > > > > > > kref);
> > > > > > >  	struct ttm_device *bdev = bo->bdev;
> > > > > > >  	int ret;
> > > > > > >  
> > > > > > >  	WARN_ON_ONCE(bo->pin_count);
> > > > > > >  	WARN_ON_ONCE(bo->bulk_move);
> > > > > > >  
> > > > > > > -	if (!bo->deleted) {
> > > > > > > -		ret = ttm_bo_individualize_resv(bo);
> > > > > > > -		if (ret) {
> > > > > > > -			/* Last resort, if we fail to
> > > > > > > allocate
> > > > > > > memory for the
> > > > > > > -			 * fences block for the BO to become
> > > > > > > idle
> > > > > > > -			 */
> > > > > > > -			dma_resv_wait_timeout(bo->base.resv,
> > > > > > > -					     
> > > > > > > DMA_RESV_USAGE_BOOKKEEP, false,
> > > > > > > -					      30 * HZ);
> > > > > > > -		}
> > > > > > > +	ret = ttm_bo_individualize_resv(bo);
> > > > > > > +	if (ret) {
> > > > > > > +		/* Last resort, if we fail to allocate
> > > > > > > memory
> > > > > > > for
> > > > > > > the
> > > > > > > +		 * fences block for the BO to become idle
> > > > > > > +		 */
> > > > > > > +		dma_resv_wait_timeout(bo->base.resv,
> > > > > > > DMA_RESV_USAGE_BOOKKEEP,
> > > > > > > +				      false, 30 * HZ);
> > > > > > > +	}
> > > > > > >  
> > > > > > > -		if (bdev->funcs->release_notify)
> > > > > > > -			bdev->funcs->release_notify(bo);
> > > > > > > +	if (bo->bdev->funcs->release_notify)
> > > > > > > +		bo->bdev->funcs->release_notify(bo);
> > > > > > >  
> > > > > > > -		drm_vma_offset_remove(bdev->vma_manager,
> > > > > > > &bo-
> > > > > > > > base.vma_node);
> > > > > > > -		ttm_mem_io_free(bdev, bo->resource);
> > > > > > > +	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,
> > > > > > > -					   
> > > > > > > DMA_RESV_USAGE_BOOKKEEP)
> > > > > > > > > 
> > > > > > > -		    (want_init_on_free() && (bo->ttm !=
> > > > > > > NULL))
> > > > > > > > > 
> > > > > > > -		    bo->type == ttm_bo_type_sg ||
> > > > > > > -		    !dma_resv_trylock(bo->base.resv)) {
> > > > > > > -			/* The BO is not idle, resurrect it
> > > > > > > for
> > > > > > > delayed destroy */
> > > > > > > -			ttm_bo_flush_all_fences(bo);
> > > > > > > -			bo->deleted = true;
> > > > > > > +	if (!dma_resv_test_signaled(&bo->base._resv,
> > > > > > > DMA_RESV_USAGE_BOOKKEEP) ||
> > > > > > > +	    (want_init_on_free() && (bo->ttm != NULL)) ||
> > > > > > > +	    bo->type == ttm_bo_type_sg ||
> > > > > > > +	    !dma_resv_trylock(bo->base.resv)) {
> > > > > > > +		/* The BO is not idle, resurrect it for
> > > > > > > delayed
> > > > > > > destroy */
> > > > > > > +		ttm_bo_flush_all_fences(bo);
> > > > > > >  
> > > > > > > -			spin_lock(&bdev->lru_lock);
> > > > > > > -
> > > > > > > -			/*
> > > > > > > -			 * Make pinned bos immediately
> > > > > > > available
> > > > > > > to
> > > > > > > -			 * shrinkers, now that they are
> > > > > > > queued
> > > > > > > for
> > > > > > > -			 * destruction.
> > > > > > > -			 *
> > > > > > > -			 * FIXME: QXL is triggering this.
> > > > > > > Can be
> > > > > > > removed when the
> > > > > > > -			 * driver is fixed.
> > > > > > > -			 */
> > > > > > > -			if (bo->pin_count) {
> > > > > > > -				bo->pin_count = 0;
> > > > > > > -
> > > > > > > 				ttm_resource_move_to_lru_tai
> > > > > > > l(bo-
> > > > > > > > resource);
> > > > > > > -			}
> > > > > > > +		spin_lock(&bo->bdev->lru_lock);
> > > > > > >  
> > > > > > > -			kref_init(&bo->kref);
> > > > > > > -			spin_unlock(&bdev->lru_lock);
> > > > > > > +		/*
> > > > > > > +		 * Make pinned bos immediately available to
> > > > > > > +		 * shrinkers, now that they are queued for
> > > > > > > +		 * destruction.
> > > > > > > +		 *
> > > > > > > +		 * FIXME: QXL is triggering this. Can be
> > > > > > > removed
> > > > > > > when the
> > > > > > > +		 * driver is fixed.
> > > > > > > +		 */
> > > > > > > +		if (bo->pin_count) {
> > > > > > > +			bo->pin_count = 0;
> > > > > > > +			ttm_resource_move_to_lru_tail(bo-
> > > > > > > > resource);
> > > > > > > +		}
> > > > > > >  
> > > > > > > -			INIT_WORK(&bo->delayed_delete,
> > > > > > > ttm_bo_delayed_delete);
> > > > > > > +		kref_init(&bo->base.refcount);
> > > > > > > +		bo->base.funcs = &ttm_deleted_object_funcs;
> > > > > > > +		spin_unlock(&bo->bdev->lru_lock);
> > > > > > >  
> > > > > > > -			/* Schedule the worker on the
> > > > > > > closest
> > > > > > > NUMA
> > > > > > > node. This
> > > > > > > -			 * improves performance since system
> > > > > > > memory
> > > > > > > might be
> > > > > > > -			 * cleared on free and that is best
> > > > > > > done
> > > > > > > on
> > > > > > > a CPU core
> > > > > > > -			 * close to it.
> > > > > > > -			 */
> > > > > > > -			queue_work_node(bdev->pool.nid,
> > > > > > > bdev-
> > > > > > > > wq,
> > > > > > > &bo->delayed_delete);
> > > > > > > -			return;
> > > > > > > -		}
> > > > > > > +		INIT_WORK(&bo->delayed_delete,
> > > > > > > ttm_bo_delayed_delete);
> > > > > > >  
> > > > > > > +		/* Schedule the worker on the closest NUMA
> > > > > > > node.
> > > > > > > This
> > > > > > > +		 * improves performance since system memory
> > > > > > > might be
> > > > > > > +		 * cleared on free and that is best done on
> > > > > > > a
> > > > > > > CPU
> > > > > > > core
> > > > > > > +		 * close to it.
> > > > > > > +		 */
> > > > > > > +		queue_work_node(bdev->pool.nid, bdev->wq,
> > > > > > > &bo-
> > > > > > > > delayed_delete);
> > > > > > > +	} else {
> > > > > > >  		ttm_bo_cleanup_memtype_use(bo);
> > > > > > >  		dma_resv_unlock(bo->base.resv);
> > > > > > > -	}
> > > > > > >  
> > > > > > > -	atomic_dec(&ttm_glob.bo_count);
> > > > > > > -	bo->destroy(bo);
> > > > > > > -}
> > > > > > > -
> > > > > > > -/* TODO: remove! */
> > > > > > > -void ttm_bo_put(struct ttm_buffer_object *bo)
> > > > > > > -{
> > > > > > > -	kref_put(&bo->kref, ttm_bo_release);
> > > > > > > -}
> > > > > > > -
> > > > > > > -void ttm_bo_fini(struct ttm_buffer_object *bo)
> > > > > > > -{
> > > > > > > -	ttm_bo_put(bo);
> > > > > > > +		atomic_dec(&ttm_glob.bo_count);
> > > > > > > +		bo->destroy(bo);
> > > > > > > +	}
> > > > > > >  }
> > > > > > >  EXPORT_SYMBOL(ttm_bo_fini);
> > > > > > >  
> > > > > > > @@ -470,7 +466,7 @@ int ttm_bo_evict_first(struct ttm_device
> > > > > > > *bdev,
> > > > > > > struct ttm_resource_manager *man
> > > > > > >  	if (!bo->resource || bo->resource->mem_type !=
> > > > > > > mem_type)
> > > > > > >  		goto out_bo_moved;
> > > > > > >  
> > > > > > > -	if (bo->deleted) {
> > > > > > > +	if (ttm_bo_is_zombie(bo)) {
> > > > > > >  		ret = ttm_bo_wait_ctx(bo, ctx);
> > > > > > >  		if (!ret)
> > > > > > >  			ttm_bo_cleanup_memtype_use(bo);
> > > > > > > @@ -524,7 +520,7 @@ static s64 ttm_bo_evict_cb(struct
> > > > > > > ttm_lru_walk
> > > > > > > *walk, struct ttm_buffer_object *
> > > > > > >  	if (bo->pin_count || !bo->bdev->funcs-
> > > > > > > > eviction_valuable(bo,
> > > > > > > evict_walk->place))
> > > > > > >  		return 0;
> > > > > > >  
> > > > > > > -	if (bo->deleted) {
> > > > > > > +	if (ttm_bo_is_zombie(bo)) {
> > > > > > >  		lret = ttm_bo_wait_ctx(bo, walk->arg.ctx);
> > > > > > >  		if (!lret)
> > > > > > >  			ttm_bo_cleanup_memtype_use(bo);
> > > > > > > @@ -624,7 +620,6 @@ static int ttm_bo_evict_alloc(struct
> > > > > > > ttm_device
> > > > > > > *bdev,
> > > > > > >  void ttm_bo_pin(struct ttm_buffer_object *bo)
> > > > > > >  {
> > > > > > >  	dma_resv_assert_held(bo->base.resv);
> > > > > > > -	WARN_ON_ONCE(!kref_read(&bo->kref));
> > > > > > >  	spin_lock(&bo->bdev->lru_lock);
> > > > > > >  	if (bo->resource)
> > > > > > >  		ttm_resource_del_bulk_move(bo->resource,
> > > > > > > bo);
> > > > > > > @@ -643,7 +638,6 @@ EXPORT_SYMBOL(ttm_bo_pin);
> > > > > > >  void ttm_bo_unpin(struct ttm_buffer_object *bo)
> > > > > > >  {
> > > > > > >  	dma_resv_assert_held(bo->base.resv);
> > > > > > > -	WARN_ON_ONCE(!kref_read(&bo->kref));
> > > > > > >  	if (WARN_ON_ONCE(!bo->pin_count))
> > > > > > >  		return;
> > > > > > >  
> > > > > > > @@ -934,7 +928,6 @@ int ttm_bo_init_reserved(struct
> > > > > > > ttm_device
> > > > > > > *bdev,
> > > > > > > struct ttm_buffer_object *bo,
> > > > > > >  {
> > > > > > >  	int ret;
> > > > > > >  
> > > > > > > -	kref_init(&bo->kref);
> > > > > > >  	bo->bdev = bdev;
> > > > > > >  	bo->type = type;
> > > > > > >  	bo->page_alignment = alignment;
> > > > > > > @@ -1131,8 +1124,8 @@ ttm_bo_swapout_cb(struct ttm_lru_walk
> > > > > > > *walk,
> > > > > > > struct ttm_buffer_object *bo)
> > > > > > >  		goto out;
> > > > > > >  	}
> > > > > > >  
> > > > > > > -	if (bo->deleted) {
> > > > > > > -		pgoff_t num_pages = tt->num_pages;
> > > > > > > +	if (ttm_bo_is_zombie(bo)) {
> > > > > > > +		pgoff_t num_pages = bo->ttm->num_pages;
> > > > > > >  
> > > > > > >  		ret = ttm_bo_wait_ctx(bo, ctx);
> > > > > > >  		if (ret)
> > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h
> > > > > > > b/drivers/gpu/drm/ttm/ttm_bo_internal.h
> > > > > > > index e0d48eac74b03..ded2a47be0bcb 100644
> > > > > > > --- a/drivers/gpu/drm/ttm/ttm_bo_internal.h
> > > > > > > +++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h
> > > > > > > @@ -27,6 +27,14 @@
> > > > > > >  
> > > > > > >  #include <drm/ttm/ttm_bo.h>
> > > > > > >  
> > > > > > > +static inline void ttm_bo_free(struct drm_gem_object *gobj)
> > > > > > > +{
> > > > > > > +	struct ttm_buffer_object *bo = container_of(gobj,
> > > > > > > typeof(*bo), base);
> > > > > > > +
> > > > > > > +	atomic_dec(&ttm_glob.bo_count);
> > > > > > > +	bo->destroy(bo);
> > > > > > > +}
> > > > > > > +
> > > > > > >  /**
> > > > > > >   * ttm_bo_get - reference a struct ttm_buffer_object
> > > > > > >   *
> > > > > > > @@ -34,7 +42,7 @@
> > > > > > >   */
> > > > > > >  static inline void ttm_bo_get(struct ttm_buffer_object *bo)
> > > > > > >  {
> > > > > > > -	kref_get(&bo->kref);
> > > > > > > +	drm_gem_object_get(&bo->base);
> > > > > > >  }
> > > > > > >  
> > > > > > >  /**
> > > > > > > @@ -50,11 +58,14 @@ static inline void ttm_bo_get(struct
> > > > > > > ttm_buffer_object *bo)
> > > > > > >  static inline __must_check struct ttm_buffer_object *
> > > > > > >  ttm_bo_get_unless_zero(struct ttm_buffer_object *bo)
> > > > > > >  {
> > > > > > > -	if (!kref_get_unless_zero(&bo->kref))
> > > > > > > +	if (!kref_get_unless_zero(&bo->base.refcount))
> > > > > > >  		return NULL;
> > > > > > >  	return bo;
> > > > > > >  }
> > > > > > >  
> > > > > > > -void ttm_bo_put(struct ttm_buffer_object *bo);
> > > > > > > +static inline void ttm_bo_put(struct ttm_buffer_object *bo)
> > > > > > > +{
> > > > > > > +	drm_gem_object_put(&bo->base);
> > > > > > > +}
> > > > > > >  
> > > > > > >  #endif
> > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > > > > > b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > > > > > index 3e3c201a02226..7ed085adf1c9b 100644
> > > > > > > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > > > > > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > > > > > @@ -41,6 +41,18 @@
> > > > > > >  
> > > > > > >  #include "ttm_bo_internal.h"
> > > > > > >  
> > > > > > > +static void ttm_transfer_object_free(struct drm_gem_object
> > > > > > > *obj)
> > > > > > > +{
> > > > > > > +	struct ttm_buffer_object *bo =
> > > > > > > +		container_of(obj, struct ttm_buffer_object,
> > > > > > > base);
> > > > > > > +
> > > > > > > +	ttm_bo_fini(bo);
> > > > > > > +}
> > > > > > > +
> > > > > > > +const struct drm_gem_object_funcs ttm_transfer_object_funcs
> > > > > > > = {
> > > > > > > +	.free = ttm_transfer_object_free,
> > > > > > > +};
> > > > > > > +
> > > > > > >  struct ttm_transfer_obj {
> > > > > > >  	struct ttm_buffer_object base;
> > > > > > >  	struct ttm_buffer_object *bo;
> > > > > > > @@ -247,7 +259,8 @@ static int
> > > > > > > ttm_buffer_object_transfer(struct
> > > > > > > ttm_buffer_object *bo,
> > > > > > >  	atomic_inc(&ttm_glob.bo_count);
> > > > > > >  	drm_vma_node_reset(&fbo->base.base.vma_node);
> > > > > > >  
> > > > > > > -	kref_init(&fbo->base.kref);
> > > > > > > +	kref_init(&fbo->base.base.refcount);
> > > > > > > +	fbo->base.base.funcs = &ttm_transfer_object_funcs;
> > > > > > >  	fbo->base.destroy = &ttm_transfered_destroy;
> > > > > > >  	fbo->base.pin_count = 0;
> > > > > > >  	if (bo->type != ttm_bo_type_sg)
> > > > > > > diff --git a/drivers/gpu/drm/xe/xe_bo.c
> > > > > > > b/drivers/gpu/drm/xe/xe_bo.c
> > > > > > > index 85e6d9a0f575b..5843f850339c7 100644
> > > > > > > --- a/drivers/gpu/drm/xe/xe_bo.c
> > > > > > > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > > > > > > @@ -1651,7 +1651,7 @@ static bool
> > > > > > > xe_ttm_bo_lock_in_destructor(struct
> > > > > > > ttm_buffer_object *ttm_bo)
> > > > > > >  	struct xe_device *xe = ttm_to_xe_device(ttm_bo-
> > > > > > > >bdev);
> > > > > > >  	bool locked;
> > > > > > >  
> > > > > > > -	xe_assert(xe, !kref_read(&ttm_bo->kref));
> > > > > > > +	xe_assert(xe, !kref_read(&ttm_bo->base.refcount));
> > > > > > >  
> > > > > > >  	/*
> > > > > > >  	 * We can typically only race with TTM trylocking
> > > > > > > under
> > > > > > > the
> > > > > > > diff --git a/include/drm/ttm/ttm_bo.h
> > > > > > > b/include/drm/ttm/ttm_bo.h
> > > > > > > index 8310bc3d55f90..1eae9eea5ff32 100644
> > > > > > > --- a/include/drm/ttm/ttm_bo.h
> > > > > > > +++ b/include/drm/ttm/ttm_bo.h
> > > > > > > @@ -78,11 +78,8 @@ enum ttm_bo_type {
> > > > > > >   * @type: The bo type.
> > > > > > >   * @page_alignment: Page alignment.
> > > > > > >   * @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.
> > > > > > >   * @resource: structure describing current placement.
> > > > > > >   * @ttm: TTM structure holding system pages.
> > > > > > > - * @deleted: True if the object is only a zombie and already
> > > > > > > deleted.
> > > > > > >   * @bulk_move: The bulk move object.
> > > > > > >   * @priority: Priority for LRU, BOs with lower priority are
> > > > > > > evicted
> > > > > > > first.
> > > > > > >   * @pin_count: Pin count.
> > > > > > > @@ -109,17 +106,11 @@ struct ttm_buffer_object {
> > > > > > >  	uint32_t page_alignment;
> > > > > > >  	void (*destroy) (struct ttm_buffer_object *);
> > > > > > >  
> > > > > > > -	/*
> > > > > > > -	* Members not needing protection.
> > > > > > > -	*/
> > > > > > > -	struct kref kref;
> > > > > > > -
> > > > > > >  	/*
> > > > > > >  	 * Members protected by the bo::resv::reserved lock.
> > > > > > >  	 */
> > > > > > >  	struct ttm_resource *resource;
> > > > > > >  	struct ttm_tt *ttm;
> > > > > > > -	bool deleted;
> > > > > > >  	struct ttm_lru_bulk_move *bulk_move;
> > > > > > >  	unsigned priority;
> > > > > > >  	unsigned pin_count;

  reply	other threads:[~2026-07-06 18:06 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-03 16:31 [PATCH 00/10] Use drm_exec to lock TTM buffers, respin Natalie Vock
2026-07-03 16:31 ` [PATCH 01/10] drm/exec: Add helper to bypass IGNORE_DUPLICATES flag Natalie Vock
2026-07-06  8:43   ` Christian König
2026-07-03 16:31 ` [PATCH 02/10] drm/ttm: replace TTMs refcount with the DRM refcount v4 Natalie Vock
2026-07-06 13:14   ` Thomas Hellström
2026-07-06 14:49     ` Christian König
2026-07-06 17:01       ` Thomas Hellström
2026-07-06 17:51         ` Matthew Brost
2026-07-06 17:53           ` Thomas Hellström
2026-07-06 18:03             ` Matthew Brost
2026-07-06 18:05               ` Matthew Brost [this message]
2026-07-06 18:03           ` Thomas Hellström
2026-07-06 18:23         ` Christian König
2026-07-06 22:26           ` Dave Airlie
2026-07-03 16:31 ` [PATCH 03/10] drm/ttm: remove ttm_lru_walk_ops Natalie Vock
2026-07-06 12:34   ` Thomas Hellström
2026-07-06 13:05     ` Christian König
2026-07-06 16:31       ` Matthew Brost
2026-07-06 13:08     ` Natalie Vock
2026-07-03 16:31 ` [PATCH 04/10] drm/ttm: grab BO reference before locking it Natalie Vock
2026-07-03 16:31 ` [PATCH 05/10] drm/ttm: switch to ttm_bo_lru_for_each_reserved_guarded for swapout Natalie Vock
2026-07-03 16:31 ` [PATCH 06/10] drm/ttm: move zombie handling into ttm_bo_evict Natalie Vock
2026-07-03 16:31 ` [PATCH 07/10] drm/ttm: use ttm_bo_lru_for_each_reserved_guarded in evict_all Natalie Vock
2026-07-03 16:31 ` [PATCH 08/10] drm/xe: remove workaround for TTM internals Natalie Vock
2026-07-03 16:31 ` [PATCH 09/10] drm/ttm: support using drm_exec during eviction v4 Natalie Vock
2026-07-03 16:31 ` [PATCH 10/10] drm/amdgpu: use drm_exec during BO validation Natalie Vock

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akvugeeF5zfAauf3@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=matthew.auld@intel.com \
    --cc=mripard@kernel.org \
    --cc=natalie.vock@gmx.de \
    --cc=ray.huang@amd.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=simona@ffwll.ch \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=tursulin@ursulin.net \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox