[Intel-gfx] [PATCH] drm/i915: Use lockdep_pin_lock() over the construction of the request

[Tvrtko Ursulin]

On 03/04/2019 09:21, Chris Wilson wrote:
> During request construction, we take the timeline->mutex to ensure
> exclusive access to the ringbuffer (for command emission) and the
> timeline itself (for command ordering). The timeline->mutex should not
> be dropped by callers until we release it in i915_request_add().
> 
> lockdep provides a pin/unpin lock facility to detect accidental unlocks
> inside critical sections, so put it to use for request construction.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_request.c |  5 +++++
>   drivers/gpu/drm/i915/i915_request.h | 10 ++++++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 82094b9f5ba7..7f8a4eba98b8 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -751,7 +751,10 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
>   	rq->infix = rq->ring->emit; /* end of header; start of user payload */
>   
>   	/* Check that we didn't interrupt ourselves with a new request */
> +	lockdep_assert_held(&rq->timeline->mutex);
>   	GEM_BUG_ON(rq->timeline->seqno != rq->fence.seqno);
> +	rq->cookie = lockdep_pin_lock(&rq->timeline->mutex);
> +
>   	return rq;
>   
>   err_unwind:
> @@ -1070,6 +1073,8 @@ void i915_request_add(struct i915_request *request)
>   		  engine->name, request->fence.context, request->fence.seqno);
>   
>   	lockdep_assert_held(&request->timeline->mutex);
> +	lockdep_unpin_lock(&request->timeline->mutex, request->cookie);
> +
>   	trace_i915_request_add(request);
>   
>   	/*
> diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
> index cd6c130964cd..875be6f71412 100644
> --- a/drivers/gpu/drm/i915/i915_request.h
> +++ b/drivers/gpu/drm/i915/i915_request.h
> @@ -26,6 +26,7 @@
>   #define I915_REQUEST_H
>   
>   #include <linux/dma-fence.h>
> +#include <linux/lockdep.h>
>   
>   #include "i915_gem.h"
>   #include "i915_scheduler.h"
> @@ -120,6 +121,15 @@ struct i915_request {
>   	 */
>   	unsigned long rcustate;
>   
> +	/*
> +	 * We pin the timeline->mutex while constructing the request to
> +	 * ensure that no caller accidentally drops it during construction.
> +	 * The timeline->mutex must be held to ensure that only this caller
> +	 * can use the ring and manipulate the associated timeline during
> +	 * construction.
> +	 */
> +	struct pin_cookie cookie;
> +
>   	/*
>   	 * Fences for the various phases in the request's lifetime.
>   	 *
> 

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

Regards,

Tvrtko