[Intel-gfx] [PATCH] drm/i915: Prune the reservation shared fence array

[Chris Wilson]

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...

It's also a bit meh:


diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index ed238961e1bf..4dcbdbba0ed1 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -129,6 +129,35 @@ reservation_object_fini(struct reservation_object *obj)
 }
 
 /**
+ * reservation_object_lock - lock the reservation object
+ * @obj: the reservation object
+ * @ctx: the locking context
+ *
+ * Locks the reservation object for exclusive access and modification.
+ * As the reservation object may be locked by multiple parties in an
+ * undefined order, a #ww_acquire_ctx is passed to unwind if a cycle
+ * is detected. See ww_mutex_lock() and ww_acquire_init()
+ */
+static inline int
+reservation_object_lock(struct reservation_object *obj,
+                       struct ww_acquire_ctx *ctx)
+{
+       return ww_mutex_lock(obj->lock, ctx);
+}
:...skipping...
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index ed238961e1bf..4dcbdbba0ed1 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -129,6 +129,35 @@ reservation_object_fini(struct reservation_object *obj)
 }
 
 /**
+ * reservation_object_lock - lock the reservation object
+ * @obj: the reservation object
+ * @ctx: the locking context
+ *
+ * Locks the reservation object for exclusive access and modification.
+ * As the reservation object may be locked by multiple parties in an
+ * undefined order, a #ww_acquire_ctx is passed to unwind if a cycle
+ * is detected. See ww_mutex_lock() and ww_acquire_init()
+ */
+static inline int
+reservation_object_lock(struct reservation_object *obj,
+                       struct ww_acquire_ctx *ctx)
+{
+       return ww_mutex_lock(obj->lock, ctx);
+}
+
+/**
+ * reservation_object_unlock - unlock the reservation object
+ * @obj: the reservation object
+ *
+ * Unlocks the reservation object following exclusive access.
+ */
+static inline void
+reservation_object_unlock(struct reservation_object *obj)
+{
+       ww_mutex_unlock(obj->lock);
+}
+
+/**
  * reservation_object_get_excl - get the reservation object's
  * exclusive fence, with update-side lock held
  * @obj: the reservation object


Nothing much is gained over using ww_mutex_lock(). Especially in the more
complicated multiple lock scenarios.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre