From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>,
Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>,
Stephane Eranian <eranian@google.com>,
Masami Hiramatsu <mhiramat@kernel.org>,
linux-toolchains@vger.kernel.org,
linux-trace-devel@vger.kernel.org
Subject: [PATCH 23/23] perf annotate-data: Do not retry for invalid types
Date: Mon, 18 Mar 2024 22:51:15 -0700 [thread overview]
Message-ID: <20240319055115.4063940-24-namhyung@kernel.org> (raw)
In-Reply-To: <20240319055115.4063940-1-namhyung@kernel.org>
In some cases, it was able to find a type or location info (for per-cpu
variable) but cannot match because of invalid offset or missing global
information. In those cases, it's meaningless to go to the outer scope
and retry because there will be no additional information.
Let's change the return type of find_matching_type() and bail out if it
returns -1 for the cases.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate-data.c | 83 +++++++++++++++++++--------------
1 file changed, 48 insertions(+), 35 deletions(-)
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 4b3184b7c799..de035db9d9b4 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1115,10 +1115,15 @@ static void setup_stack_canary(struct data_loc_info *dloc)
}
}
-/* It's at the target address, check if it has a matching type */
-static bool check_matching_type(struct type_state *state,
- struct data_loc_info *dloc, int reg,
- Dwarf_Die *cu_die, Dwarf_Die *type_die)
+/*
+ * It's at the target address, check if it has a matching type.
+ * It returns 1 if found, 0 if not or -1 if not found but no need to
+ * repeat the search. The last case is for per-cpu variables which
+ * are similar to global variables and no additional info is needed.
+ */
+static int check_matching_type(struct type_state *state,
+ struct data_loc_info *dloc, int reg,
+ Dwarf_Die *cu_die, Dwarf_Die *type_die)
{
Dwarf_Word size;
u32 insn_offset = dloc->ip - dloc->ms->sym->start;
@@ -1137,20 +1142,20 @@ static bool check_matching_type(struct type_state *state,
* dereference a memory location.
*/
if (tag != DW_TAG_pointer_type && tag != DW_TAG_array_type)
- return false;
+ return -1;
/* Remove the pointer and get the target type */
if (die_get_real_type(&state->regs[reg].type, type_die) == NULL)
- return false;
+ return -1;
dloc->type_offset = dloc->op->offset;
/* Get the size of the actual type */
if (dwarf_aggregate_size(type_die, &size) < 0 ||
(unsigned)dloc->type_offset >= size)
- return false;
+ return -1;
- return true;
+ return 1;
}
if (reg == dloc->fbreg) {
@@ -1160,18 +1165,18 @@ static bool check_matching_type(struct type_state *state,
stack = find_stack_state(state, dloc->type_offset);
if (stack == NULL)
- return false;
+ return 0;
if (stack->kind == TSR_KIND_CANARY) {
setup_stack_canary(dloc);
- return false;
+ return -1;
}
*type_die = stack->type;
/* Update the type offset from the start of slot */
dloc->type_offset -= stack->offset;
- return true;
+ return 1;
}
if (dloc->fb_cfa) {
@@ -1185,22 +1190,22 @@ static bool check_matching_type(struct type_state *state,
fbreg = -1;
if (reg != fbreg)
- return false;
+ return 0;
stack = find_stack_state(state, dloc->type_offset - fboff);
if (stack == NULL)
- return false;
+ return 0;
if (stack->kind == TSR_KIND_CANARY) {
setup_stack_canary(dloc);
- return false;
+ return -1;
}
*type_die = stack->type;
/* Update the type offset from the start of slot */
dloc->type_offset -= fboff + stack->offset;
- return true;
+ return 1;
}
if (state->regs[reg].kind == TSR_KIND_PERCPU_BASE) {
@@ -1212,9 +1217,10 @@ static bool check_matching_type(struct type_state *state,
if (get_global_var_type(cu_die, dloc, dloc->ip, var_addr,
&var_offset, type_die)) {
dloc->type_offset = var_offset;
- return true;
+ return 1;
}
- return false;
+ /* No need to retry per-cpu (global) variables */
+ return -1;
}
if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_POINTER) {
@@ -1231,9 +1237,9 @@ static bool check_matching_type(struct type_state *state,
/* Get the size of the actual type */
if (dwarf_aggregate_size(type_die, &size) < 0 ||
(unsigned)dloc->type_offset >= size)
- return false;
+ return -1;
- return true;
+ return 1;
}
if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_CANARY) {
@@ -1246,7 +1252,7 @@ static bool check_matching_type(struct type_state *state,
*/
setup_stack_canary(dloc);
- return false;
+ return -1;
}
if (map__dso(dloc->ms->map)->kernel && arch__is(dloc->arch, "x86")) {
@@ -1262,9 +1268,9 @@ static bool check_matching_type(struct type_state *state,
if (get_global_var_type(cu_die, dloc, dloc->ip, addr,
&offset, type_die)) {
dloc->type_offset = offset;
- return true;
+ return 1;
}
- return false;
+ return -1;
}
/* Access to per-cpu base like "-0x7dcf0500(,%rdx,8)" */
@@ -1280,26 +1286,28 @@ static bool check_matching_type(struct type_state *state,
pr_debug_dtp(" percpu base\n");
dloc->type_offset = offset;
- return true;
+ return 1;
}
+ pr_debug_dtp(" negative offset\n");
+ return -1;
}
}
pr_debug_dtp("\n");
- return false;
+ return 0;
}
/* Iterate instructions in basic blocks and update type table */
-static bool find_data_type_insn(struct data_loc_info *dloc, int reg,
- struct list_head *basic_blocks,
- struct die_var_type *var_types,
- Dwarf_Die *cu_die, Dwarf_Die *type_die)
+static int find_data_type_insn(struct data_loc_info *dloc, int reg,
+ struct list_head *basic_blocks,
+ struct die_var_type *var_types,
+ Dwarf_Die *cu_die, Dwarf_Die *type_die)
{
struct type_state state;
struct symbol *sym = dloc->ms->sym;
struct annotation *notes = symbol__annotation(sym);
struct annotated_basic_block *bb;
- bool found = false;
+ int ret = 0;
init_type_state(&state, dloc->arch);
@@ -1317,8 +1325,8 @@ static bool find_data_type_insn(struct data_loc_info *dloc, int reg,
update_var_state(&state, dloc, addr, dl->al.offset, var_types);
if (this_ip == dloc->ip) {
- found = check_matching_type(&state, dloc, reg,
- cu_die, type_die);
+ ret = check_matching_type(&state, dloc, reg,
+ cu_die, type_die);
goto out;
}
@@ -1331,7 +1339,7 @@ static bool find_data_type_insn(struct data_loc_info *dloc, int reg,
out:
exit_type_state(&state);
- return found;
+ return ret;
}
/*
@@ -1355,6 +1363,7 @@ static int find_data_type_block(struct data_loc_info *dloc, int reg,
for (int i = nr_scopes - 1; i >= 0; i--) {
Dwarf_Addr base, start, end;
LIST_HEAD(this_blocks);
+ int found;
if (dwarf_ranges(&scopes[i], 0, &base, &start, &end) < 0)
break;
@@ -1385,15 +1394,19 @@ static int find_data_type_block(struct data_loc_info *dloc, int reg,
fixup_var_address(var_types, start);
/* Find from start of this scope to the target instruction */
- if (find_data_type_insn(dloc, reg, &basic_blocks, var_types,
- cu_die, type_die)) {
- ret = 0;
+ found = find_data_type_insn(dloc, reg, &basic_blocks, var_types,
+ cu_die, type_die);
+ if (found > 0) {
pr_debug_dtp("found by insn track: %#x(reg%d) type-offset=%#x",
dloc->op->offset, reg, dloc->type_offset);
pr_debug_type_name(type_die, TSR_KIND_TYPE);
+ ret = 0;
break;
}
+ if (found < 0)
+ break;
+
/* Go up to the next scope and find blocks to the start */
prev_dst_ip = dst_ip;
dst_ip = src_ip;
--
2.44.0.291.gc1ea87d7ee-goog
next prev parent reply other threads:[~2024-03-19 5:51 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-19 5:50 [PATCHSET 00/23] Remaining bits of data type profiling (v7) Namhyung Kim
2024-03-19 5:50 ` [PATCH 01/23] perf dwarf-aux: Remove unused pc argument Namhyung Kim
2024-03-19 13:43 ` Arnaldo Carvalho de Melo
2024-03-19 17:39 ` Namhyung Kim
2024-03-19 5:50 ` [PATCH 02/23] perf dwarf-aux: Add die_collect_vars() Namhyung Kim
2024-03-19 13:45 ` Arnaldo Carvalho de Melo
2024-03-19 5:50 ` [PATCH 03/23] perf dwarf-aux: Handle type transfer for memory access Namhyung Kim
2024-03-19 13:55 ` Arnaldo Carvalho de Melo
2024-03-19 17:41 ` Namhyung Kim
2024-03-19 5:50 ` [PATCH 04/23] perf dwarf-aux: Add die_find_func_rettype() Namhyung Kim
2024-03-19 13:56 ` Arnaldo Carvalho de Melo
2024-03-19 17:42 ` Namhyung Kim
2024-03-19 18:19 ` Arnaldo Carvalho de Melo
2024-03-19 20:33 ` Namhyung Kim
2024-03-19 5:50 ` [PATCH 05/23] perf map: Add map__objdump_2rip() Namhyung Kim
2024-03-19 5:50 ` [PATCH 06/23] perf annotate-data: Introduce struct data_loc_info Namhyung Kim
2024-03-19 5:50 ` [PATCH 07/23] perf annotate: Add annotate_get_basic_blocks() Namhyung Kim
2024-03-19 5:51 ` [PATCH 08/23] perf annotate-data: Add debug messages Namhyung Kim
2024-03-19 14:05 ` Arnaldo Carvalho de Melo
2024-03-19 5:51 ` [PATCH 09/23] perf annotate-data: Maintain variable type info Namhyung Kim
2024-03-19 14:07 ` Arnaldo Carvalho de Melo
2024-03-19 17:44 ` Namhyung Kim
2024-03-19 18:12 ` Arnaldo Carvalho de Melo
2024-03-19 20:34 ` Namhyung Kim
2024-03-19 5:51 ` [PATCH 10/23] perf annotate-data: Add update_insn_state() Namhyung Kim
2024-03-19 5:51 ` [PATCH 11/23] perf annotate-data: Add get_global_var_type() Namhyung Kim
2024-03-19 5:51 ` [PATCH 12/23] perf annotate-data: Handle global variable access Namhyung Kim
2024-03-19 5:51 ` [PATCH 13/23] perf annotate-data: Handle call instructions Namhyung Kim
2024-03-19 5:51 ` [PATCH 14/23] perf annotate-data: Implement instruction tracking Namhyung Kim
2024-03-19 5:51 ` [PATCH 15/23] perf annotate-data: Check register state for type Namhyung Kim
2024-03-19 5:51 ` [PATCH 16/23] perf annotate: Parse x86 segment register location Namhyung Kim
2024-03-19 5:51 ` [PATCH 17/23] perf annotate-data: Handle this-cpu variables in kernel Namhyung Kim
2024-03-19 5:51 ` [PATCH 18/23] perf annotate-data: Track instructions with a this-cpu variable Namhyung Kim
2024-03-19 5:51 ` [PATCH 19/23] perf annotate-data: Support general per-cpu access Namhyung Kim
2024-03-19 5:51 ` [PATCH 20/23] perf annotate-data: Handle ADD instructions Namhyung Kim
2024-03-19 5:51 ` [PATCH 21/23] perf annotate-data: Add stack canary type Namhyung Kim
2024-03-19 5:51 ` [PATCH 22/23] perf annotate-data: Add a cache for global variable types Namhyung Kim
2024-03-19 18:05 ` Namhyung Kim
2024-03-19 18:07 ` Arnaldo Carvalho de Melo
2024-03-19 18:09 ` Arnaldo Carvalho de Melo
2024-03-19 18:09 ` Namhyung Kim
2024-03-19 5:51 ` Namhyung Kim [this message]
2024-03-19 14:17 ` [PATCHSET 00/23] Remaining bits of data type profiling (v7) 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=20240319055115.4063940-24-namhyung@kernel.org \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linux-toolchains@vger.kernel.org \
--cc=linux-trace-devel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=torvalds@linux-foundation.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