Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Use a small char buffer inside the i915_gem_context to store the user
> friendly name so that ctx->name has the same lifetime as the RCU
> protected GEM context. That is, e.g. when using print_request() that
> prints the timeline name (ctx->name), the name will not be prematurely
> freed upon the context being closed and the last reference dropped.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Matthew Auld <matthew.auld@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 12 ++----------
> .../gpu/drm/i915/gem/i915_gem_context_types.h | 18 +++++++++---------
> .../gpu/drm/i915/gem/selftests/mock_context.c | 4 +---
> 3 files changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 94b51b13ef17..982770e8163d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -277,9 +277,7 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
> if (ctx->timeline)
> intel_timeline_put(ctx->timeline);
>
> - kfree(ctx->name);
> put_pid(ctx->pid);
> -
> mutex_destroy(&ctx->mutex);
>
> kfree_rcu(ctx, rcu);
> @@ -789,12 +787,8 @@ static int gem_context_register(struct i915_gem_context *ctx,
> mutex_unlock(&ctx->mutex);
>
> ctx->pid = get_task_pid(current, PIDTYPE_PID);
> - ctx->name = kasprintf(GFP_KERNEL, "%s[%d]",
> - current->comm, pid_nr(ctx->pid));
> - if (!ctx->name) {
> - ret = -ENOMEM;
> - goto err_pid;
> - }
> + snprintf(ctx->name, sizeof(ctx->name), "%s[%d]",
> + current->comm, pid_nr(ctx->pid));
>
> /* And finally expose ourselves to userspace via the idr */
> mutex_lock(&fpriv->context_idr_lock);
> @@ -803,8 +797,6 @@ static int gem_context_register(struct i915_gem_context *ctx,
> if (ret >= 0)
> goto out;
>
> - kfree(fetch_and_zero(&ctx->name));
> -err_pid:
> put_pid(fetch_and_zero(&ctx->pid));
> out:
> return ret;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> index 861d7d92fe9f..69df5459c350 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> @@ -100,15 +100,6 @@ struct i915_gem_context {
> */
> struct pid *pid;
>
> - /**
> - * @name: arbitrary name
> - *
> - * A name is constructed for the context from the creator's process
> - * name, pid and user handle in order to uniquely identify the
> - * context in messages.
> - */
> - const char *name;
> -
> /** link: place with &drm_i915_private.context_list */
> struct list_head link;
> struct llist_node free_link;
> @@ -176,6 +167,15 @@ struct i915_gem_context {
> * per vm, which may be one per context or shared with the global GTT)
> */
> struct radix_tree_root handles_vma;
> +
> + /**
> + * @name: arbitrary name, used for user debug
> + *
> + * A name is constructed for the context from the creator's process
> + * name, pid and user handle in order to uniquely identify the
> + * context in messages.
> + */
> + char name[TASK_COMM_LEN + 8];
I consider 24 bytes as quite cheap for robustness and simplicity.
In a playground where we throw pages around like sand.
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> };
>
> #endif /* __I915_GEM_CONTEXT_TYPES_H__ */
> diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> index cdcb006321a7..53e89efb09c0 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> @@ -37,9 +37,7 @@ mock_context(struct drm_i915_private *i915,
> if (name) {
> struct i915_ppgtt *ppgtt;
>
> - ctx->name = kstrdup(name, GFP_KERNEL);
> - if (!ctx->name)
> - goto err_put;
> + strncpy(ctx->name, name, sizeof(ctx->name));
>
> ppgtt = mock_ppgtt(i915, name);
> if (!ppgtt)
> --
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
