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

2019-04-05 Thread 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 
Cc: Tvrtko Ursulin 
---
  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(>timeline->mutex);
GEM_BUG_ON(rq->timeline->seqno != rq->fence.seqno);
+   rq->cookie = lockdep_pin_lock(>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(>timeline->mutex);

+   lockdep_unpin_lock(>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 

+#include 
  
  #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 

Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2019-04-03 Thread Chris Wilson 
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 
Cc: Tvrtko Ursulin 
---
 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(>timeline->mutex);
GEM_BUG_ON(rq->timeline->seqno != rq->fence.seqno);
+   rq->cookie = lockdep_pin_lock(>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(>timeline->mutex);
+   lockdep_unpin_lock(>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 
+#include 
 
 #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.
 *
-- 
2.20.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx