Re: [PATCH 03/12] drm/i915: Apply lockdep annotations to i915_gem_context.c

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx