From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Yang Jihong <yangjihong1@huawei.com>,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: [PATCH v1 2/2] perf srcline: Add a timeout to reading from addr2line
Date: Wed, 7 Jun 2023 23:18:12 -0700 [thread overview]
Message-ID: <20230608061812.3715566-2-irogers@google.com> (raw)
In-Reply-To: <20230608061812.3715566-1-irogers@google.com>
addr2line may fail to send expected values causing perf to wait
indefinitely. Add a 1 second timeout (twice the timeout for reading
from /proc/pid/maps) so that such reads don't cause perf to appear to
lock up.
There are already checks that the file for addr2line contains a debug
section but this isn't always sufficient. The problem was observed
when a valid elf file would set the configuration for binutils
addr2line, then a later read of vmlinux with ELF debug sections would
cause a failing write/read which would block indefinitely.
As a service to future readers, if the io hits eof or an error,
cleanup the addr2line process.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/config.c | 7 +++++--
tools/perf/util/srcline.c | 11 ++++++++---
tools/perf/util/srcline.h | 1 +
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index f340dc73db6d..46f144c46827 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -19,6 +19,7 @@
#include "util/llvm-utils.h" /* perf_llvm_config */
#include "util/stat.h" /* perf_stat__set_big_num */
#include "util/evsel.h" /* evsel__hw_names, evsel__use_bpf_counters */
+#include "util/srcline.h" /* addr2line_timeout_ms */
#include "build-id.h"
#include "debug.h"
#include "config.h"
@@ -434,12 +435,14 @@ static int perf_buildid_config(const char *var, const char *value)
return 0;
}
-static int perf_default_core_config(const char *var __maybe_unused,
- const char *value __maybe_unused)
+static int perf_default_core_config(const char *var, const char *value)
{
if (!strcmp(var, "core.proc-map-timeout"))
proc_map_timeout = strtoul(value, NULL, 10);
+ if (!strcmp(var, "core.addr2line-timeout"))
+ addr2line_timeout_ms = strtoul(value, NULL, 10);
+
/* Add other config variables here. */
return 0;
}
diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index cfca03abd6f8..e06a345c5d19 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -21,6 +21,8 @@
#include "symbol.h"
#include "subcmd/run-command.h"
+/* If addr2line doesn't return data for 1 second then timeout. */
+int addr2line_timeout_ms = 1 * 1000;
bool srcline_full_filename;
static const char *dso__name(struct dso *dso)
@@ -512,7 +514,6 @@ static int read_addr2line_record(struct io *io,
zfree(&line);
return 0;
}
-
if (function != NULL)
*function = strdup(strim(line));
@@ -574,7 +575,7 @@ static int addr2line(const char *dso_name, u64 addr,
int len;
char buf[128];
ssize_t written;
- struct io io;
+ struct io io = { .eof = false };
enum a2l_style a2l_style;
if (!a2l) {
@@ -616,7 +617,7 @@ static int addr2line(const char *dso_name, u64 addr,
goto out;
}
io__init(&io, a2l->out, buf, sizeof(buf));
-
+ io.timeout_ms = addr2line_timeout_ms;
switch (read_addr2line_record(&io, a2l_style,
&record_function, &record_filename, &record_line_nr)) {
case -1:
@@ -686,6 +687,10 @@ static int addr2line(const char *dso_name, u64 addr,
out:
free(record_function);
free(record_filename);
+ if (io.eof) {
+ dso->a2l = NULL;
+ addr2line_subprocess_cleanup(a2l);
+ }
return ret;
}
diff --git a/tools/perf/util/srcline.h b/tools/perf/util/srcline.h
index b11a0aaaa676..35a5ff3e78f5 100644
--- a/tools/perf/util/srcline.h
+++ b/tools/perf/util/srcline.h
@@ -9,6 +9,7 @@
struct dso;
struct symbol;
+extern int addr2line_timeout_ms;
extern bool srcline_full_filename;
char *get_srcline(struct dso *dso, u64 addr, struct symbol *sym,
bool show_sym, bool show_addr, u64 ip);
--
2.41.0.rc0.172.g3f132b7071-goog
next prev parent reply other threads:[~2023-06-08 6:20 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-08 6:18 [PATCH v1 1/2] tools api: Add simple timeout to io read Ian Rogers
2023-06-08 6:18 ` Ian Rogers [this message]
2023-06-14 17:36 ` Ian Rogers
2023-06-14 18:44 ` Arnaldo Carvalho de Melo
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=20230608061812.3715566-2-irogers@google.com \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=yangjihong1@huawei.com \
/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;
as well as URLs for NNTP newsgroup(s).