[Intel-gfx] [PATCH v2] drm/i915: Stop storing ctx->user_handle

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Wed Mar 20 10:43:13 UTC 2019


On 19/03/2019 12:58, Chris Wilson wrote:
> The user_handle need only be known by userspace for it to lookup the
> context via the idr; internally we have no use for it.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c           |  5 ++--
>   drivers/gpu/drm/i915/i915_gem_context.c       | 23 ++++++++-----------
>   drivers/gpu/drm/i915/i915_gem_context.h       |  5 ----
>   drivers/gpu/drm/i915/i915_gem_context_types.h |  9 --------
>   drivers/gpu/drm/i915/i915_gpu_error.c         | 11 ++++-----
>   drivers/gpu/drm/i915/i915_gpu_error.h         |  1 -
>   drivers/gpu/drm/i915/selftests/mock_context.c |  2 +-
>   7 files changed, 16 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f4a07190a0e8..7970770f23a9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -409,9 +409,8 @@ static void print_context_stats(struct seq_file *m,
>   
>   			rcu_read_lock();
>   			task = pid_task(ctx->pid ?: file->pid, PIDTYPE_PID);
> -			snprintf(name, sizeof(name), "%s/%d",
> -				 task ? task->comm : "<unknown>",
> -				 ctx->user_handle);
> +			snprintf(name, sizeof(name), "%s",
> +				 task ? task->comm : "<unknown>");
>   			rcu_read_unlock();
>   
>   			print_file_stats(m, name, stats);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 799684d05704..95c5103e15a5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -602,20 +602,15 @@ static int gem_context_register(struct i915_gem_context *ctx,
>   
>   	/* And finally expose ourselves to userspace via the idr */
>   	mutex_lock(&fpriv->context_idr_lock);
> -	ret = idr_alloc(&fpriv->context_idr, ctx,
> -			DEFAULT_CONTEXT_HANDLE, 0, GFP_KERNEL);
> -	if (ret >= 0)
> -		ctx->user_handle = ret;
> +	ret = idr_alloc(&fpriv->context_idr, ctx, 0, 0, GFP_KERNEL);
>   	mutex_unlock(&fpriv->context_idr_lock);
> -	if (ret < 0)
> -		goto err_name;
> -
> -	return 0;
> +	if (ret >= 0)
> +		goto out;
>   
> -err_name:
>   	kfree(fetch_and_zero(&ctx->name));
>   err_pid:
>   	put_pid(fetch_and_zero(&ctx->pid));
> +out:
>   	return ret;
>   }
>   
> @@ -638,11 +633,11 @@ int i915_gem_context_open(struct drm_i915_private *i915,
>   	}
>   
>   	err = gem_context_register(ctx, file_priv);
> -	if (err)
> +	if (err < 0)
>   		goto err_ctx;
>   
> -	GEM_BUG_ON(ctx->user_handle != DEFAULT_CONTEXT_HANDLE);
>   	GEM_BUG_ON(i915_gem_context_is_kernel(ctx));
> +	GEM_BUG_ON(err > 0);
>   
>   	return 0;
>   
> @@ -852,10 +847,10 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>   		return PTR_ERR(ctx);
>   
>   	ret = gem_context_register(ctx, file_priv);
> -	if (ret)
> +	if (ret < 0)
>   		goto err_ctx;
>   
> -	args->ctx_id = ctx->user_handle;
> +	args->ctx_id = ret;
>   	DRM_DEBUG("HW context %d created\n", args->ctx_id);
>   
>   	return 0;
> @@ -877,7 +872,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>   	if (args->pad != 0)
>   		return -EINVAL;
>   
> -	if (args->ctx_id == DEFAULT_CONTEXT_HANDLE)
> +	if (!args->ctx_id)
>   		return -ENOENT;
>   
>   	if (mutex_lock_interruptible(&file_priv->context_idr_lock))
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index 8a1377691d6d..849b2a83c1ec 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -126,11 +126,6 @@ static inline void i915_gem_context_unpin_hw_id(struct i915_gem_context *ctx)
>   	atomic_dec(&ctx->hw_id_pin_count);
>   }
>   
> -static inline bool i915_gem_context_is_default(const struct i915_gem_context *c)
> -{
> -	return c->user_handle == DEFAULT_CONTEXT_HANDLE;
> -}
> -
>   static inline bool i915_gem_context_is_kernel(struct i915_gem_context *ctx)
>   {
>   	return !ctx->file_priv;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context_types.h b/drivers/gpu/drm/i915/i915_gem_context_types.h
> index 2bf19730eaa9..63ae8eb21939 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context_types.h
> @@ -129,15 +129,6 @@ struct i915_gem_context {
>   	struct list_head active_engines;
>   	struct mutex mutex;
>   
> -	/**
> -	 * @user_handle: userspace identifier
> -	 *
> -	 * A unique per-file identifier is generated from
> -	 * &drm_i915_file_private.contexts.
> -	 */
> -	u32 user_handle;
> -#define DEFAULT_CONTEXT_HANDLE 0
> -
>   	struct i915_sched_attr sched;
>   
>   	/** hw_contexts: per-engine logical HW state */
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index e8674347f589..b101f037b61f 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -454,8 +454,8 @@ static void error_print_context(struct drm_i915_error_state_buf *m,
>   				const char *header,
>   				const struct drm_i915_error_context *ctx)
>   {
> -	err_printf(m, "%s%s[%d] user_handle %d hw_id %d, prio %d, guilty %d active %d\n",
> -		   header, ctx->comm, ctx->pid, ctx->handle, ctx->hw_id,
> +	err_printf(m, "%s%s[%d] hw_id %d, prio %d, guilty %d active %d\n",
> +		   header, ctx->comm, ctx->pid, ctx->hw_id,
>   		   ctx->sched_attr.priority, ctx->guilty, ctx->active);
>   }
>   
> @@ -758,11 +758,9 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
>   		if (obj) {
>   			err_puts(m, m->i915->engine[i]->name);
>   			if (ee->context.pid)
> -				err_printf(m, " (submitted by %s [%d], ctx %d [%d])",
> +				err_printf(m, " (submitted by %s [%d])",
>   					   ee->context.comm,
> -					   ee->context.pid,
> -					   ee->context.handle,
> -					   ee->context.hw_id);
> +					   ee->context.pid);
>   			err_printf(m, " --- gtt_offset = 0x%08x %08x\n",
>   				   upper_32_bits(obj->gtt_offset),
>   				   lower_32_bits(obj->gtt_offset));
> @@ -1330,7 +1328,6 @@ static void record_context(struct drm_i915_error_context *e,
>   		rcu_read_unlock();
>   	}
>   
> -	e->handle = ctx->user_handle;
>   	e->hw_id = ctx->hw_id;
>   	e->sched_attr = ctx->sched;
>   	e->guilty = atomic_read(&ctx->guilty_count);
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
> index d011cb90bee1..5dc761e85d9d 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.h
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.h
> @@ -116,7 +116,6 @@ struct i915_gpu_state {
>   		struct drm_i915_error_context {
>   			char comm[TASK_COMM_LEN];
>   			pid_t pid;
> -			u32 handle;
>   			u32 hw_id;
>   			int active;
>   			int guilty;
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
> index 1cc8be732435..d63025dc1d83 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.c
> @@ -98,7 +98,7 @@ live_context(struct drm_i915_private *i915, struct drm_file *file)
>   		return ctx;
>   
>   	err = gem_context_register(ctx, file->driver_priv);
> -	if (err)
> +	if (err < 0)
>   		goto err_ctx;
>   
>   	return ctx;
> 

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

Regards,

Tvrtko


More information about the Intel-gfx mailing list