[Intel-gfx] [PATCH 16/27] drm/i915: Reinstate reservation_object zapping for batch_pool objects

[Tvrtko Ursulin]

On 19/04/2017 10:41, Chris Wilson wrote:
> I removed the zapping of the reservation_object->fence array of shared
> fences prematurely. We don't yet have the code to zap that array when
> retiring the object, and so currently it remains possible to continually
> grow the shared array trapping requests when reusing the batch_pool
> object across many timelines.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: Matthew Auld <matthew.auld at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_batch_pool.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
> index 41aa598c4f3b..414e46e2f072 100644
> --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
> +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
> @@ -114,12 +114,26 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool,
>  	list_for_each_entry(obj, list, batch_pool_link) {
>  		/* The batches are strictly LRU ordered */
>  		if (i915_gem_object_is_active(obj)) {
> -			if (!reservation_object_test_signaled_rcu(obj->resv,
> -								  true))
> +			struct reservation_object *resv = obj->resv;
> +
> +			if (!reservation_object_test_signaled_rcu(resv, true))
>  				break;
>
>  			i915_gem_retire_requests(pool->engine->i915);
>  			GEM_BUG_ON(i915_gem_object_is_active(obj));
> +
> +			/* The object is now idle, clear the array of shared
> +			 * fences before we add a new request. Although, we
> +			 * remain on the same engine, we may be on a different
> +			 * timeline and so may continually grow the array,
> +			 * trapping a reference to all the old fences, rather
> +			 * than replace the existing fence.
> +			 */
> +			if (rcu_access_pointer(resv->fence)) {
> +				reservation_object_lock(resv, NULL);
> +				reservation_object_add_excl_fence(resv, NULL);
> +				reservation_object_unlock(resv);
> +			}
>  		}
>
>  		GEM_BUG_ON(!reservation_object_test_signaled_rcu(obj->resv,
>

Not too familiar with the reservation object stuff but having read some 
kerneldoc it looks correct to me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko