| Message ID | 20190403082132.327-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Use lockdep_pin_lock() over the construction of the request | expand |
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@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com> Regards, Tvrtko
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. *
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@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_request.c | 5 +++++ drivers/gpu/drm/i915/i915_request.h | 10 ++++++++++ 2 files changed, 15 insertions(+)