| 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
| ||
|
Message-ID: <20240805234648.1453689-1-namhyung@kernel.org>
Date: Mon, 5 Aug 2024 16:46:48 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Kan Liang <kan.liang@...ux.intel.com>
Cc: Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
Subject: [PATCH v3] perf annotate: Cache debuginfo for data type profiling
In find_data_type(), it creates and deletes a debug info whenver it
tries to find data type for a sample. This is inefficient and it most
likely accesses the same binary again and again.
Let's add a single entry cache the debug info structure for the last DSO.
Depending on sample data, it usually gives me 2~3x (and sometimes more)
speed ups.
Note that this will introduce a little difference in the output due to
the order of checking stack operations. It used to check the stack ops
before checking the availability of debug info but I moved it after the
symbol check. So it'll report stack operations in DSOs without debug
info as unknown. But I think it's ok and better to have the checking
near the caching logic.
Cc: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@...nel.org>
---
v2) add an assertion for the single-threaded case.
v3) update the comment according to Ian.
tools/perf/util/annotate-data.c | 15 ++-----------
tools/perf/util/annotate-data.h | 2 +-
tools/perf/util/annotate.c | 37 +++++++++++++++++++++++++++++++++
tools/perf/util/annotate.h | 2 ++
tools/perf/util/session.c | 2 ++
5 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 734acdd8c4b7..f125ac5f0bda 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1345,16 +1345,9 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die)
*/
struct annotated_data_type *find_data_type(struct data_loc_info *dloc)
{
- struct annotated_data_type *result = NULL;
struct dso *dso = map__dso(dloc->ms->map);
Dwarf_Die type_die;
- dloc->di = debuginfo__new(dso__long_name(dso));
- if (dloc->di == NULL) {
- pr_debug_dtp("cannot get the debug info\n");
- return NULL;
- }
-
/*
* The type offset is the same as instruction offset by default.
* But when finding a global variable, the offset won't be valid.
@@ -1364,13 +1357,9 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc)
dloc->fbreg = -1;
if (find_data_type_die(dloc, &type_die) < 0)
- goto out;
-
- result = dso__findnew_data_type(dso, &type_die);
+ return NULL;
-out:
- debuginfo__delete(dloc->di);
- return result;
+ return dso__findnew_data_type(dso, &type_die);
}
static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_entries)
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 992b7ce4bd11..37a1a3b68e0b 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -123,9 +123,9 @@ struct data_loc_info {
u64 var_addr;
u8 cpumode;
struct annotated_op_loc *op;
+ struct debuginfo *di;
/* These are used internally */
- struct debuginfo *di;
int fbreg;
bool fb_cfa;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index a2ee4074f768..a28d8a4b8a14 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -25,6 +25,7 @@
#include "srcline.h"
#include "units.h"
#include "debug.h"
+#include "debuginfo.h"
#include "annotate.h"
#include "annotate-data.h"
#include "evsel.h"
@@ -2326,6 +2327,20 @@ u64 annotate_calc_pcrel(struct map_symbol *ms, u64 ip, int offset,
return map__rip_2objdump(ms->map, addr);
}
+static struct debuginfo_cache {
+ struct dso *dso;
+ struct debuginfo *dbg;
+} di_cache;
+
+void debuginfo_cache__delete(void)
+{
+ dso__put(di_cache.dso);
+ di_cache.dso = NULL;
+
+ debuginfo__delete(di_cache.dbg);
+ di_cache.dbg = NULL;
+}
+
/**
* hist_entry__get_data_type - find data type for given hist entry
* @he: hist entry
@@ -2360,6 +2375,27 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
return NULL;
}
+ /*
+ * di_cache holds a pair of values, but code below assumes
+ * di_cache.dso can be compared/updated and di_cache.dbg can be
+ * read/updated independently from each other. That assumption only
+ * holds in single threaded code.
+ */
+ assert(perf_singlethreaded);
+
+ if (map__dso(ms->map) != di_cache.dso) {
+ dso__put(di_cache.dso);
+ di_cache.dso = dso__get(map__dso(ms->map));
+
+ debuginfo__delete(di_cache.dbg);
+ di_cache.dbg = debuginfo__new(dso__long_name(di_cache.dso));
+ }
+
+ if (di_cache.dbg == NULL) {
+ ann_data_stat.no_dbginfo++;
+ return NULL;
+ }
+
/* Make sure it has the disasm of the function */
if (symbol__annotate(ms, evsel, &arch) < 0) {
ann_data_stat.no_insn++;
@@ -2404,6 +2440,7 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
.ip = ms->sym->start + dl->al.offset,
.cpumode = he->cpumode,
.op = op_loc,
+ .di = di_cache.dbg,
};
if (!op_loc->mem_ref && op_loc->segment == INSN_SEG_NONE)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 9ba772f46270..27d9540604ef 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -543,4 +543,6 @@ struct annotated_basic_block {
int annotate_get_basic_blocks(struct symbol *sym, s64 src, s64 dst,
struct list_head *head);
+void debuginfo_cache__delete(void);
+
#endif /* __PERF_ANNOTATE_H */
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 5596bed1b8c8..f9072e003367 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -36,6 +36,7 @@
#include "util.h"
#include "arch/common.h"
#include "units.h"
+#include "annotate.h"
#include <internal/lib.h>
#ifdef HAVE_ZSTD_SUPPORT
@@ -304,6 +305,7 @@ void perf_session__delete(struct perf_session *session)
return;
auxtrace__free(session);
auxtrace_index__free(&session->auxtrace_index);
+ debuginfo_cache__delete();
perf_session__destroy_kernel_maps(session);
perf_decomp__release_events(session->decomp_data.decomp);
perf_env__exit(&session->header.env);
--
2.46.0.rc2.264.g509ed76dc8-goog
Powered by blists - more mailing lists