mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Paul Burton <paulburton@google.com>
To: linux-kernel@vger.kernel.org
Cc: Paul Burton <paulburton@google.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	Joel Fernandes <joelaf@google.com>,
	stable@vger.kernel.org
Subject: [PATCH 1/2] tracing: Simplify & fix saved_tgids logic
Date: Tue, 29 Jun 2021 17:34:05 -0700	[thread overview]
Message-ID: <20210630003406.4013668-1-paulburton@google.com> (raw)

The tgid_map array records a mapping from pid to tgid, where the index
of an entry within the array is the pid & the value stored at that index
is the tgid.

The saved_tgids_next() function iterates over pointers into the tgid_map
array & dereferences the pointers which results in the tgid, but then it
passes that dereferenced value to trace_find_tgid() which treats it as a
pid & does a further lookup within the tgid_map array. It seems likely
that the intent here was to skip over entries in tgid_map for which the
recorded tgid is zero, but instead we end up skipping over entries for
which the thread group leader hasn't yet had its own tgid recorded in
tgid_map.

A minimal fix would be to remove the call to trace_find_tgid, turning:

  if (trace_find_tgid(*ptr))

into:

  if (*ptr)

..but it seems like this logic can be much simpler if we simply let
seq_read() iterate over the whole tgid_map array & filter out empty
entries by returning SEQ_SKIP from saved_tgids_show(). Here we take that
approach, removing the incorrect logic here entirely.

Signed-off-by: Paul Burton <paulburton@google.com>
Fixes: d914ba37d714 ("tracing: Add support for recording tgid of tasks")
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Joel Fernandes <joelaf@google.com>
Cc: <stable@vger.kernel.org>
---
 kernel/trace/trace.c | 38 +++++++++++++-------------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d23a09d3eb37b..9570667310bcc 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5608,37 +5608,20 @@ static const struct file_operations tracing_readme_fops = {
 
 static void *saved_tgids_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	int *ptr = v;
+	int pid = ++(*pos);
 
-	if (*pos || m->count)
-		ptr++;
-
-	(*pos)++;
-
-	for (; ptr <= &tgid_map[PID_MAX_DEFAULT]; ptr++) {
-		if (trace_find_tgid(*ptr))
-			return ptr;
-	}
+	if (pid > PID_MAX_DEFAULT)
+		return NULL;
 
-	return NULL;
+	return &tgid_map[pid];
 }
 
 static void *saved_tgids_start(struct seq_file *m, loff_t *pos)
 {
-	void *v;
-	loff_t l = 0;
-
-	if (!tgid_map)
+	if (!tgid_map || *pos > PID_MAX_DEFAULT)
 		return NULL;
 
-	v = &tgid_map[0];
-	while (l <= *pos) {
-		v = saved_tgids_next(m, v, &l);
-		if (!v)
-			return NULL;
-	}
-
-	return v;
+	return &tgid_map[*pos];
 }
 
 static void saved_tgids_stop(struct seq_file *m, void *v)
@@ -5647,9 +5630,14 @@ static void saved_tgids_stop(struct seq_file *m, void *v)
 
 static int saved_tgids_show(struct seq_file *m, void *v)
 {
-	int pid = (int *)v - tgid_map;
+	int *entry = (int *)v;
+	int pid = entry - tgid_map;
+	int tgid = *entry;
+
+	if (tgid == 0)
+		return SEQ_SKIP;
 
-	seq_printf(m, "%d %d\n", pid, trace_find_tgid(pid));
+	seq_printf(m, "%d %d\n", pid, tgid);
 	return 0;
 }
 

base-commit: 62fb9874f5da54fdb243003b386128037319b219
-- 
2.32.0.93.g670b81a890-goog


             reply	other threads:[~2021-06-30  0:34 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-30  0:34 Paul Burton [this message]
2021-06-30  0:34 ` [PATCH 2/2] tracing: Resize tgid_map to PID_MAX_LIMIT, not PID_MAX_DEFAULT Paul Burton
2021-06-30 12:35   ` Steven Rostedt
2021-06-30 21:09     ` Paul Burton
2021-06-30 21:34       ` Steven Rostedt
2021-06-30 22:34         ` Joel Fernandes
2021-06-30 23:11           ` Steven Rostedt
2021-07-01 13:55             ` Steven Rostedt
2021-07-01 17:24               ` [PATCH v2 1/2] tracing: Simplify & fix saved_tgids logic Paul Burton
2021-07-01 17:24                 ` [PATCH v2 2/2] tracing: Resize tgid_map to pid_max, not PID_MAX_DEFAULT Paul Burton
2021-07-01 18:12                   ` Steven Rostedt
2021-07-01 18:15                     ` Paul Burton
2021-07-01 18:27                       ` Steven Rostedt
2021-07-01 18:07                 ` [PATCH v2 1/2] tracing: Simplify & fix saved_tgids logic Joel Fernandes
2021-06-30 12:31 ` [PATCH " Steven Rostedt
2021-06-30 16:43   ` Joel Fernandes
2021-06-30 22:29 ` Joel Fernandes
2021-07-01 17:31   ` Paul Burton
2021-07-01 18:05     ` Joel Fernandes
2021-07-01 18:07     ` Steven Rostedt
2021-07-01 18:09       ` Joel Fernandes
2021-07-01 18:12       ` Paul Burton
2021-07-01 18:26         ` Steven Rostedt
2021-07-01 19:35           ` Joe Perches
2021-07-01 19:51             ` Steven Rostedt
2021-07-01 21:07               ` Joe Perches
2021-07-01 23:49                 ` Joel Fernandes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210630003406.4013668-1-paulburton@google.com \
    --to=paulburton@google.com \
    --cc=joelaf@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®