[Intel-gfx] [PATCH 28/33] drm/i915: Move per-request pid from request to ctx
Joonas Lahtinen
joonas.lahtinen at linux.intel.com
Thu Aug 11 12:32:18 UTC 2016
On su, 2016-08-07 at 15:45 +0100, Chris Wilson wrote:
> Since contexts are not currently shared between userspace processes, we
How about future?
Patch itself seems fine. Title could be more like "Move PID tracking
from request to context".
Regards, Joonas
> have an exact correspondence between context creator and guilty batch
> submitter. Therefore we can save some per-batch work by inspecting the
> context->pid upon error instead. Note that we take the context's
> creator's pid rather than the file's pid in order to better track fd
> passed over sockets.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 25 ++++++++++++++++---------
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_gem_context.c | 4 ++++
> drivers/gpu/drm/i915/i915_gem_request.c | 5 -----
> drivers/gpu/drm/i915/i915_gem_request.h | 3 ---
> drivers/gpu/drm/i915/i915_gpu_error.c | 13 ++++++++++---
> 6 files changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5f00d6347905..963c6d28d332 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -460,6 +460,8 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
> print_context_stats(m, dev_priv);
> list_for_each_entry_reverse(file, &dev->filelist, lhead) {
> struct file_stats stats;
> + struct drm_i915_file_private *file_priv = file->driver_priv;
> + struct drm_i915_gem_request *request;
> struct task_struct *task;
>
> memset(&stats, 0, sizeof(stats));
> @@ -473,10 +475,17 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
> * still alive (e.g. get_pid(current) => fork() => exit()).
> * Therefore, we need to protect this ->comm access using RCU.
> */
> + mutex_lock(&dev->struct_mutex);
> + request = list_first_entry_or_null(&file_priv->mm.request_list,
> + struct drm_i915_gem_request,
> + client_list);
> rcu_read_lock();
> - task = pid_task(file->pid, PIDTYPE_PID);
> + task = pid_task(request && request->ctx->pid ?
> + request->ctx->pid : file->pid,
> + PIDTYPE_PID);
> print_file_stats(m, task ? task->comm : "", stats);
> rcu_read_unlock();
> + mutex_unlock(&dev->struct_mutex);
> }
> mutex_unlock(&dev->filelist_mutex);
>
> @@ -657,12 +666,11 @@ static int i915_gem_request_info(struct seq_file *m, void *data)
>
> seq_printf(m, "%s requests: %d\n", engine->name, count);
> list_for_each_entry(req, &engine->request_list, link) {
> + struct pid *pid = req->ctx->pid;
> struct task_struct *task;
>
> rcu_read_lock();
> - task = NULL;
> - if (req->pid)
> - task = pid_task(req->pid, PIDTYPE_PID);
> + task = pid ? pid_task(pid, PIDTYPE_PID) : NULL;
> seq_printf(m, " %x @ %d: %s [%d]\n",
> req->fence.seqno,
> (int) (jiffies - req->emitted_jiffies),
> @@ -1951,18 +1959,17 @@ static int i915_context_status(struct seq_file *m, void *unused)
>
> list_for_each_entry(ctx, &dev_priv->context_list, link) {
> seq_printf(m, "HW context %u ", ctx->hw_id);
> - if (IS_ERR(ctx->file_priv)) {
> - seq_puts(m, "(deleted) ");
> - } else if (ctx->file_priv) {
> - struct pid *pid = ctx->file_priv->file->pid;
> + if (ctx->pid) {
> struct task_struct *task;
>
> - task = get_pid_task(pid, PIDTYPE_PID);
> + task = get_pid_task(ctx->pid, PIDTYPE_PID);
> if (task) {
> seq_printf(m, "(%s [%d]) ",
> task->comm, task->pid);
> put_task_struct(task);
> }
> + } else if (IS_ERR(ctx->file_priv)) {
> + seq_puts(m, "(deleted) ");
> } else {
> seq_puts(m, "(kernel) ");
> }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4023718017a8..e7357656728e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -560,6 +560,7 @@ struct drm_i915_error_state {
>
> struct drm_i915_error_request {
> long jiffies;
> + pid_t pid;
> u32 seqno;
> u32 tail;
> } *requests;
> @@ -880,6 +881,7 @@ struct i915_gem_context {
> struct drm_i915_private *i915;
> struct drm_i915_file_private *file_priv;
> struct i915_hw_ppgtt *ppgtt;
> + struct pid *pid;
>
> struct i915_ctx_hang_stats hang_stats;
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 15eed897b498..c026d591d142 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -158,6 +158,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
> i915_gem_object_put(ce->state->obj);
> }
>
> + put_pid(ctx->pid);
> list_del(&ctx->link);
>
> ida_simple_remove(&ctx->i915->context_hw_ida, ctx->hw_id);
> @@ -311,6 +312,9 @@ __create_hw_context(struct drm_device *dev,
> ret = DEFAULT_CONTEXT_HANDLE;
>
> ctx->file_priv = file_priv;
> + if (file_priv)
> + ctx->pid = get_task_pid(current, PIDTYPE_PID);
> +
> ctx->user_handle = ret;
> /* NB: Mark all slices as needing a remap so that when the context first
> * loads it will restore whatever remap state already exists. If there
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 187c4f9ce8d0..8fdd313248f9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -137,8 +137,6 @@ int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
> list_add_tail(&req->client_list, &file_priv->mm.request_list);
> spin_unlock(&file_priv->mm.lock);
>
> - req->pid = get_pid(task_pid(current));
> -
> return 0;
> }
>
> @@ -154,9 +152,6 @@ i915_gem_request_remove_from_client(struct drm_i915_gem_request *request)
> list_del(&request->client_list);
> request->file_priv = NULL;
> spin_unlock(&file_priv->mm.lock);
> -
> - put_pid(request->pid);
> - request->pid = NULL;
> }
>
> void i915_gem_retire_noop(struct i915_gem_active *active,
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
> index 1f396f470a86..72a4b73cbb79 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -134,9 +134,6 @@ struct drm_i915_gem_request {
> /** file_priv list entry for this request */
> struct list_head client_list;
>
> - /** process identifier submitting this request */
> - struct pid *pid;
> -
> /**
> * The ELSP only accepts two elements at a time, so we queue
> * context/tail pairs on a given queue (ring->execlist_queue) until the
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 9faac19029cd..52d1564f37c4 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -460,7 +460,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> dev_priv->engine[i].name,
> ee->num_requests);
> for (j = 0; j < ee->num_requests; j++) {
> - err_printf(m, " seqno 0x%08x, emitted %ld, tail 0x%08x\n",
> + err_printf(m, " pid %d, seqno 0x%08x, emitted %ld, tail 0x%08x\n",
> + ee->requests[j].pid,
> ee->requests[j].seqno,
> ee->requests[j].jiffies,
> ee->requests[j].tail);
> @@ -1061,6 +1062,7 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
> request = i915_gem_find_active_request(engine);
> if (request) {
> struct intel_ring *ring;
> + struct pid *pid;
>
> ee->vm = request->ctx->ppgtt ?
> &request->ctx->ppgtt->base : &ggtt->base;
> @@ -1082,11 +1084,12 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
> i915_error_object_create(dev_priv,
> request->ctx->engine[i].state);
>
> - if (request->pid) {
> + pid = request->ctx->pid;
> + if (pid) {
> struct task_struct *task;
>
> rcu_read_lock();
> - task = pid_task(request->pid, PIDTYPE_PID);
> + task = pid_task(pid, PIDTYPE_PID);
> if (task) {
> strcpy(ee->comm, task->comm);
> ee->pid = task->pid;
> @@ -1150,6 +1153,10 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
> erq->seqno = request->fence.seqno;
> erq->jiffies = request->emitted_jiffies;
> erq->tail = request->postfix;
> +
> + rcu_read_lock();
> + erq->pid = request->ctx ? pid_nr(request->ctx->pid) : 0;
> + rcu_read_unlock();
> }
> }
> }
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
More information about the Intel-gfx
mailing list