Re: [PATCH 5/6] drm/i915: Wrap engine->schedule in RCU locks for set-wedge protection

From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/6] drm/i915: Wrap engine->schedule in RCU locks for set-wedge protection
Date: Fri, 09 Mar 2018 15:49:56 +0200	[thread overview]
Message-ID: <87woylmk8r.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20180307134226.25492-5-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Similar to the staging around handling of engine->submit_request, we
> need to stop adding to the execlists->queue prior to calling
> engine->cancel_requests. cancel_requests will move requests from the
> queue onto the timeline, so if we add a request onto the queue after that
> point, it will be lost.
>
> Fixes: af7a8ffad9c5 ("drm/i915: Use rcu instead of stop_machine in set_wedged")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c     | 13 +++++++------
>  drivers/gpu/drm/i915/i915_request.c |  2 ++
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 30cd07ebcb8e..3b44952e089f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -471,10 +471,11 @@ static void __fence_set_priority(struct dma_fence *fence, int prio)
>  
>  	rq = to_request(fence);
>  	engine = rq->engine;
> -	if (!engine->schedule)
> -		return;
>  
> -	engine->schedule(rq, prio);
> +	rcu_read_lock();
> +	if (engine->schedule)
> +		engine->schedule(rq, prio);
> +	rcu_read_unlock();
>  }
>  
>  static void fence_set_priority(struct dma_fence *fence, int prio)
> @@ -3214,8 +3215,11 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
>  	 */
>  	for_each_engine(engine, i915, id) {
>  		i915_gem_reset_prepare_engine(engine);
> +
>  		engine->submit_request = nop_submit_request;
> +		engine->schedule = NULL;
>  	}
> +	i915->caps.scheduler = 0;
>  
>  	/*
>  	 * Make sure no one is running the old callback before we proceed with
> @@ -3233,11 +3237,8 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
>  		 * start to complete all requests.
>  		 */
>  		engine->submit_request = nop_complete_submit_request;
> -		engine->schedule = NULL;
>  	}
>  
> -	i915->caps.scheduler = 0;
> -
>  	/*
>  	 * Make sure no request can slip through without getting completed by
>  	 * either this call here to intel_engine_init_global_seqno, or the one
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 69b378a323fc..0258449579f8 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1082,8 +1082,10 @@ void __i915_request_add(struct i915_request *request, bool flush_caches)
>  	 * decide whether to preempt the entire chain so that it is ready to
>  	 * run at the earliest possible convenience.
>  	 */
> +	rcu_read_lock();
>  	if (engine->schedule)
>  		engine->schedule(request, request->ctx->priority);
> +	rcu_read_unlock();
>  
>  	local_bh_disable();
>  	i915_sw_fence_commit(&request->submit);
> -- 
> 2.16.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx