| Message ID | 1463922152-2709-3-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22/05/16 14:02, Chris Wilson wrote: > Markup the functions that require the caller to hold struct_mutex with > lockdep_assert_held(). In the hopefully not-too-distant future we will > split the struct_mutex up, and in doing so we need to be sure that we > know what it protects - here the lockdep annotations are invaluable. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_context.c | 16 ++++++++++++++-- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8380102bbdd8..48a222ddd8b3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3429,6 +3429,7 @@ static inline void i915_gem_context_reference(struct i915_gem_context *ctx) > > static inline void i915_gem_context_unreference(struct i915_gem_context *ctx) > { > + lockdep_assert_held(&ctx->i915->dev->struct_mutex); > kref_put(&ctx->ref, i915_gem_context_free); > } > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 8484da26b5d4..af747f14522e 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -153,6 +153,7 @@ void i915_gem_context_free(struct kref *ctx_ref) > { > struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref); > > + lockdep_assert_held(&ctx->i915->dev->struct_mutex); > trace_i915_context_free(ctx); > > if (i915.enable_execlists) > @@ -181,6 +182,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size) > struct drm_i915_gem_object *obj; > int ret; > > + lockdep_assert_held(&dev->struct_mutex); > + > obj = i915_gem_object_create(dev, size); > if (IS_ERR(obj)) > return obj; > @@ -368,6 +371,8 @@ void i915_gem_context_reset(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > + lockdep_assert_held(&dev->struct_mutex); > + > if (i915.enable_execlists) { > struct i915_gem_context *ctx; > > @@ -433,6 +438,8 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv) > { > struct intel_engine_cs *engine; > > + lockdep_assert_held(&dev_priv->dev->struct_mutex); > + > for_each_engine(engine, dev_priv) { > if (engine->last_context == NULL) > continue; > @@ -451,6 +458,8 @@ void i915_gem_context_fini(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct i915_gem_context *dctx = dev_priv->kernel_context; > > + lockdep_assert_held(&dev->struct_mutex); > + > if (dctx->legacy_hw_ctx.rcs_state) > i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state); > > @@ -491,6 +500,8 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file) > { > struct drm_i915_file_private *file_priv = file->driver_priv; > > + lockdep_assert_held(&dev->struct_mutex); > + > idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL); > idr_destroy(&file_priv->context_idr); > } > @@ -500,6 +511,8 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id) > { > struct i915_gem_context *ctx; > > + lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex); > + > ctx = idr_find(&file_priv->context_idr, id); > if (!ctx) > return ERR_PTR(-ENOENT); > @@ -852,10 +865,9 @@ unpin_out: > int i915_switch_context(struct drm_i915_gem_request *req) > { > struct intel_engine_cs *engine = req->engine; > - struct drm_i915_private *dev_priv = req->i915; > > WARN_ON(i915.enable_execlists); > - WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); > + lockdep_assert_held(&req->i915->dev->struct_mutex); > > if (engine->id != RCS || > req->ctx->legacy_hw_ctx.rcs_state == NULL) { > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8380102bbdd8..48a222ddd8b3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3429,6 +3429,7 @@ static inline void i915_gem_context_reference(struct i915_gem_context *ctx) static inline void i915_gem_context_unreference(struct i915_gem_context *ctx) { + lockdep_assert_held(&ctx->i915->dev->struct_mutex); kref_put(&ctx->ref, i915_gem_context_free); } diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 8484da26b5d4..af747f14522e 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -153,6 +153,7 @@ void i915_gem_context_free(struct kref *ctx_ref) { struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref); + lockdep_assert_held(&ctx->i915->dev->struct_mutex); trace_i915_context_free(ctx); if (i915.enable_execlists) @@ -181,6 +182,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size) struct drm_i915_gem_object *obj; int ret; + lockdep_assert_held(&dev->struct_mutex); + obj = i915_gem_object_create(dev, size); if (IS_ERR(obj)) return obj; @@ -368,6 +371,8 @@ void i915_gem_context_reset(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + lockdep_assert_held(&dev->struct_mutex); + if (i915.enable_execlists) { struct i915_gem_context *ctx; @@ -433,6 +438,8 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv) { struct intel_engine_cs *engine; + lockdep_assert_held(&dev_priv->dev->struct_mutex); + for_each_engine(engine, dev_priv) { if (engine->last_context == NULL) continue; @@ -451,6 +458,8 @@ void i915_gem_context_fini(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct i915_gem_context *dctx = dev_priv->kernel_context; + lockdep_assert_held(&dev->struct_mutex); + if (dctx->legacy_hw_ctx.rcs_state) i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state); @@ -491,6 +500,8 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file) { struct drm_i915_file_private *file_priv = file->driver_priv; + lockdep_assert_held(&dev->struct_mutex); + idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL); idr_destroy(&file_priv->context_idr); } @@ -500,6 +511,8 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id) { struct i915_gem_context *ctx; + lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex); + ctx = idr_find(&file_priv->context_idr, id); if (!ctx) return ERR_PTR(-ENOENT); @@ -852,10 +865,9 @@ unpin_out: int i915_switch_context(struct drm_i915_gem_request *req) { struct intel_engine_cs *engine = req->engine; - struct drm_i915_private *dev_priv = req->i915; WARN_ON(i915.enable_execlists); - WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); + lockdep_assert_held(&req->i915->dev->struct_mutex); if (engine->id != RCS || req->ctx->legacy_hw_ctx.rcs_state == NULL) {
Markup the functions that require the caller to hold struct_mutex with lockdep_assert_held(). In the hopefully not-too-distant future we will split the struct_mutex up, and in doing so we need to be sure that we know what it protects - here the lockdep annotations are invaluable. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_context.c | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-)