The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: "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>,
	"Matthew Brost" <matthew.brost@intel.com>,
	"Alex Deucher" <alexander.deucher@amd.com>
Cc: 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: Tue, 07 Jul 2026 11:53:54 +0200	[thread overview]
Message-ID: <a5e1afa24b975a1fcf17949875d4a0e0772dcc70.camel@linux.intel.com> (raw)
In-Reply-To: <a67cc4c5-7545-4f8e-aa89-e90b3bee523a@amd.com>

On Tue, 2026-07-07 at 10:48 +0200, Christian König wrote:
> On 7/7/26 08:56, Thomas Hellström wrote:
> > On Mon, 2026-07-06 at 20:23 +0200, Christian König wrote:
> > > On 7/6/26 19:01, Thomas Hellström wrote:
> ...
> > > 
> > > Well, I clearly rejected the idea to wait for the delayed delete
> > > worker because that can cause deadlocks no matter how we do it.
> > > 
> > > The only possibility I see is to grab a reference on the next
> > > busy
> > > fence and wait for that to signal.
> > > 
> > > But that approach still leaves a window open where the cleanup
> > > worker
> > > has not yet cleaned up the resource before we try to allocate a
> > > new
> > > one.
> > > 
> > > The only two possibilities I see to close that window are:
> > > 1. Resurrect the GEM object.
> > > 2. Keep a reference to the GEM object while it is on the LRU.
> > > 
> > > I would really like to do #2 instead, but that requires that we
> > > distinct between GEM object referenze and backing store
> > > reference.
> > > 
> > > The GEM object does have some kind of backing store reference
> > > with
> > > the handle_count field, but that unfortunately doesn't have the
> > > right
> > > semantic (e.g. only goes from 0->1 when you actually create a
> > > handle
> > > etc..).
> > 
> > I think putting a struct completion + refcount on the TTM resource
> > would work. Or a much simpler variant, wait for anyone to release
> > something on the resource manager in question, (although the
> > simpler
> > variant would be harder to debug if a deadlock occured).
> > 
> > Both of these would probably need to come with lockdep annotation
> > so
> > that we could annotate the critical sections from ttm_bo_fini() to
> > resource_free, similar to the dma-fence signalling annotation.
> > 
> > At this point, following that previous discussion we know that
> > imported
> > dma-bufs are an exception because we grab the shared dma-resv to
> > release the attachment. So until we add a way to avoid that in the
> > dma-
> > buf code, we need to wake / signal completion before we grab that
> > lock.
> > Not a big deal because imported bos will not matter for local
> > reclaim
> > anyway.
> 
> The problem is that locking the dma_resv object for release is
> mandatory not only for proper DMA-buf import handling but for tons of
> other use cases as well. In other words DMA-buf imports, page tables,
> per process buffers who use the same dma_resv object as the root PD
> etc...
> 
> By now we I don't think we can't avoid locking the dma_resv object in
> the delete path, so a completeion event or any other method which
> blocks for resource cleanup without deadlock detection won't work as
> far as I can see.

So in the TTM code itself we have only trylocks, so that works. For the
other ones, trylocking the individualized resv in release_notify() or
delete_mem_notify() should cover all / most cases except the dma-buf
one? xe is doing exactly that. For imported dma-bufs it might even
possible to release the resource before cleaning up the bo attachment.

One problem remains for shrinkers, though, The memory allocation in
individualization becomes a problem. We'd either need to annotate those
allocations with PF_MEMALLOC flags or skip shrinking zombies.

> 
> The only way forward seems to be to lock the ww_mutex with deadlock
> avoidance handling and that in turn needs to make sure that the
> ww_mutex doesn't go away until we eventually have bubbled up the
> error to the driver and the drm_exec object had the chance to lock
> the offending ww_mutex before anything else.
> 
> > > > > > 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?
> > > 
> > > Of course could the drm_exec object work with different reference
> > > counters, but as far as I can see that just complicates the
> > > situation
> > > and again creates a TTM specific solutions which I clearly want
> > > to
> > > avoid.
> > > 
> > > I would rather go with the resurrection approach instead, that
> > > has
> > > less potential for problem I think.
> > > 
> > > Alaternatively we could re-work the whole GEM refcount/GEM handle
> > > count/TTM refcount mess, but that has even more potential to
> > > break.
> > 
> > I think looking forward to a "final" solution we're happy with, we
> > would have removed the TTM refcount and found a way to deal with
> > zombie
> > resources by finding something to wait for.
> 
> We should probably nail down the "final" solution now, it's just that
> way to many ugly workarounds arounds have been proposed so far and I
> completely agree that resuccertion of zombies is just another one.
> 
> Just a random idea we haven't discussed so far: What about making the
> ww_mutex ref counted?
> 
> There are basically just three users in the kernel for the ww_mutex,
> so that shouldn't be to much trouble.
> 
> > I'm also convinced that we want apps with shared bos to fully
> > participate so that a process trying to validate / map a shared bo
> > doesn't get killed because a trylock-driven eviction failed.
> 
> I might not fully understand that, but if you mean that applications
> should be able to use all resources even if those resources has just
> recently been released than I'm all with you.
> 
> > 
> > Since it's a layer violation to pass a drm_exec in an augmented
> > dma-buf
> > map we will be needing something at the dma-buf level to handle
> > that,
> > and given that, it also makes sense to base the eviction mechanism
> > on
> > it.
> 
> Well I disagree. The DMA-buf invalidation notification intentionally
> doesn't have a ww_acquire_ctx.
> 
> Background is that when this is called exporters have often already
> passed the point of no return and the importers just *must* act on
> the request and can't say 'Na, sorry I can't take the locks I need'.

I'm not referring to the invalidation. I'm thinking of the following
sequence:

xe_bo_validate()       // Importer. Finds it's an evicted dma-buf.
dma_buf_map();
xe_bo_validate();      // Exporter. Short on memory.
ttm_bo_evict();        // Exporter wants to evict but lacks a ticket
		       // and is limited to trylock and loses.

The remedy is to optionally pass a ticket in an augmented
dma_buf_map(), but then it can't be a drm_exec. dma_buf invalidation
stays as it is.

> 
> > The ability to reuse the TTM refcount for now would just be a way
> > to
> > separate out the zombie handling from this series until we can do
> > it
> > properly. After all this series is about enabling sleeping ww
> > locks.
> 
> Well the problem is that I don't see how it should work with the TTM
> refcount.
> 
> This basically means that for each lock operation has an ops table
> with put/get, in that case it would just easier to make the ww_mutex
> ref counted.

Each object type embedding a ww_mutex needs an ops table pointed to by
the lock operation. But get() is not needed, only put(). (See the RFC),

If we refcount the ww mutex, instead it would need a separate
destructor for each object type. We'd save a pointer chase on put but
need extra care on object creation and destruction, so I'm not sure
it's easier.

/Thomas


> 
> Regards,
> Christian.
> 
> > 
> > Thanks,
> > Thomas
> > 
> > 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > 
> > > > 
> > > > 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_objec
> > > > > > > t_ma
> > > > > > > ke_s
> > > > > > > hrinkable(obj);
> > > > > > > -				else
> > > > > > > -
> > > > > > > 					__i915_gem_objec
> > > > > > > t_ma
> > > > > > > ke_p
> > > > > > > urgeable(obj);
> > > > > > > -			} else {
> > > > > > > -
> > > > > > > 				i915_gem_object_make_uns
> > > > > > > hrin
> > > > > > > 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_s
> > > > > > > hrin
> > > > > > > kabl
> > > > > > > e(ob
> > > > > > > j);
> > > > > > > +			else
> > > > > > > +				__i915_gem_object_make_p
> > > > > > > urge
> > > > > > > able
> > > > > > > (obj
> > > > > > > );
> > > > > > > +		} else {
> > > > > > > +			i915_gem_object_make_unshrinkabl
> > > > > > > e(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-07  9:54 UTC|newest]

Thread overview: 44+ 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-07 12:09   ` Thomas Hellström
2026-07-07 12:28     ` Christian König
2026-07-07 12:41       ` Thomas Hellström
2026-07-07 12:54         ` Christian König
2026-07-07 13:12           ` Natalie Vock
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
2026-07-07  6:30                 ` Thomas Hellström
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-07  6:56           ` Thomas Hellström
2026-07-07  7:35             ` Natalie Vock
2026-07-07  8:48             ` Christian König
2026-07-07  9:53               ` Thomas Hellström [this message]
2026-07-07 12:52                 ` Christian König
2026-07-07 14:07                   ` Thomas Hellström
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-07 12:12   ` Thomas Hellström
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-07 12:17   ` Thomas Hellström
2026-07-03 16:31 ` [PATCH 06/10] drm/ttm: move zombie handling into ttm_bo_evict Natalie Vock
2026-07-07 12:24   ` Thomas Hellström
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-07 12:35   ` Thomas Hellström
2026-07-03 16:31 ` [PATCH 08/10] drm/xe: remove workaround for TTM internals Natalie Vock
2026-07-07 11:52   ` Thomas Hellström
2026-07-03 16:31 ` [PATCH 09/10] drm/ttm: support using drm_exec during eviction v4 Natalie Vock
2026-07-07 12:48   ` Thomas Hellström
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=a5e1afa24b975a1fcf17949875d4a0e0772dcc70.camel@linux.intel.com \
    --to=thomas.hellstrom@linux.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=matthew.brost@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=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