linux-kernel - [PATCH 1/6] drm/msm: show task cmdline in gpu recovery messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20171024132256.20286-2-robdclark@gmail.com>
Date:   Tue, 24 Oct 2017 09:22:48 -0400
From:   Rob Clark <robdclark@...il.com>
To:     dri-devel@...ts.freedesktop.org
Cc:     linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org,
        Jordan Crouse <jcrouse@...eaurora.org>,
        Rob Clark <robdclark@...il.com>,
        David Airlie <airlied@...ux.ie>, linux-kernel@...r.kernel.org
Subject: [PATCH 1/6] drm/msm: show task cmdline in gpu recovery messages

Now that freedreno gallium driver defaults to using submit_queue task
(render reordering), just showing task->comm is not so useful (ie. it is
always "flush_queue:0"), so also dump the cmdline.  This should also be
more useful for piglit/shader_runner.

Signed-off-by: Rob Clark <robdclark@...il.com>
---
 drivers/gpu/drm/msm/msm_gpu.c | 54 +++++++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index a05aa119f22b..d26a7282466e 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -235,6 +235,20 @@ static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
 	}
 }
 
+static struct msm_gem_submit *
+find_submit(struct msm_ringbuffer *ring, uint32_t fence)
+{
+	struct msm_gem_submit *submit;
+
+	WARN_ON(!mutex_is_locked(&ring->gpu->dev->struct_mutex));
+
+	list_for_each_entry(submit, &ring->submits, node)
+		if (submit->seqno == fence)
+			return submit;
+
+	return NULL;
+}
+
 static void retire_submits(struct msm_gpu *gpu);
 
 static void recover_worker(struct work_struct *work)
@@ -268,19 +282,35 @@ static void recover_worker(struct work_struct *work)
 	dev_err(dev->dev, "%s: hangcheck recover!\n", gpu->name);
 	fence = cur_ring->memptrs->fence + 1;
 
-	list_for_each_entry(submit, &cur_ring->submits, node) {
-		if (submit->seqno == fence) {
-			struct task_struct *task;
-
-			rcu_read_lock();
-			task = pid_task(submit->pid, PIDTYPE_PID);
-			if (task) {
-				dev_err(dev->dev, "%s: offending task: %s\n",
-						gpu->name, task->comm);
-			}
-			rcu_read_unlock();
-			break;
+	submit = find_submit(cur_ring, fence);
+	if (submit) {
+		struct task_struct *task;
+
+		rcu_read_lock();
+		task = pid_task(submit->pid, PIDTYPE_PID);
+		if (task) {
+			char buf[256];
+			int len;
+
+			/*
+			 * so slightly annoying, in other paths like mmap'ing gem
+			 * buffers, mmap_sem is acquired before struct_mutex, which
+			 * means we can't hold struct_mutex across the call to
+			 * get_cmdline().  But we don't need 'submit' any further
+			 * so no issue if hypothetically it goes away when dropping
+			 * struct_mutex.  For good measure null out the submit ptr
+			 * to make this obvious.
+			 */
+			submit = NULL;
+			mutex_unlock(&dev->struct_mutex);
+			len = get_cmdline(task, buf, sizeof(buf));
+			mutex_lock(&dev->struct_mutex);
+
+			dev_err(dev->dev, "%s: offending task: %s (%-*s)\n",
+					gpu->name, task->comm, len, buf);
 		}
+		rcu_read_unlock();
+
 	}
 
 	if (msm_gpu_active(gpu)) {
-- 
2.13.6