diff mbox

drm/i915: Prune the reservation shared fence array

Message ID 20161114085327.16327-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Nov. 14, 2016, 8:53 a.m. UTC
The shared fence array is not autopruning and may continue to grow as an
object is shared between new timelines. Take the opportunity when we
think the object is idle (we have to confirm that any external fence is
also signaled) to decouple all the fences.

Fixes: d07f0e59b2c7 ("drm/i915: Move GEM activity tracking into a common struct reservation_object")
Fixes: 80b204bce8f2 ("drm/i915: Enable multiple timelines")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_vma.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Joonas Lahtinen Nov. 15, 2016, 2:38 p.m. UTC | #1
On ma, 2016-11-14 at 08:53 +0000, Chris Wilson wrote:
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -53,6 +53,12 @@ i915_vma_retire(struct i915_gem_active *active,
>  	if (--obj->active_count)
>  		return;
>  
> +	/* Prune the shared fence arrays iff completely idle (inc. external) */
> +	ww_mutex_lock(&obj->resv->lock, NULL);
> +	if (reservation_object_test_signaled_rcu(obj->resv, true))
> +		reservation_object_add_excl_fence(obj->resv, NULL);
> +	ww_mutex_unlock(&obj->resv->lock);
> +

This is not the first instance of "resv->lock" usage, but I think we
should not be doing that, and add reservation_* functions instead...

Regards, Joonas
Chris Wilson Nov. 15, 2016, 3:26 p.m. UTC | #2
On Tue, Nov 15, 2016 at 04:38:19PM +0200, Joonas Lahtinen wrote:
> On ma, 2016-11-14 at 08:53 +0000, Chris Wilson wrote:
> > +++ b/drivers/gpu/drm/i915/i915_vma.c
> > @@ -53,6 +53,12 @@ i915_vma_retire(struct i915_gem_active *active,
> >  	if (--obj->active_count)
> >  		return;
> >  
> > +	/* Prune the shared fence arrays iff completely idle (inc. external) */
> > +	ww_mutex_lock(&obj->resv->lock, NULL);
> > +	if (reservation_object_test_signaled_rcu(obj->resv, true))
> > +		reservation_object_add_excl_fence(obj->resv, NULL);
> > +	ww_mutex_unlock(&obj->resv->lock);
> > +
> 
> This is not the first instance of "resv->lock" usage, but I think we
> should not be doing that, and add reservation_* functions instead...

But in the short term wrt to how we consume many, many more megabytes
than we used to in gem_ctx_*....
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 6d456582edf4..44585300fdff 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -53,6 +53,12 @@  i915_vma_retire(struct i915_gem_active *active,
 	if (--obj->active_count)
 		return;
 
+	/* Prune the shared fence arrays iff completely idle (inc. external) */
+	ww_mutex_lock(&obj->resv->lock, NULL);
+	if (reservation_object_test_signaled_rcu(obj->resv, true))
+		reservation_object_add_excl_fence(obj->resv, NULL);
+	ww_mutex_unlock(&obj->resv->lock);
+
 	/* Bump our place on the bound list to keep it roughly in LRU order
 	 * so that we don't steal from recently used but inactive objects
 	 * (unless we are forced to ofc!)