Hi,
sample__fprintf_callchain currently did not use the already available
code to get the symbols of an inlined function if such information is
available in a dso. This patchset adds the required logic to add
appropriate lines.
As I am quite new to the code base of perf I am not sure how to test
that changeset in a correct way. At least the codes builds and the tools
that make use of sample__fprintf_callchain (perf-script, perf-trace and
perf-sched as far as I can see) did not fail to run - also I did not get
into the details of perf-trace and perf-sched as I have never used them
before.
Another thing I am not sure how to deal with are some warnings of
checkpatch.pl due to the 80 character line limit. Due to the long
function names in use the current implementation already exceeded that
limit in the same spots by even more characters as I have taken the
inner loop and put it into a separate function.
I hope to expand my work to perf-report but thought it might be usefull
to get already early feedback on those patches.
Thank you,
Jonas
Jonas Rabenstein (2):
perf evsel: split sample__fprintf_callchain in output and iteration
perf evsel: add inline functions to sample callchain output
tools/perf/util/evsel_fprintf.c | 157 ++++++++++++++++++++++----------
1 file changed, 109 insertions(+), 48 deletions(-)
--
2.19.2
Split the iteration over the callchain and the actual output of an
link in the callchain into separate functions. This allows to reuse
the output function in a follow up patch to add inline symbols to
the output.
Signed-off-by: Jonas Rabenstein <[email protected]>
---
tools/perf/util/evsel_fprintf.c | 111 ++++++++++++++++++--------------
1 file changed, 63 insertions(+), 48 deletions(-)
diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
index 95ea147f9e18..c710f687ddf4 100644
--- a/tools/perf/util/evsel_fprintf.c
+++ b/tools/perf/util/evsel_fprintf.c
@@ -99,12 +99,12 @@ int perf_evsel__fprintf(struct perf_evsel *evsel,
return ++printed;
}
-int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
- unsigned int print_opts, struct callchain_cursor *cursor,
- FILE *fp)
+static int __fprintf_callchain_link(u64 ip, struct map *map, struct symbol *symbol,
+ const char *srcline, bool first, int left_alignment,
+ unsigned int print_opts, FILE *fp)
{
+ u64 addr = 0;
int printed = 0;
- struct callchain_cursor_node *node;
int print_ip = print_opts & EVSEL__PRINT_IP;
int print_sym = print_opts & EVSEL__PRINT_SYM;
int print_dso = print_opts & EVSEL__PRINT_DSO;
@@ -115,63 +115,80 @@ int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
int print_arrow = print_opts & EVSEL__PRINT_CALLCHAIN_ARROW;
int print_skip_ignored = print_opts & EVSEL__PRINT_SKIP_IGNORED;
char s = print_oneline ? ' ' : '\t';
- bool first = true;
+ struct addr_location node_al;
- if (sample->callchain) {
- struct addr_location node_al;
- callchain_cursor_commit(cursor);
+ if (symbol && symbol->ignore && print_skip_ignored)
+ return 0;
- while (1) {
- u64 addr = 0;
+ printed += fprintf(fp, "%-*.*s", left_alignment, left_alignment, " ");
- node = callchain_cursor_current(cursor);
- if (!node)
- break;
+ if (print_arrow && !first)
+ printed += fprintf(fp, " <-");
- if (node->sym && node->sym->ignore && print_skip_ignored)
- goto next;
+ if (print_ip)
+ printed += fprintf(fp, "%c%16" PRIx64, s, ip);
- printed += fprintf(fp, "%-*.*s", left_alignment, left_alignment, " ");
+ if (map)
+ addr = map->map_ip(map, ip);
- if (print_arrow && !first)
- printed += fprintf(fp, " <-");
+ if (print_sym) {
+ printed += fprintf(fp, " ");
+ node_al.addr = addr;
+ node_al.map = map;
- if (print_ip)
- printed += fprintf(fp, "%c%16" PRIx64, s, node->ip);
+ if (print_symoffset) {
+ printed += __symbol__fprintf_symname_offs(symbol, &node_al,
+ print_unknown_as_addr,
+ true, fp);
+ } else {
+ printed += __symbol__fprintf_symname(symbol, &node_al,
+ print_unknown_as_addr,
+ fp);
+ }
+ }
- if (node->map)
- addr = node->map->map_ip(node->map, node->ip);
+ if (print_dso && (!symbol || !symbol->inlined)) {
+ printed += fprintf(fp, " (");
+ printed += map__fprintf_dsoname(map, fp);
+ printed += fprintf(fp, ")");
+ }
- if (print_sym) {
- printed += fprintf(fp, " ");
- node_al.addr = addr;
- node_al.map = node->map;
+ if (print_srcline && srcline)
+ printed += fprintf(fp, "\n %s", srcline);
+ else if (print_srcline)
+ printed += map__fprintf_srcline(map, addr, "\n ", fp);
- if (print_symoffset) {
- printed += __symbol__fprintf_symname_offs(node->sym, &node_al,
- print_unknown_as_addr,
- true, fp);
- } else {
- printed += __symbol__fprintf_symname(node->sym, &node_al,
- print_unknown_as_addr, fp);
- }
- }
+ if (symbol && symbol->inlined)
+ printed += fprintf(fp, " (inlined)");
- if (print_dso && (!node->sym || !node->sym->inlined)) {
- printed += fprintf(fp, " (");
- printed += map__fprintf_dsoname(node->map, fp);
- printed += fprintf(fp, ")");
- }
+ if (!print_oneline)
+ printed += fprintf(fp, "\n");
- if (print_srcline)
- printed += map__fprintf_srcline(node->map, addr, "\n ", fp);
+ return printed;
+}
+
+int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
+ unsigned int print_opts, struct callchain_cursor *cursor,
+ FILE *fp)
+{
+ int printed = 0;
+ struct callchain_cursor_node *node;
+
+ if (sample->callchain) {
+ callchain_cursor_commit(cursor);
+
+ while (1) {
+ node = callchain_cursor_current(cursor);
+ if (!node)
+ break;
- if (node->sym && node->sym->inlined)
- printed += fprintf(fp, " (inlined)");
- if (!print_oneline)
- printed += fprintf(fp, "\n");
+ printed += __fprintf_callchain_link(node->ip, node->map,
+ node->sym, NULL,
+ (printed == 0),
+ left_alignment,
+ print_opts, fp);
/* Add srccode here too? */
if (symbol_conf.bt_stop_list &&
@@ -181,8 +198,6 @@ int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
break;
}
- first = false;
-next:
callchain_cursor_advance(cursor);
}
}
--
2.19.2
Whenever a callchain shall be printed search for each address whether
inline information is available and add those symbols to the output
if symbol_conf.inline_name is enabled.
Signed-off-by: Jonas Rabenstein <[email protected]>
---
tools/perf/util/evsel_fprintf.c | 46 +++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
index c710f687ddf4..9f9ece453dc4 100644
--- a/tools/perf/util/evsel_fprintf.c
+++ b/tools/perf/util/evsel_fprintf.c
@@ -168,6 +168,49 @@ static int __fprintf_callchain_link(u64 ip, struct map *map, struct symbol *symb
return printed;
}
+static int __fprintf_callchain_inlines(struct callchain_cursor_node *node,
+ bool first, int left_alignment,
+ unsigned int print_opts, FILE *fp)
+{
+ int printed = 0;
+ struct inline_node *inline_node;
+ struct inline_list *ilist;
+ u64 addr;
+
+ if (!symbol_conf.inline_name)
+ return 0;
+
+ if (!node->map || !node->map->dso)
+ return 0;
+
+ addr = node->map->map_ip(node->map, node->ip);
+
+ inline_node = inlines__tree_find(&node->map->dso->inlined_nodes,
+ addr);
+ if (!inline_node) {
+ inline_node = dso__parse_addr_inlines(node->map->dso,
+ addr, node->sym);
+ if (!inline_node)
+ return 0;
+ inlines__tree_insert(&node->map->dso->inlined_nodes,
+ inline_node);
+ }
+
+ list_for_each_entry(ilist, &inline_node->val, list) {
+ if (ilist->symbol == node->sym)
+ break;
+
+ printed += __fprintf_callchain_link(node->ip, node->map,
+ ilist->symbol,
+ ilist->srcline,
+ first, left_alignment,
+ print_opts, fp);
+ first = (first && printed == 0);
+ }
+
+ return printed;
+}
+
int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
unsigned int print_opts, struct callchain_cursor *cursor,
FILE *fp)
@@ -183,6 +226,9 @@ int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
if (!node)
break;
+ printed += __fprintf_callchain_inlines(node, (printed == 0),
+ left_alignment,
+ print_opts, fp);
printed += __fprintf_callchain_link(node->ip, node->map,
node->sym, NULL,
--
2.19.2
Em Tue, Feb 19, 2019 at 07:38:08PM +0100, Jonas Rabenstein escreveu:
> Hi,
> sample__fprintf_callchain currently did not use the already available
> code to get the symbols of an inlined function if such information is
> available in a dso. This patchset adds the required logic to add
> appropriate lines.
You forgot to add the person that did lots of work in this area more
recently, Milian, which I'm adding here so that he have the opportunity
to comment on your work,
Thanks for working on this!
- Arnaldo
> As I am quite new to the code base of perf I am not sure how to test
> that changeset in a correct way. At least the codes builds and the tools
> that make use of sample__fprintf_callchain (perf-script, perf-trace and
> perf-sched as far as I can see) did not fail to run - also I did not get
> into the details of perf-trace and perf-sched as I have never used them
> before.
>
> Another thing I am not sure how to deal with are some warnings of
> checkpatch.pl due to the 80 character line limit. Due to the long
> function names in use the current implementation already exceeded that
> limit in the same spots by even more characters as I have taken the
> inner loop and put it into a separate function.
>
> I hope to expand my work to perf-report but thought it might be usefull
> to get already early feedback on those patches.
>
> Thank you,
> Jonas
>
> Jonas Rabenstein (2):
> perf evsel: split sample__fprintf_callchain in output and iteration
> perf evsel: add inline functions to sample callchain output
>
> tools/perf/util/evsel_fprintf.c | 157 ++++++++++++++++++++++----------
> 1 file changed, 109 insertions(+), 48 deletions(-)
>
> --
> 2.19.2
--
- Arnaldo
On Tue, Feb 19, 2019 at 07:38:08PM +0100, Jonas Rabenstein wrote:
> Hi,
> sample__fprintf_callchain currently did not use the already available
> code to get the symbols of an inlined function if such information is
> available in a dso. This patchset adds the required logic to add
> appropriate lines.
>
> As I am quite new to the code base of perf I am not sure how to test
> that changeset in a correct way. At least the codes builds and the tools
> that make use of sample__fprintf_callchain (perf-script, perf-trace and
> perf-sched as far as I can see) did not fail to run - also I did not get
> into the details of perf-trace and perf-sched as I have never used them
> before.
you could provide some examples that shows what u changed
I can see your changes change the perf script callchains,
displaying less or different callchains:
--- old 2019-02-20 15:29:34.872312007 +0100
+++ new 2019-02-20 15:30:47.784725456 +0100
@@ -23,10 +23,9 @@ yes 11807 203482.590491: 246238 cycl
40 [unknown] ([unknown])
yes 11807 203482.590654: 534592 cycles:uppp:
- 7f83e3c7fd0c _dl_addr+0x11c (/usr/lib64/libc-2.27.so)
+ 7f83e3c7fd0c __GI__dl_addr+0x11c (inlined)
yes 11807 203482.592371: 455976 cycles:uppp:
- 55ddec8a3f6e [unknown] (/usr/bin/yes)
a790a790a790a79 [unknown] ([unknown])
yes 11807 203482.594176: 326514 cycles:uppp:
@@ -38,11 +37,9 @@ yes 11807 203482.595464: 262867 cycl
a790a790a790a79 [unknown] ([unknown])
yes 11807 203482.596502: 214762 cycles:uppp:
- 55ddec8a3f6e [unknown] (/usr/bin/yes)
a790a790a790a79 [unknown] ([unknown])
>
> Another thing I am not sure how to deal with are some warnings of
> checkpatch.pl due to the 80 character line limit. Due to the long
> function names in use the current implementation already exceeded that
> limit in the same spots by even more characters as I have taken the
> inner loop and put it into a separate function.
don't worry much about the long lines warnings
>
> I hope to expand my work to perf-report but thought it might be usefull
> to get already early feedback on those patches.
yep, good idea
thanks,
jirka