Implement lbr-as-callgraph v7
[Even more review feedback and some bugs addressed.] [Only port to changes in perf/core. No other changes.] [Rebase to latest perf/core]
This patchkit implements lbr-as-callgraphs in per freport, as an alternative way to present LBR information.
Current perf report does a histogram over the branch edges, which is useful to look at basic blocks, but doesn't tell you anything about the larger control flow behaviour.
This patchkit adds a new option --branch-history that adds the branch paths to the callgraph history instead.
This allows to reason about individual branch paths leading to specific samples.
Also available atgit://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc perf/lbr-callgraph4
v2:
- rebased on perf/core
- fix various issues
- rename the option to --branch-history
- various fixes to display the information more concise v3:
- White space changes
- Consolidate some patches
- Update some descriptions
v4:
- Fix various display problems
- Unknown srcline is now printed as symbol+offset
- Refactor some code to address review feedback
- Merge with latest tip
- Fix missing srcline display in stdio hist output. v5:
- Rename functions
- Fix gtk build problem
- Fix crash without -g
- Improve error messages
- Improve srcline display in various ways
v6:
- Port to latest perf/core
v7:
- Really port to latest perf/core
Example output:
% perf record -b -g ./tsrc/tcall
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.044 MB perf.data (~1923 samples) ] % perf report --branch-history
...
54.91% tcall.c:6 [.] f2 tcall
|
|--65.53%-- f2 tcall.c:5
| |
| |--70.83%-- f1 tcall.c:11
| | f1 tcall.c:10
| | main tcall.c:18
| | main tcall.c:18
| | main tcall.c:17
| | main tcall.c:17
| | f1 tcall.c:13
| | f1 tcall.c:13
| | f2 tcall.c:7
| | f2 tcall.c:5
| | f1 tcall.c:12
| | f1 tcall.c:12
| | f2 tcall.c:7
| | f2 tcall.c:5
| | f1 tcall.c:11
PATCH 1/9 perf, tools: Support handling complete branch stacks as histograms v6
From: Andi Kleen <[email protected]>
Currently branch stacks can be only shown as edge histograms for individual branches. I never found this display particularly useful.
This implements an alternative mode that creates histograms over complete branch traces, instead of individual branches, similar to how normal callgraphs are handled. This is done by putting it in
front of the normal callgraph and then using the normal callgraph histogram infrastructure to unify them.
This way in complex functions we can understand the control flow that lead to a particular sample, and may even see some control flow in the caller for short functions.
Example (simplified, of course for such simple code this is usually not needed):
tcall.c:
volatile a = 10000, b = 100000, c;
__attribute__((noinline)) f2()
{
c = a / b;
}__attribute__((noinline)) f1()
{
f2();
f2();
}
main()
{
int i;
for (i = 0; i < 1000000; i++)
f1();
}% perf record -b -g ./tsrc/tcall
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.044 MB perf.data (~1923 samples) ] % perf report --branch-history
...
54.91% tcall.c:6 [.] f2 tcall
|
|--65.53%-- f2 tcall.c:5
| |
| |--70.83%-- f1 tcall.c:11
| | f1 tcall.c:10
| | main tcall.c:18
| | main tcall.c:18
| | main tcall.c:17
| | main tcall.c:17
| | f1 tcall.c:13
| | f1 tcall.c:13
| | f2 tcall.c:7
| | f2 tcall.c:5
| | f1 tcall.c:12
| | f1 tcall.c:12
| | f2 tcall.c:7
| | f2 tcall.c:5
| | f1 tcall.c:11
| |
| --29.17%-- f1 tcall.c:12
| f1 tcall.c:12
| f2 tcall.c:7
| f2 tcall.c:5
| f1 tcall.c:11
| f1 tcall.c:10
| main tcall.c:18
| main tcall.c:18
| main tcall.c:17
| main tcall.c:17
| f1 tcall.c:13
| f1 tcall.c:13
| f2 tcall.c:7
| f2 tcall.c:5
| f1 tcall.c:12The default output is unchanged.
This is only implemented in perf report, no change to record or anywhere else.
This adds the basic code to report:
- add a new "branch" option to the -g option parser to enable this mode
- when the flag is set include the LBR into the callstack in machine.c. The rest of the history code is unchanged and doesn't know the difference between LBR entry and normal call entry.
- detect overlaps with the callchain
- remove small loop duplicates in the LBR
Current limitations:
- The LBR flags (mispredict etc.) are not shown in the history and LBR entries have no special marker.
- It would be nice if annotate marked the LBR entries somehow (e.g. with arrows)
v2: Various fixes.
v3: Merge further patches into this one. Fix white space.
v4: Improve manpage. Address review feedback.
v5: Rename functions. Better error message without -g. Fix crash without
-b.
v6: Rebase
--- tools/perf/Documentation/perf-report.txt | 7 +- tools/perf/builtin-report.c | 4 +- tools/perf/util/callchain.c | 11 ++- tools/perf/util/callchain.h | 1 + tools/perf/util/machine.c | 159 +++++++++++++++++++++++++++---- tools/perf/util/symbol.h | 3 +- 6 files changed, 158 insertions(+), 27 deletions(-)
tools/perf/Documentation/perf-report.txt +6 -1
@@ -124,7 +124,7 @@ OPTIONS --dump-raw-trace:: Dump raw trace in ASCII. --g [type,min[,limit],order[,key]]:: +-g [type,min[,limit],order[,key][,branch]]:: --call-graph:: Display call chains using type, min percent threshold, optional print limit and order.
@@ -142,6 +142,11 @@ OPTIONS - function: compare on functions - address: compare on individual code addresses + branch can be: + - branch: include last branch information in callgraph + when available. Usually more convenient to use --branch-history + for this. + Default: fractal,0.5,callee,function. --max-stack::
tools/perf/builtin-report.c +2 -2
@@ -709,8 +709,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) "regex filter to identify parent, see: '--sort parent'"),OPT_BOOLEAN ('x' ,"exclude-other" , &symbol_conf.exclude_other,"Only display entries with parent-match" ), -OPT_CALLBACK_DEFAULT ('g' ,"call-graph" , &report,"output_type,min_percent[,print_limit],call_order" , -"Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address). " +OPT_CALLBACK_DEFAULT ('g' ,"call-graph" , &report,"output_type,min_percent[,print_limit],call_order[,branch]" , +"Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address), add branches. " "Default: fractal,0.5,callee,function" , &report_parse_callchain_opt, callchain_default_opt),OPT_INTEGER (0 ,"max-stack" , &report.max_stack, "Set the maximum stack depth when parsing the callchain, "
tools/perf/util/callchain.c +9 -2
@@ -82,7 +82,7 @@ parse_callchain_report_opt(const char *arg) callchain_param.order =ORDER_CALLER ; else if (!strncmp(tok2, "callee", strlen("callee"))) callchain_param.order =ORDER_CALLEE ; - else + else if (tok2[0] != 0) return -1;/* Get the sort key */
@@ -93,8 +93,15 @@ parse_callchain_report_opt(const char *arg) callchain_param.key =CCKEY_FUNCTION ; else if (!strncmp(tok2, "address", strlen("address"))) callchain_param.key =CCKEY_ADDRESS ; - else + else if (tok2[0] != 0) return -1; + + tok2 =strtok (NULL ,"," ); +if (!tok2) + goto setup; +if (!strncmp (tok2,"branch" ,6 )) + callchain_param.branch_callstack =1 ; + setup:if (callchain_register_param (&callchain_param) <0 ) {pr_err ("Can't register callchain params\n" );
tools/perf/util/callchain.h +1 -0
@@ -60,6 +60,7 @@ struct callchain_param {sort_chain_func_t sort;enum chain_order order;enum chain_key key; +bool branch_callstack; };struct callchain_list {
tools/perf/util/machine.c +138 -21
201 lines not on this page · raw message
tools/perf/util/symbol.h +2 -1
@@ -117,7 +117,8 @@ struct symbol_conf { annotate_src, event_group, demangle, - filter_relative; + filter_relative, + branch_callstack;const char *vmlinux_name, *kallsyms_name, *source_prefix,
Signature · 1 line
1.9.0
Re: [PATCH 1/9] perf, tools: Support handling complete branch stacks as histograms v6
Hi Andi,
Quoted 14 lines · Andi Kleen
From: Andi Kleen <[email protected]>
Currently branch stacks can be only shown as edge histograms for individual branches. I never found this display particularly useful.
This implements an alternative mode that creates histograms over complete branch traces, instead of individual branches, similar to how normal callgraphs are handled. This is done by putting it in
front of the normal callgraph and then using the normal callgraph histogram infrastructure to unify them.This way in complex functions we can understand the control flow that lead to a particular sample, and may even see some control flow in the caller for short functions.
[SNIP]
Quoted 16 lines
tools/perf/util/machine.c +16
-0+static int add_callchain_ip (struct machine *machine, +struct thread *thread, +struct symbol **parent, +struct addr_location *root_al, +int cpumode, +u64 ip) +{ +struct addr_location al; + + al.filtered =0 ; + al.sym =NULL ; +thread__find_addr_location (thread, machine, cpumode,MAP__FUNCTION , + ip, &al); +if (al.sym !=NULL ) { +if (sort__has_parent && !*parent && +symbol__match_regex (al.sym, &parent_regex))
tools/perf/util/machine.c struct branch_info *sample__resolve_bstack(struct perf_sample *sample, · line 1308
Quoted 10 lines
tools/perf/util/machine.c +10
-0+ *parent = al.sym; + else if (have_ignore_callees && root_al && + symbol__match_regex(al.sym, &ignore_callees_regex)) { +/* Treat this symbol as the root, +forgetting its callees. */ + *root_al = al; +callchain_cursor_reset (&callchain_cursor); + } +if (!symbol_conf.use_callchain) +return -EINVAL ;
This is gone with 540476de74c9 ("perf tools: Remove symbol_conf.use_callchain check").
Quoted 7 lines
tools/perf/util/machine.c +7
-0+ } + +return callchain_cursor_append (&callchain_cursor, ip, al.map, al.sym); +} + +#define CHASHSZ 127 +#define CHASHBITS 7
tools/perf/util/machine.c struct branch_info *sample__resolve_bstack(struct perf_sample *sample, · line 1325
Quoted 10 lines
tools/perf/util/machine.c +10
-0+#define NO_ENTRY 0xff + +#define PERF_MAX_BRANCH_DEPTH 127 + +/* Remove loops. */ +static int remove_loops (struct branch_entry *l,int nr) +{ +int i, j, off; +unsigned char chash[CHASHSZ ]; +memset (chash,-1 ,sizeof (chash));
s/-1/NO_ENTRY/ ?
Quoted 67 lines
tools/perf/util/machine.c +54
-0+ +BUG_ON (nr >=256 ); +for (i =0 ; i < nr; i++) { +int h =hash_64 (l[i].from,CHASHBITS ) %CHASHSZ ; + +/* no collision handling for now */ +if (chash[h] ==NO_ENTRY ) { + chash[h] = i; + }else if (l[chash[h]].from == l[i].from) { +bool is_loop = true; +/* check if it is a real loop */ + off =0 ; +for (j = chash[h]; j < i && i + off < nr; j++, off++) +if (l[j].from != l[i + off].from) { + is_loop = false; +break ; + } +if (is_loop) { +memmove (l + i, l + i + off, + (nr - (i + off)) + *sizeof (struct branch_entry )); + nr -= off; + } + } + } +return nr; +} +static int machine__resolve_callchain_sample (struct machine *machine,struct thread *thread,struct ip_callchain *chain, +struct branch_stack *branch,struct symbol **parent,struct addr_location *root_al,int max_stack)@@ -1290,17 +1363,73 @@ static int machine__resolve_callchain_sample(struct machine *machine,int chain_nr =min (max_stack, (int )chain->nr);int i;int err; +int first_call =0 ;callchain_cursor_reset (&callchain_cursor); +/* +* Add branches to call stack for easier browsing. This gives +* more context for a sample than just the callers. +* +* This uses individual histograms of paths compared to the +* aggregated histograms the normal LBR mode uses. +* +* Limitations for now: +* - No extra filters +* - No annotations (should annotate somehow) +*/ + +if (branch->nr >PERF_MAX_BRANCH_DEPTH ) { +pr_warning ("corrupted branch chain. skipping...\n" ); +return 0 ; + } + +if (callchain_param.branch_callstack) { +int nr =min (max_stack, (int )branch->nr); +struct branch_entry be[nr]; + +for (i =0 ; i < nr; i++) { +if (callchain_param.order ==ORDER_CALLEE ) { + be[i] = branch->entries[i];
tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1403
Quoted 10 lines
tools/perf/util/machine.c +10
-0+/* +* Check for overlap into the callchain. +* The return address is one off compared to +* the branch entry. To adjust for this +* assume the calling instruction is not longer +* than 8 bytes. +*/ +if (be[i].from < chain->ips[first_call] && + be[i].from >= chain->ips[first_call] -8 ) + first_call++;
It seems that you need to check chain->ips[first_call] is greater than PERF_CONTEXT_MAX and use such value as the cpumode...
Quoted 4 lines
tools/perf/util/machine.c +4
-0+ }else + be[i] = branch->entries[branch->nr - i -1 ]; + } +
tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1417
Quoted 10 lines
tools/perf/util/machine.c +10
-0+ nr =remove_loops (be, nr); + +for (i =0 ; i < nr; i++) { + err =add_callchain_ip (machine, thread, parent, + root_al, +-1 , be[i].to); +if (!err) + err =add_callchain_ip (machine, thread, + parent, root_al, +-1 , be[i].from);
... for here.
tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1423
Quoted 6 lines
tools/perf/util/machine.c +6
-0+if (err == -EINVAL ) +break ; +if (err) +return err; + } + chain_nr -= nr;
It seems it could make some callchain nodes being ignored. What if a case like small callchains with matches to only 2 nodes in the LBR?
nr = 16, chain_nr = 10 and first_call = 2
Thanks,
Namhyung
Re: [PATCH 1/9] perf, tools: Support handling complete branch stacks as histograms v6
Quoted 2 lines · Namhyung Kim
This is gone with
540476de74c9("perf tools: Remove symbol_conf.use_callchain check").
The patchkit applies to tip/perf/core.
Quoted 4 lines
Quoted 4 lines
tools/perf/util/machine.c +4
-0+* Check for overlap into the callchain. +* The return address is one off compared to +* the branch entry. To adjust for this +* assume the calling instruction is not longer
Quoted 8 lines
tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1403
Quoted 5 lines
tools/perf/util/machine.c +5
-0+* than 8 bytes. +*/ +if (be[i].from < chain->ips[first_call] && + be[i].from >= chain->ips[first_call] -8 ) + first_call++;It seems that you need to check chain->ips[first_call] is greater than
PERF_CONTEXT_MAXand use such value as the cpumode...
I don't understand the comment. The only IP that gets resolved is the from/to. And add_callchain_ip does it own resolution.
Wouldn't make any sense to get it from first_call
Quoted 30 lines
Quoted 6 lines
tools/perf/util/machine.c +4
-0+ }else + be[i] = branch->entries[branch->nr - i -1 ]; + } +tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1417
Quoted 10 lines
tools/perf/util/machine.c +10
-0+ nr =remove_loops (be, nr); + +for (i =0 ; i < nr; i++) { + err =add_callchain_ip (machine, thread, parent, + root_al, +-1 , be[i].to); +if (!err) + err =add_callchain_ip (machine, thread, + parent, root_al, +-1 , be[i].from);... for here.
tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1423
Quoted 6 lines
tools/perf/util/machine.c +6
-0+if (err == -EINVAL ) +break ; +if (err) +return err; + } + chain_nr -= nr;It seems it could make some callchain nodes being ignored. What if a case like small callchains with matches to only 2 nodes in the LBR?
Quoted 1 line
nr = 16,
chain_nr= 10 andfirst_call= 2
The chain_nr variable is just to handle it when the user specified a max_stack value. nr is always capped to max_stack too. If lbr size is >= max_stack it will end up being 0 or negative and the following loop to add normal call stack entries will do nothing.
I think that's the correct behavior.
-Andi
Re: [PATCH 1/9] perf, tools: Support handling complete branch stacks as histograms v6
Hi Andi,
Quoted 5 lines · Andi Kleen
Quoted 2 lines · Namhyung Kim
This is gone with
540476de74c9("perf tools: Remove symbol_conf.use_callchain check").The patchkit applies to tip/perf/core.
The commit 540476de74c9 is also in the tip/perf/core. Please check machine_resolve_callchain_sample().
Quoted 14 lines
Quoted 13 lines
tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1403
Quoted 10 lines
tools/perf/util/machine.c +9
-0+* Check for overlap into the callchain. +* The return address is one off compared to +* the branch entry. To adjust for this +* assume the calling instruction is not longer +* than 8 bytes. +*/ +if (be[i].from < chain->ips[first_call] && + be[i].from >= chain->ips[first_call] -8 ) + first_call++;It seems that you need to check chain->ips[first_call] is greater than
PERF_CONTEXT_MAXand use such value as the cpumode...
Quoted 4 lines
I don't understand the comment. The only IP that gets resolved is the from/to. And
add_callchain_ipdoes it own resolution.Wouldn't make any sense to get it from
first_call
Okay, let me explain it this way..
You're checking the branch stack with normal callchain to find overlap by comparing the 'from' address and the address in chain->ips[]. But chain->ips[0] doesn't contain a valid address but a PERF_CONTEXT_XXX for cpumode of subsequent callchains. So the first_call of 0 won't do anything meaningful for you and it'd still contain overlapped callchains.
$ perf --version perf version 3.15.rc4.g816bf8
$ perf record -b -g ./tcall
$ perf report -D | grep -A35 SAMPLE 4748858059190923 0x3608 [0x240]: PERF_RECORD_SAMPLE(IP, 0x1): 31914/31914: 0xffffffff81043ffa period: 1 addr: 0 ... chain: nr:17 ..... 0: ffffffffffffff80 ..... 1: ffffffff81043ffa ..... 2: ffffffff81029d40 ..... 3: ffffffff81025554 ..... 4: ffffffff811246c7 ..... 5: ffffffff81125d69 ..... 6: ffffffff811280dc ..... 7: ffffffff811a4266 ..... 8: ffffffff811a4bb1 ..... 9: ffffffff811f1a4f ..... 10: ffffffff811a344c ..... 11: ffffffff811a49bb ..... 12: ffffffff811a4ac8 ..... 13: ffffffff811a4d3d ..... 14: ffffffff81664689 ..... 15: fffffffffffffe00 ..... 16: 0000003153ebca47 ... branch stack: nr:16 ..... 0: ffffffff81029d3b -> ffffffff81043ff0 ..... 1: ffffffff810280c9 -> ffffffff81029d18 ..... 2: ffffffff81043ffd -> ffffffff810280be ..... 3: 0000000000000000 -> 0000000000000000 ..... 4: 0000000000000000 -> 0000000000000000 ..... 5: 0000000000000000 -> 0000000000000000 ..... 6: 0000000000000000 -> 0000000000000000 ..... 7: 0000000000000000 -> 0000000000000000 ..... 8: 0000000000000000 -> 0000000000000000 ..... 9: 0000000000000000 -> 0000000000000000 ..... 10: 0000000000000000 -> 0000000000000000 ..... 11: 0000000000000000 -> 0000000000000000 ..... 12: 0000000000000000 -> 0000000000000000 ..... 13: 0000000000000000 -> 0000000000000000 ..... 14: 0000000000000000 -> 0000000000000000 ..... 15: 0000000000000000 -> 0000000000000000
As you can see, chain->ips[0] is ffffffffffffff80 (= -128) that is defined as PERF_CONTEXT_KERNEL. And in this case nr of branch stack is 16 but it's actually 3. I guess you need to ignore 0 entries..
Also perf report seems to fail to resolve symbols/srclines in branch stack (possibly due to missing cpumode) and find loops.
$ perf report --branch-history --stdio
...
0.00% native_writ [k] native_write_msr_safe [kernel.kallsyms]
|
---0xffffffff81043ff0
0xffffffff81029d3b
0xffffffff81029d18
0xffffffff810280c9
0xffffffff810280be
0xffffffff81043ffd
|
|--99.77%-- 0xffffffff81043ff0
| 0xffffffff81029d3b
| 0xffffffff81029d18
| 0xffffffff810280c9
| 0xffffffff810280be
| 0xffffffff81043ffd
| 0
| 0
| |
| |--91.43%-- native_write_msr_safe +10
| | intel_pmu_enable_all +80
| | x86_pmu_enable +628
| | perf_pmu_enable +39
| | perf_event_context_sched_in +121
| | perf_event_comm +364
| | set_task_comm +102
| | setup_new_exec +129
| | load_elf_binary +1007
| |
| --8.57%-- 0
| 0
| |
| |--91.78%-- native_write_msr_safe +10
| | intel_pmu_enable_all +80
| | x86_pmu_enable +628
| | perf_pmu_enable +39
| | perf_event_context_sched_in +121
| | perf_event_comm +364
| | set_task_comm +102
| | setup_new_exec +129
| |
| --8.22%-- 0
| 0
| native_write_msr_safe +10
| intel_pmu_enable_all +80
| x86_pmu_enable +628
| perf_pmu_enable +39
| perf_event_context_sched_in +121
| perf_event_comm +364
| set_task_comm +102
--0.23%-- [...]Quoted 31 lines
Quoted 31 lines
Quoted 7 lines
tools/perf/util/machine.c +4
-0+ }else + be[i] = branch->entries[branch->nr - i -1 ]; + } +tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1417
Quoted 10 lines
tools/perf/util/machine.c +10
-0+ nr =remove_loops (be, nr); + +for (i =0 ; i < nr; i++) { + err =add_callchain_ip (machine, thread, parent, + root_al, +-1 , be[i].to); +if (!err) + err =add_callchain_ip (machine, thread, + parent, root_al, +-1 , be[i].from);... for here.
tools/perf/util/machine.c static int machine__resolve_callchain_sample(struct machine *machine, · line 1423
Quoted 6 lines
tools/perf/util/machine.c +6
-0+if (err == -EINVAL ) +break ; +if (err) +return err; + } + chain_nr -= nr;It seems it could make some callchain nodes being ignored. What if a case like small callchains with matches to only 2 nodes in the LBR?
Quoted 8 lines
Quoted 1 line
nr = 16,
chain_nr= 10 andfirst_call= 2The
chain_nrvariable is just to handle it when the user specified amax_stackvalue. nr is always capped tomax_stacktoo. If lbr size is >=max_stackit will end up being 0 or negative and the following loop to add normal call stack entries will do nothing.I think that's the correct behavior.
Hmm.. I assumed user didn't specify a max_stack but failed to understand how the code prevents from ignoring remaining 8 callchain nodes.
Thanks,
Namhyung
PATCH 2/9 perf, tools: Add --branch-history option to report v3
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
Add a --branch-history option to perf report that changes all the settings necessary for using the branches in callstacks.
This is just a short cut to make this nicer to use, it does not enable any functionality by itself.
v2: Change sort order. Rename option to --branch-history to be less confusing.
v3: Updates
--- tools/perf/Documentation/perf-report.txt | 5 +++++ tools/perf/builtin-report.c | 33 +++++++++++++++++++++++++++----- tools/perf/util/machine.c | 12 ++++++------ 3 files changed, 39 insertions(+), 11 deletions(-)
tools/perf/Documentation/perf-report.txt +5 -0
@@ -231,6 +231,11 @@ OPTIONS branch stacks and it will automatically switch to the branch view mode, unless --no-branch-stack is used. +--branch-history:: + Add the addresses of sampled taken branches to the callstack. + This allows to examine the path the program took to each sample. + The data collection must have used -b (or -j) and -g. + --objdump=<path>:: Path to objdump binary.
tools/perf/builtin-report.c +28 -5
@@ -315,8 +315,9 @@ static int report__setup_sample_type(struct report *rep)return -EINVAL ; }if (symbol_conf.use_callchain) { -ui__error ("Selected -g but no callchain data. Did " -"you call 'perf record' without -g?\n" ); +ui__error ("Selected -g or --branch-history but no " +"callchain data. Did\n" +"you call 'perf record' without -g?\n" );return -1 ; } } else if (!rep->dont_use_callchains &&
@@ -631,6 +632,16 @@ parse_branch_mode(const struct option *opt __maybe_unused, }static int +parse_branch_call_mode (const struct option *opt__maybe_unused , +const char *str__maybe_unused ,int unset) +{ +int *branch_mode = opt->value; + + *branch_mode = !unset; +return 0 ; +} + +static int parse_percent_limit (const struct option *opt,const char *str,int unset__maybe_unused ) {
@@ -645,7 +656,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)struct perf_session *session;struct stat st;bool has_br_stack = false; -int branch_mode =-1 ; +int branch_mode =-1 , branch_call_mode =-1 ;int ret =-1 ;char callchain_default_opt[] ="fractal,0.5,callee" ;const char *const report_usage[] = {
@@ -754,7 +765,11 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)OPT_BOOLEAN (0 ,"group" , &symbol_conf.event_group,"Show event group information together" ),OPT_CALLBACK_NOOPT ('b' ,"branch-stack" , &branch_mode,"" , -"use branch records for histogram filling" , parse_branch_mode), +"use branch records for per branch histogram filling" , + parse_branch_mode), +OPT_CALLBACK_NOOPT (0 ,"branch-history" , &branch_call_mode,"" , +"add last branch records to call history" , + parse_branch_call_mode),OPT_STRING (0 ,"objdump" , &objdump_path,"path" ,"objdump binary to use for disassembly and annotations" ), OPT_BOOLEAN(0, "demangle", &symbol_conf.demangle,
@@ -804,8 +819,16 @@ repeat: has_br_stack =perf_header__has_feat (&session->header,HEADER_BRANCH_STACK ); -if (branch_mode ==-1 && has_br_stack) +if (branch_mode ==-1 && has_br_stack && branch_call_mode ==-1 ) sort__mode =SORT_MODE__BRANCH ; +if (branch_call_mode !=-1 ) { + callchain_param.branch_callstack =1 ; + callchain_param.key =CCKEY_ADDRESS ; + symbol_conf.use_callchain = true; +callchain_register_param (&callchain_param); +if (sort_order == default_sort_order) + sort_order ="srcline,symbol,dso" ; + }/* sort__mode could be NORMAL if --no-branch-stack */ if (sort__mode ==SORT_MODE__BRANCH ) {
tools/perf/util/machine.c +6 -6
@@ -1379,15 +1379,15 @@ static int machine__resolve_callchain_sample(struct machine *machine,* - No annotations (should annotate somehow) */ -if (branch->nr >PERF_MAX_BRANCH_DEPTH ) { -pr_warning ("corrupted branch chain. skipping...\n" ); -return 0 ; - } - -if (callchain_param.branch_callstack) { +if (branch && callchain_param.branch_callstack) {int nr =min (max_stack, (int )branch->nr);struct branch_entry be[nr]; +if (branch->nr >PERF_MAX_BRANCH_DEPTH ) { +pr_warning ("corrupted branch chain. skipping...\n" ); +return 0 ; + } +for (i =0 ; i < nr; i++) {if (callchain_param.order ==ORDER_CALLEE ) { be[i] = branch->entries[i];
Signature · 1 line
1.9.0
Re: [PATCH 2/9] perf, tools: Add --branch-history option to report v3
Quoted 39 lines · Andi Kleen
From: Andi Kleen <[email protected]>
Add a --branch-history option to perf report that changes all the settings necessary for using the branches in callstacks.
This is just a short cut to make this nicer to use, it does not enable any functionality by itself.
v2: Change sort order. Rename option to --branch-history to be less confusing.
v3: UpdatesSigned-off-by Andi Kleen <[email protected]>--- tools/perf/Documentation/perf-report.txt | 5 +++++ tools/perf/builtin-report.c | 33 +++++++++++++++++++++++++++----- tools/perf/util/machine.c | 12 ++++++------ 3 files changed, 39 insertions(+), 11 deletions(-)tools/perf/Documentation/perf-report.txt +5
-0@@ -231,6 +231,11 @@ OPTIONS branch stacks and it will automatically switch to the branch view mode, unless --no-branch-stack is used. +--branch-history:: + Add the addresses of sampled taken branches to the callstack. + This allows to examine the path the program took to each sample. + The data collection must have used -b (or -j) and -g. + --objdump=<path>:: Path to objdump binary.tools/perf/builtin-report.c +0
-0
tools/perf/builtin-report.c static int report__setup_sample_type(struct report *rep) · line 319
Quoted 7 lines
tools/perf/builtin-report.c +2
-2@@ -315,8 +315,9 @@ static int report__setup_sample_type(struct report *rep)return -EINVAL ; }if (symbol_conf.use_callchain) { -ui__error ("Selected -g but no callchain data. Did " -"you call 'perf record' without -g?\n" ); +ui__error ("Selected -g or --branch-history but no " + "callchain data. Did\n"
An unwanted newline in the message.
Quoted 31 lines
tools/perf/builtin-report.c +12
-1+ "you call 'perf record' without -g?\n");return -1 ; } } else if (!rep->dont_use_callchains &&@@ -631,6 +632,16 @@ parse_branch_mode(const struct option *opt __maybe_unused, }static int +parse_branch_call_mode (const struct option *opt__maybe_unused , +const char *str__maybe_unused ,int unset) +{ +int *branch_mode = opt->value; + + *branch_mode = !unset; +return 0 ; +} + +static int parse_percent_limit (const struct option *opt,const char *str,int unset__maybe_unused ) {@@ -645,7 +656,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)struct perf_session *session;struct stat st;bool has_br_stack = false; -int branch_mode =-1 ; +int branch_mode =-1 , branch_call_mode =-1 ;int ret =-1 ;char callchain_default_opt[] ="fractal,0.5,callee" ;const char *const report_usage[] = {
tools/perf/builtin-report.c int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) · line 772
Quoted 9 lines
tools/perf/builtin-report.c +5
-1@@ -754,7 +765,11 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)OPT_BOOLEAN (0 ,"group" , &symbol_conf.event_group,"Show event group information together" ),OPT_CALLBACK_NOOPT ('b' ,"branch-stack" , &branch_mode,"" , - "use branch records for histogram filling", parse_branch_mode), +"use branch records for per branch histogram filling" , + parse_branch_mode), +OPT_CALLBACK_NOOPT (0 ,"branch-history" , &branch_call_mode,"" , +"add last branch records to call history" , + parse_branch_call_mode),
Looks like it can be a boolean option, or at least can share parse_branch_mode() callback.
tools/perf/builtin-report.c repeat: · line 824
Quoted 11 lines
tools/perf/builtin-report.c +2
-1OPT_STRING (0 ,"objdump" , &objdump_path,"path" ,"objdump binary to use for disassembly and annotations" ), OPT_BOOLEAN(0, "demangle", &symbol_conf.demangle,@@ -804,8 +819,16 @@ repeat: has_br_stack =perf_header__has_feat (&session->header,HEADER_BRANCH_STACK ); -if (branch_mode ==-1 && has_br_stack) +if (branch_mode ==-1 && has_br_stack && branch_call_mode ==-1 ) sort__mode =SORT_MODE__BRANCH ; +if (branch_call_mode !=-1 ) {
s/-1/1/ ?
tools/perf/builtin-report.c repeat: · line 828
Quoted 4 lines
tools/perf/builtin-report.c +4
-0+ callchain_param.branch_callstack =1 ; + callchain_param.key =CCKEY_ADDRESS ; + symbol_conf.use_callchain = true; +callchain_register_param (&callchain_param);
Isn't it better doing this in report__setup_sample_type()?
Thanks,
Namhyung
Quoted 32 lines
tools/perf/builtin-report.c +3
-0+if (sort_order == default_sort_order) + sort_order ="srcline,symbol,dso" ; + }/* sort__mode could be NORMAL if --no-branch-stack */ if (sort__mode ==SORT_MODE__BRANCH ) {tools/perf/util/machine.c +6
-6@@ -1379,15 +1379,15 @@ static int machine__resolve_callchain_sample(struct machine *machine,* - No annotations (should annotate somehow) */ -if (branch->nr >PERF_MAX_BRANCH_DEPTH ) { -pr_warning ("corrupted branch chain. skipping...\n" ); -return 0 ; - } - -if (callchain_param.branch_callstack) { +if (branch && callchain_param.branch_callstack) {int nr =min (max_stack, (int )branch->nr);struct branch_entry be[nr]; +if (branch->nr >PERF_MAX_BRANCH_DEPTH ) { +pr_warning ("corrupted branch chain. skipping...\n" ); +return 0 ; + } +for (i =0 ; i < nr; i++) {if (callchain_param.order ==ORDER_CALLEE ) { be[i] = branch->entries[i];
Re: [PATCH 2/9] perf, tools: Add --branch-history option to report v3
Quoted 6 lines
Quoted 6 lines
tools/perf/builtin-report.c +0
-0@@ -315,8 +315,9 @@ static int report__setup_sample_type(struct report *rep)return -EINVAL ;
Quoted 8 lines
tools/perf/builtin-report.c static int report__setup_sample_type(struct report *rep) · line 319
Quoted 6 lines
tools/perf/builtin-report.c +2
-2}if (symbol_conf.use_callchain) { -ui__error ("Selected -g but no callchain data. Did " -"you call 'perf record' without -g?\n" ); +ui__error ("Selected -g or --branch-history but no " + "callchain data. Did\n"An unwanted newline in the message.
It was intentional to make it fit onto a 80 column terminal
Quoted 37 lines
Quoted 37 lines
tools/perf/builtin-report.c +12
-2+ "you call 'perf record' without -g?\n");return -1 ; } } else if (!rep->dont_use_callchains &&@@ -631,6 +632,16 @@ parse_branch_mode(const struct option *opt __maybe_unused, }static int +parse_branch_call_mode (const struct option *opt__maybe_unused , +const char *str__maybe_unused ,int unset) +{ +int *branch_mode = opt->value; + + *branch_mode = !unset; +return 0 ; +} + +static int parse_percent_limit (const struct option *opt,const char *str,int unset__maybe_unused ) {@@ -645,7 +656,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)struct perf_session *session;struct stat st;bool has_br_stack = false; -int branch_mode =-1 ; +int branch_mode =-1 , branch_call_mode =-1 ;int ret =-1 ;char callchain_default_opt[] ="fractal,0.5,callee" ;const char *const report_usage[] = {@@ -754,7 +765,11 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)OPT_BOOLEAN (0 ,"group" , &symbol_conf.event_group,"Show event group information together" ), OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "", - "use branch records for histogram filling", parse_branch_mode),
Quoted 8 lines
tools/perf/builtin-report.c int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) · line 772
Quoted 5 lines
tools/perf/builtin-report.c +5
-0+ "use branch records for per branch histogram filling", + parse_branch_mode), + OPT_CALLBACK_NOOPT(0, "branch-history", &branch_call_mode, "", + "add last branch records to call history", + parse_branch_call_mode),Looks like it can be a boolean option, or at least can share
parse_branch_mode()callback.
No it can't. It's a tristate.
Quoted 9 lines
Quoted 9 lines
tools/perf/builtin-report.c +0
-0OPT_STRING (0 ,"objdump" , &objdump_path,"path" ,"objdump binary to use for disassembly and annotations" ), OPT_BOOLEAN(0, "demangle", &symbol_conf.demangle,@@ -804,8 +819,16 @@ repeat: has_br_stack =perf_header__has_feat (&session->header,HEADER_BRANCH_STACK );
Quoted 6 lines
tools/perf/builtin-report.c repeat: · line 824
Quoted 4 lines
tools/perf/builtin-report.c +2
-1-if (branch_mode ==-1 && has_br_stack) +if (branch_mode ==-1 && has_br_stack && branch_call_mode ==-1 ) sort__mode =SORT_MODE__BRANCH ; +if (branch_call_mode !=-1 ) {s/-1/1/ ?
-1 means not specified by the user.
-Andi
Re: [PATCH 2/9] perf, tools: Add --branch-history option to report v3
Quoted 15 lines · Andi Kleen
Quoted 6 lines
Quoted 6 lines
tools/perf/builtin-report.c +0
-0@@ -315,8 +315,9 @@ static int report__setup_sample_type(struct report *rep)return -EINVAL ;Quoted 8 lines
tools/perf/builtin-report.c static int report__setup_sample_type(struct report *rep) · line 319
Quoted 6 lines
tools/perf/builtin-report.c +2
-2}if (symbol_conf.use_callchain) { -ui__error ("Selected -g but no callchain data. Did " -"you call 'perf record' without -g?\n" ); +ui__error ("Selected -g or --branch-history but no " + "callchain data. Did\n"An unwanted newline in the message.
Quoted 1 line
It was intentional to make it fit onto a 80 column terminal
Did you mean '\n' after the "Did" or line break after "but no"? I meant the former is weird.. The 80 column rule (in the source code) is not applied to user messages AFAIK. And ui__error() also needs to handle line breaks to fit the message on the screen in a dynamic manner.
Quoted 56 lines
Quoted 37 lines
Quoted 37 lines
tools/perf/builtin-report.c +12
-2+ "you call 'perf record' without -g?\n");return -1 ; } } else if (!rep->dont_use_callchains &&@@ -631,6 +632,16 @@ parse_branch_mode(const struct option *opt __maybe_unused, }static int +parse_branch_call_mode (const struct option *opt__maybe_unused , +const char *str__maybe_unused ,int unset) +{ +int *branch_mode = opt->value; + + *branch_mode = !unset; +return 0 ; +} + +static int parse_percent_limit (const struct option *opt,const char *str,int unset__maybe_unused ) {@@ -645,7 +656,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)struct perf_session *session;struct stat st;bool has_br_stack = false; -int branch_mode =-1 ; +int branch_mode =-1 , branch_call_mode =-1 ;int ret =-1 ;char callchain_default_opt[] ="fractal,0.5,callee" ;const char *const report_usage[] = {@@ -754,7 +765,11 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)OPT_BOOLEAN (0 ,"group" , &symbol_conf.event_group,"Show event group information together" ), OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "", - "use branch records for histogram filling", parse_branch_mode),Quoted 8 lines
tools/perf/builtin-report.c int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) · line 772
Quoted 5 lines
tools/perf/builtin-report.c +5
-0+ "use branch records for per branch histogram filling", + parse_branch_mode), + OPT_CALLBACK_NOOPT(0, "branch-history", &branch_call_mode, "", + "add last branch records to call history", + parse_branch_call_mode),Looks like it can be a boolean option, or at least can share
parse_branch_mode()callback.No it can't. It's a tristate.
Quoted 9 lines
Quoted 9 lines
tools/perf/builtin-report.c +0
-0OPT_STRING (0 ,"objdump" , &objdump_path,"path" ,"objdump binary to use for disassembly and annotations" ), OPT_BOOLEAN(0, "demangle", &symbol_conf.demangle,@@ -804,8 +819,16 @@ repeat: has_br_stack =perf_header__has_feat (&session->header,HEADER_BRANCH_STACK );
Quoted 8 lines
Quoted 6 lines
tools/perf/builtin-report.c repeat: · line 824
Quoted 4 lines
tools/perf/builtin-report.c +2
-1-if (branch_mode ==-1 && has_br_stack) +if (branch_mode ==-1 && has_br_stack && branch_call_mode ==-1 ) sort__mode =SORT_MODE__BRANCH ; +if (branch_call_mode !=-1 ) {s/-1/1/ ?
-1 means not specified by the user.
But what about --no-branch-history?
Thanks,
Namhyung
PATCH 3/9 perf, tools: Enable printing the srcline in the history v4
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
For lbr-as-callgraph we need to see the line number in the history, because many LBR entries can be in a single function, and just showing the same function name many times is not useful.
When the history code is configured to sort by address, also try to resolve the address to a file:srcline and display this in the browser. If that doesn't work still display the address.
This can be also useful without LBRs for understanding which call in a large function (or in which inlined function) called something else.
Contains fixes from Namhyung Kim
v2: Refactor code into common function
v3: Fix GTK build
v4: Rebase
--- tools/perf/ui/browsers/hists.c | 17 ----------------- tools/perf/ui/gtk/hists.c | 11 +---------- tools/perf/ui/stdio/hist.c | 23 +++++++++-------------- tools/perf/util/callchain.c | 29 +++++++++++++++++++++++++++++ tools/perf/util/callchain.h | 5 +++++ tools/perf/util/machine.c | 2 +- tools/perf/util/srcline.c | 6 ++++-- 7 files changed, 49 insertions(+), 44 deletions(-)
tools/perf/ui/browsers/hists.c +0 -17
@@ -423,23 +423,6 @@ out:return key; } -static char *callchain_list__sym_name (struct callchain_list *cl, -char *bf,size_t bfsize,bool show_dso) -{ -int printed; - -if (cl->ms.sym) - printed =scnprintf (bf, bfsize,"%s" , cl->ms.sym->name); -else - printed =scnprintf (bf, bfsize,"%#" PRIx64, cl->ip); - -if (show_dso) -scnprintf (bf + printed, bfsize - printed," %s" , - cl->ms.map ? cl->ms.map->dso->short_name :"unknown" ); - -return bf; -} -#define LEVEL_OFFSET_STEP 3 static int hist_browser__show_callchain_node_rb_tree(struct hist_browser *browser,
tools/perf/ui/gtk/hists.c +1 -10
@@ -72,15 +72,6 @@ void perf_gtk__init_hpp(void) perf_gtk__hpp_color_overhead_guest_us; } -static void callchain_list__sym_name (struct callchain_list *cl, -char *bf,size_t bfsize) -{ -if (cl->ms.sym) -scnprintf (bf, bfsize,"%s" , cl->ms.sym->name); -else -scnprintf (bf, bfsize,"%#" PRIx64, cl->ip); -} -static void perf_gtk__add_callchain (struct rb_root *root,GtkTreeStore *store,GtkTreeIter *parent,int col,u64 total) {
@@ -111,7 +102,7 @@ static void perf_gtk__add_callchain(struct rb_root *root, GtkTreeStore *store,scnprintf (buf,sizeof (buf),"%5.2f%%" , percent);gtk_tree_store_set (store, &iter,0 , buf,-1 ); -callchain_list__sym_name (chain, buf,sizeof (buf)); +callchain_list__sym_name (chain, buf,sizeof (buf), false);gtk_tree_store_set (store, &iter, col, buf,-1 );if (need_new_parent) {
tools/perf/ui/stdio/hist.c +9 -14
@@ -41,6 +41,7 @@ static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain, {int i;size_t ret =0 ; +char bf[1024 ]; ret +=callchain__fprintf_left_margin (fp, left_margin);for (i =0 ; i < depth; i++) {
@@ -56,11 +57,8 @@ static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain, }else ret +=fprintf (fp,"%s" ," " ); } -if (chain->ms.sym) - ret +=fprintf (fp,"%s\n" , chain->ms.sym->name); -else - ret +=fprintf (fp,"0x%0" PRIx64"\n" , chain->ip); - +fputs (callchain_list__sym_name (chain, bf,sizeof (bf), false), fp); +fputc ('\n' , fp);return ret; }
@@ -168,6 +166,7 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,struct rb_node *node;int i =0 ;int ret =0 ; +char bf[1024 ];/* * If have one single callchain root, don't bother printing
@@ -195,10 +194,8 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root, }else ret +=callchain__fprintf_left_margin (fp, left_margin); -if (chain->ms.sym) - ret +=fprintf (fp," %s\n" , chain->ms.sym->name); -else - ret +=fprintf (fp," %p\n" , (void *)(long )chain->ip); + ret +=fprintf (fp,"%s\n" ,callchain_list__sym_name (chain, bf,sizeof (bf), + false));if (++entries_printed == callchain_param.print_limit)break ;
@@ -218,6 +215,7 @@ static size_t __callchain__fprintf_flat(FILE *fp, struct callchain_node *node, {struct callchain_list *chain;size_t ret =0 ; +char bf[1024 ];if (!node)return 0 ;
@@ -228,11 +226,8 @@ static size_t __callchain__fprintf_flat(FILE *fp, struct callchain_node *node,list_for_each_entry (chain, &node->val, list) {if (chain->ip >=PERF_CONTEXT_MAX )continue ; -if (chain->ms.sym) - ret +=fprintf (fp," %s\n" , chain->ms.sym->name); -else - ret +=fprintf (fp," %p\n" , - (void *)(long )chain->ip); + ret +=fprintf (fp," %s\n" ,callchain_list__sym_name (chain, + bf,sizeof (bf), false)); }return ret;
tools/perf/util/callchain.c +29 -0
@@ -636,3 +636,32 @@ int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *sampreturn 0 ;return callchain_append (he->callchain, &callchain_cursor, sample->period); } + +char *callchain_list__sym_name (struct callchain_list *cl, +char *bf,size_t bfsize,bool show_dso) +{ +int printed; + +if (cl->ms.sym) { +if (callchain_param.key ==CCKEY_ADDRESS && + cl->ms.map && !cl->srcline) + cl->srcline =get_srcline (cl->ms.map->dso, +map__rip_2objdump (cl->ms.map, + cl->ip)); +if (cl->srcline) + printed =scnprintf (bf, bfsize,"%s %s" , + cl->ms.sym->name, cl->srcline); +else + printed =scnprintf (bf, bfsize,"%s" , + cl->ms.sym->name); + }else + printed =scnprintf (bf, bfsize,"%#" PRIx64, cl->ip); + +if (show_dso) +scnprintf (bf + printed, bfsize - printed," %s" , + cl->ms.map ? + cl->ms.map->dso->short_name : +"unknown" ); + +return bf; +}
tools/perf/util/callchain.h +5 -0
@@ -66,6 +66,7 @@ struct callchain_param {struct callchain_list {u64 ip;struct map_symbol ms; +char *srcline;struct list_head list; };
@@ -166,4 +167,8 @@ int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *sampextern const char record_callchain_help[];int parse_callchain_report_opt (const char *arg); + +char *callchain_list__sym_name (struct callchain_list *cl, +char *bf,size_t bfsize,bool show_dso); +#endif /* __PERF_CALLCHAIN_H */
tools/perf/util/machine.c +1 -1
@@ -1308,7 +1308,7 @@ static int add_callchain_ip(struct machine *machine,return -EINVAL ; } -return callchain_cursor_append (&callchain_cursor, ip, al.map, al.sym); +return callchain_cursor_append (&callchain_cursor, al.addr, al.map, al.sym); }#define CHASHSZ 127
tools/perf/util/srcline.c +4 -2
@@ -258,7 +258,7 @@ char *get_srcline(struct dso *dso, unsigned long addr)const char *dso_name;if (!dso->has_srcline) -return SRCLINE_UNKNOWN ; + goto out;if (dso->symsrc_filename) dso_name = dso->symsrc_filename;
@@ -289,7 +289,9 @@ out: dso->has_srcline =0 ;dso__free_a2l (dso); } -return SRCLINE_UNKNOWN ; +if (asprintf (&srcline,"%s[%lx]" , dso->short_name, addr) <0 ) +return SRCLINE_UNKNOWN ; +return srcline; }void free_srcline (char *srcline)
Signature · 1 line
1.9.0
PATCH 4/9 perf, tools: Only print base source file for srcline
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
For perf report with --sort srcline only print the base source file name. This makes the results generally fit much better to the screen. The path is usually not that useful anyways because it is often from different systems.
--- tools/perf/util/srcline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/srcline.c +1 -1
@@ -274,7 +274,7 @@ char *get_srcline(struct dso *dso, unsigned long addr)if (!addr2line (dso_name, addr, &file, &line, dso)) goto out; -if (asprintf (&srcline,"%s:%u" , file, line) <0 ) { +if (asprintf (&srcline,"%s:%u" ,basename (file), line) <0 ) {free (file); goto out; }
Signature · 1 line
1.9.0
PATCH 5/9 perf, tools: Support source line numbers in annotate
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
With srcline key/sort'ing it's useful to have line numbers in the annotate window. This patch implements this.
Use objdump -l to request the line numbers and
save them in the line structure. Then the browser
displays them for source lines.
The line numbers are not displayed by default, but can be toggled on with 'k'
There is one unfortunate problem with this setup. For lines not containing source and which are outside functions objdump -l reports line numbers off by a few: it always reports the first line number in the next function even for lines that are outside the function.
I haven't found a nice way to detect/correct this. Probably objdump has to be fixed.
See https://sourceware.org/bugzilla/show_bug.cgi?id=16433
The line numbers are still useful even with these problems, as most are correct and the ones which are not are nearby.
--- tools/perf/ui/browsers/annotate.c | 13 ++++++++++++- tools/perf/util/annotate.c | 30 +++++++++++++++++++++++++----- tools/perf/util/annotate.h | 1 + 3 files changed, 38 insertions(+), 6 deletions(-)
tools/perf/ui/browsers/annotate.c +12 -1
@@ -27,6 +27,7 @@ static struct annotate_browser_opt {bool hide_src_code, use_offset, jump_arrows, + show_linenr, show_nr_jumps; } annotate_browser__opts = { .use_offset = true,
@@ -128,7 +129,11 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, intif (!*dl->line)slsmg_write_nstring (" " , width - pcnt_width);else if (dl->offset ==-1 ) { - printed =scnprintf (bf,sizeof (bf),"%*s " , +if (dl->line_nr && annotate_browser__opts.show_linenr) + printed =scnprintf (bf,sizeof (bf),"%*s %-5d " , + ab->addr_width," " , dl->line_nr); +else + printed =scnprintf (bf,sizeof (bf),"%*s " , ab->addr_width," " );slsmg_write_nstring (bf, printed);slsmg_write_nstring (dl->line, width - printed - pcnt_width +1 );
@@ -733,6 +738,7 @@ static int annotate_browser__run(struct annotate_browser *browser,"o Toggle disassembler output/simplified view\n" "s Toggle source code view\n" "/ Search string\n" +"k Toggle line numbers\n" "r Run available scripts\n" "? Search string backwards\n");continue ;
@@ -741,6 +747,10 @@ static int annotate_browser__run(struct annotate_browser *browser,script_browse (NULL );continue ; } +case 'k' : + annotate_browser__opts.show_linenr = + !annotate_browser__opts.show_linenr; +break ;case 'H' : nd = browser->curr_hot;break ;
@@ -984,6 +994,7 @@ static struct annotate_config { } annotate__configs[] = {ANNOTATE_CFG (hide_src_code),ANNOTATE_CFG (jump_arrows), +ANNOTATE_CFG (show_linenr),ANNOTATE_CFG (show_nr_jumps), ANNOTATE_CFG(use_offset), };
tools/perf/util/annotate.c +25 -5
@@ -17,11 +17,13 @@#include "debug.h" #include "annotate.h" #include "evsel.h" +#include <regex.h> #include <pthread.h> #include <linux/bitops.h> const char *disassembler_style;const char *objdump_path; +static regex_t file_lineno;static struct ins *ins__find (const char *name);static int disasm_line__parse (char *line,char **namep,char **rawp);
@@ -564,13 +566,15 @@ out_free_name:return -1 ; } -static struct disasm_line *disasm_line__new (s64 offset,char *line,size_t privsize) +static struct disasm_line *disasm_line__new (s64 offset,char *line, +size_t privsize,int line_nr) {struct disasm_line *dl =zalloc (sizeof (*dl) + privsize);if (dl !=NULL ) { dl->offset = offset; dl->line =strdup (line); + dl->line_nr = line_nr;if (dl->line ==NULL ) goto out_delete;
@@ -782,13 +786,15 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st* The ops.raw part will be parsed further according to type of the instruction. */ static int symbol__parse_objdump_line (struct symbol *sym,struct map *map, -FILE *file,size_t privsize) +FILE *file,size_t privsize, +int *line_nr) {struct annotation *notes =symbol__annotation (sym);struct disasm_line *dl;char *line =NULL , *parsed_line, *tmp, *tmp2, *c;size_t line_len;s64 line_ip, offset =-1 ; +regmatch_t match[2 ];if (getline (&line, &line_len, file) <0 )return -1 ;
@@ -806,6 +812,12 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map, line_ip =-1 ; parsed_line = line; +/* /filename:linenr ? Save line number and ignore. */ +if (regexec (&file_lineno, line,2 , match,0 ) ==0 ) { + *line_nr =atoi (line + match[1 ].rm_so); +return 0 ; + } +/* * Strip leading spaces: */
@@ -836,8 +848,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map, parsed_line = tmp2 +1 ; } - dl =disasm_line__new (offset, parsed_line, privsize); + dl =disasm_line__new (offset, parsed_line, privsize, *line_nr);free (line); + (*line_nr)++;if (dl ==NULL )return -1 ;
@@ -863,6 +876,11 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,return 0 ; } +static __attribute__((constructor))void symbol__init_regexpr (void ) +{ +regcomp (&file_lineno,"^/[^:]+:([0-9]+)$" ,REG_EXTENDED ); +} +static void delete_last_nop (struct symbol *sym) {struct annotation *notes =symbol__annotation (sym);
@@ -898,6 +916,7 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)char symfs_filename[PATH_MAX ];struct kcore_extract kce;bool delete_extract = false; +int lineno =0 ;if (filename) { snprintf(symfs_filename, sizeof(symfs_filename), "%s%s",
@@ -979,7 +998,7 @@ fallback: snprintf(command, sizeof(command), "%s %s%s --start-address=0x%016" PRIx64 " --stop-address=0x%016" PRIx64 - " -d %s %s -C %s 2>/dev/null|grep -v %s|expand", + " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand", objdump_path ? objdump_path : "objdump", disassembler_style ? "-M " : "", disassembler_style ? disassembler_style : "",
@@ -996,7 +1015,8 @@ fallback: goto out_free_filename;while (!feof (file)) -if (symbol__parse_objdump_line (sym, map, file, privsize) <0 ) +if (symbol__parse_objdump_line (sym, map, file, privsize, + &lineno) <0 )break ;/*
tools/perf/util/annotate.h +1 -0
@@ -58,6 +58,7 @@ struct disasm_line {char *line;char *name;struct ins *ins; +int line_nr;struct ins_operands ops; };
Signature · 1 line
1.9.0
PATCH 6/9 perf, tools: Fix srcline sort key output to use width
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
The srcline sort output ignored the width, which caused various problems with displaying srcline in the tui browser. Just cut it off at width.
--- tools/perf/util/sort.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/sort.c +1 -1
@@ -284,7 +284,7 @@ static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf, size_t size, unsigned int width __maybe_unused) { -return repsep_snprintf (bf, size,"%s" , he->srcline); +return repsep_snprintf (bf, size,"%.*s" , width, he->srcline); }struct sort_entry sort_srcline = {
Signature · 1 line
1.9.0
PATCH 7/9 tools, perf: Make get_srcline fall back to sym+offset
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
When the source line is not found fall back to sym + offset. This is generally much more useful than a raw address. For this we need to pass in the symbol from the caller. For some callers it's awkward to compute, so we stay at the old behaviour.
--- tools/perf/util/annotate.c | 2 +- tools/perf/util/callchain.c | 3 ++- tools/perf/util/map.c | 2 +- tools/perf/util/sort.c | 6 ++++-- tools/perf/util/srcline.c | 12 +++++++++--- tools/perf/util/util.h | 4 +++- 6 files changed, 20 insertions(+), 9 deletions(-)
tools/perf/util/annotate.c +1 -1
@@ -1187,7 +1187,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, goto next; offset = start + i; - src_line->path =get_srcline (map->dso, offset); + src_line->path =get_srcline (map->dso, offset,NULL , false);insert_source_line (&tmp_root, src_line); next:
tools/perf/util/callchain.c +2 -1
@@ -647,7 +647,8 @@ char *callchain_list__sym_name(struct callchain_list *cl, cl->ms.map && !cl->srcline) cl->srcline =get_srcline (cl->ms.map->dso,map__rip_2objdump (cl->ms.map, - cl->ip)); + cl->ip), + cl->ms.sym, false);if (cl->srcline) printed =scnprintf (bf, bfsize,"%s %s" , cl->ms.sym->name, cl->srcline);
tools/perf/util/map.c +1 -1
@@ -262,7 +262,7 @@ int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,if (map && map->dso) { srcline =get_srcline (map->dso, -map__rip_2objdump (map, addr)); +map__rip_2objdump (map, addr),NULL , true);if (srcline !=SRCLINE_UNKNOWN ) ret =fprintf (fp,"%s%s" , prefix, srcline);free_srcline (srcline);
tools/perf/util/sort.c +4 -2
@@ -265,7 +265,8 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)else {struct map *map = left->ms.map; left->srcline =get_srcline (map->dso, -map__rip_2objdump (map, left->ip)); +map__rip_2objdump (map, left->ip), + left->ms.sym, true); } }if (!right->srcline) {
@@ -274,7 +275,8 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)else {struct map *map = right->ms.map; right->srcline =get_srcline (map->dso, -map__rip_2objdump (map, right->ip)); +map__rip_2objdump (map, right->ip), + right->ms.sym, true); } }return strcmp (left->srcline, right->srcline);
tools/perf/util/srcline.c +9 -3
@@ -8,12 +8,13 @@#include "util/util.h" #include "util/debug.h" +#include "symbol.h" +#ifdef HAVE_LIBBFD_SUPPORT /* * Implement addr2line using libbfd. */ -#define PACKAGE "perf"#include <bfd.h> struct a2l_data {
@@ -250,7 +251,8 @@ void dso__free_a2l(struct dso *dso __maybe_unused)*/ #define A2L_FAIL_LIMIT 123 -char *get_srcline (struct dso *dso,unsigned long addr) +char *get_srcline (struct dso *dso,unsigned long addr,struct symbol *sym, +bool show_sym) {char *file =NULL ;unsigned line =0 ;
@@ -289,7 +291,11 @@ out: dso->has_srcline =0 ;dso__free_a2l (dso); } -if (asprintf (&srcline,"%s[%lx]" , dso->short_name, addr) <0 ) +if (sym) { +if (asprintf (&srcline,"%s+%ld" , show_sym ? sym->name :"" , + addr - sym->start) <0 ) +return SRCLINE_UNKNOWN ; + }else if (asprintf (&srcline,"%s[%lx]" , dso->short_name, addr) <0 )return SRCLINE_UNKNOWN ;return srcline; }
tools/perf/util/util.h +3 -1
@@ -317,8 +317,10 @@ unsigned long parse_tag_value(const char *str, struct parse_tag *tags);#define SRCLINE_UNKNOWN ((char *) "??:0")struct dso ; +struct symbol ; -char *get_srcline (struct dso *dso,unsigned long addr); +char *get_srcline (struct dso *dso,unsigned long addr,struct symbol *sym, +bool show_sym);void free_srcline (char *srcline);int filename__read_int (const char *filename,int *value);
Signature · 1 line
1.9.0
PATCH 8/9 tools, perf: Make srcline output address with -v
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
When -v is specified always print the hex address for the srcline.
--- tools/perf/util/srcline.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
tools/perf/util/srcline.c +15 -3
@@ -258,6 +258,12 @@ char *get_srcline(struct dso *dso, unsigned long addr, struct symbol *sym,unsigned line =0 ;char *srcline;const char *dso_name; +char astr[50 ]; + +if (verbose) +snprintf (astr,sizeof astr," %#lx" , addr); +else + astr[0 ] =0 ;if (!dso->has_srcline) goto out;
@@ -276,7 +282,12 @@ char *get_srcline(struct dso *dso, unsigned long addr, struct symbol *sym,if (!addr2line (dso_name, addr, &file, &line, dso)) goto out; -if (asprintf (&srcline,"%s:%u" ,basename (file), line) <0 ) { +if (line ==0 ) { +free (file); + goto fallback; + } + +if (asprintf (&srcline,"%s:%u%s" ,basename (file), line, astr) <0 ) {free (file); goto out; }
@@ -291,9 +302,10 @@ out: dso->has_srcline =0 ;dso__free_a2l (dso); } +fallback:if (sym) { -if (asprintf (&srcline,"%s+%ld" , show_sym ? sym->name :"" , - addr - sym->start) <0 ) +if (asprintf (&srcline,"%s+%ld%s" , show_sym ? sym->name :"" , + addr - sym->start, astr) <0 )return SRCLINE_UNKNOWN ; }else if (asprintf (&srcline,"%s[%lx]" , dso->short_name, addr) <0 )return SRCLINE_UNKNOWN ;
Signature · 1 line
1.9.0
PATCH 9/9 tools, perf: Add asprintf replacement
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
asprintf corrupts memory on some older glibc versions. Provide a replacement. This fixes various segfaults with --branch-history on older Fedoras.
--- tools/perf/Makefile.perf | 1 + tools/perf/util/asprintf.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 tools/perf/util/asprintf.c
tools/perf/Makefile.perf +1 -0
@@ -372,6 +372,7 @@ LIB_OBJS += $(OUTPUT)util/vdso.o LIB_OBJS += $(OUTPUT)util/stat.o LIB_OBJS += $(OUTPUT)util/record.o LIB_OBJS += $(OUTPUT)util/srcline.o +LIB_OBJS += $(OUTPUT)util/asprintf.o LIB_OBJS += $(OUTPUT)util/data.o LIB_OBJS += $(OUTPUT)ui/setup.o
tools/perf/util/asprintf.c new file +28 -0
@@ -0,0 +1,28 @@ +/* Replacement for asprintf as it's buggy in older glibc versions */ +#include <stdio.h> +#include <stdarg.h> +#include <stdlib.h> +#include <string.h> + +int vasprintf (char **str,const char *fmt,va_list ap) +{ +char buf[1024 ]; +int len =vsnprintf (buf,sizeof buf, fmt, ap); + + *str =malloc (len +1 ); +if (!*str) +return -1 ; +strcpy (*str, buf); +return len; +} + +int asprintf (char **str,const char *fmt, ...) +{ +va_list ap; +int ret; + +va_start (ap, fmt); + ret =vasprintf (str, fmt, ap); +va_end (ap); +return ret; +}
Signature · 1 line
1.9.0