Em Fri, May 06, 2016 at 02:27:49PM +0300, Adrian Hunter escreveu: > On 28/04/16 11:19, Chris Phlipot wrote: > > This change enables the db export api to export callchains. This > > is accomplished by adding callchains obtained from samples to the > > call_path_root structure and exporting them via the current call path > > export API. > > > > While the current API does support exporting call paths, this is not > > supported when sampling. This commit addresses that missing feature by > > allowing the export of call paths when callchains are present in samples. > > > > Summary: > > -This feature is activated by initializing the call_path_root member > > inside the db_export structure to a non-null value. > > -Callchains are resolved with thread__resolve_callchain() and then stored > > and exported by adding a call path under call path root. > > -Symbol and DSO for each callchain node are exported via db_ids_from_al() > > > > This commit puts in place infrastructure to be used by subsequent commits, > > and by itself, does not introduce any user-visible changes. > > > > Signed-off-by: Chris Phlipot <[email protected]> > > Looks good. A few of nitpicks below. But apart from those:
Agreed, will fix those. - Arnaldo > Acked-by: Adrian Hunter <[email protected]> > > > --- > > tools/perf/util/db-export.c | 86 > > +++++++++++++++++++++++++++++++++++++++++++++ > > tools/perf/util/db-export.h | 2 ++ > > 2 files changed, 88 insertions(+) > > > > diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c > > index 4fc607c..cb96591 100644 > > --- a/tools/perf/util/db-export.c > > +++ b/tools/perf/util/db-export.c > > @@ -15,6 +15,8 @@ > > > > #include <errno.h> > > > > +#include <sys/types.h> > > Why? > > > + > > #include "evsel.h" > > #include "machine.h" > > #include "thread.h" > > @@ -23,6 +25,7 @@ > > #include "event.h" > > #include "util.h" > > #include "thread-stack.h" > > +#include "callchain.h" > > #include "call-path.h" > > #include "db-export.h" > > > > @@ -277,6 +280,81 @@ static int db_ids_from_al(struct db_export *dbe, > > struct addr_location *al, > > return 0; > > } > > > > +static struct call_path *call_path_from_sample(struct db_export *dbe, > > + struct machine *machine, > > + struct thread *thread, > > + struct perf_sample *sample, > > + struct perf_evsel *evsel > > + ) > > +{ > > + u64 kernel_start = machine__kernel_start(machine); > > + > > + struct call_path *current = &dbe->cpr->call_path; > > + enum chain_order saved_order = callchain_param.order; > > + > > + int err = 0; > > err needn't be initialized. And remove blank lines in the group of local > declarations. > > > + > > + if (!symbol_conf.use_callchain || !sample->callchain) > > + return NULL; > > + > > + /* Since the call path tree must be built starting with the root, we > > Normal kernel comment style has "/*" on separate line. > > > + * must use ORDER_CALL for call chain resolution, in order to process > > + * the callchain starting with the root node and ending with the leaf. > > + */ > > + callchain_param.order = ORDER_CALLER; > > + err = thread__resolve_callchain(thread, &callchain_cursor, evsel, > > + sample, NULL, NULL, > > + PERF_MAX_STACK_DEPTH); > > Should probably be sysctl_perf_event_max_stack not PERF_MAX_STACK_DEPTH > > > + if (err) { > > + callchain_param.order = saved_order; > > + return NULL; > > + } > > + callchain_cursor_commit(&callchain_cursor); > > + > > + while (1) { > > + struct callchain_cursor_node *node; > > + struct addr_location al; > > + u64 dso_db_id = 0, sym_db_id = 0, offset = 0; > > + > > + memset(&al, 0, sizeof(al)); > > + > > + node = callchain_cursor_current(&callchain_cursor); > > + if (!node) > > + break; > > + > > + /* handle export of symbol and dso for this node by > > Normal kernel comment style has "/*" on separate line. > > > + * constructing an addr_location struct and then passing it to > > + * db_ids_from_al() to perform the export. > > + */ > > + al.sym = node->sym; > > + al.map = node->map; > > + al.machine = machine; > > + if (al.map) > > + al.addr = al.map->map_ip(al.map, node->ip); > > + else > > + al.addr = node->ip; > > + > > + db_ids_from_al(dbe, &al, &dso_db_id, &sym_db_id, &offset); > > + > > + /* add node to the call path tree if it doesn't exist */ > > + current = call_path__findnew(dbe->cpr, current, > > + al.sym, node->ip, > > + kernel_start); > > + > > + callchain_cursor_advance(&callchain_cursor); > > + } > > + > > + /* Reset the callchain order to its prior value. */ > > + callchain_param.order = saved_order; > > + > > + if (current == &dbe->cpr->call_path) { > > + /* Bail because the callchain was empty. */ > > + return NULL; > > + } > > + > > + return current; > > +} > > + > > int db_export__branch_type(struct db_export *dbe, u32 branch_type, > > const char *name) > > { > > @@ -330,6 +408,14 @@ int db_export__sample(struct db_export *dbe, union > > perf_event *event, > > if (err) > > goto out_put; > > > > + if (dbe->cpr) { > > + struct call_path *cp = call_path_from_sample(dbe, al->machine, > > + thread, sample, > > + evsel); > > + if (cp) > > + db_export__call_path(dbe, cp); > > + } > > + > > if ((evsel->attr.sample_type & PERF_SAMPLE_ADDR) && > > sample_addr_correlates_sym(&evsel->attr)) { > > struct addr_location addr_al; > > diff --git a/tools/perf/util/db-export.h b/tools/perf/util/db-export.h > > index 25e22fd..f5daf55 100644 > > --- a/tools/perf/util/db-export.h > > +++ b/tools/perf/util/db-export.h > > @@ -27,6 +27,7 @@ struct dso; > > struct perf_sample; > > struct addr_location; > > struct call_return_processor; > > +struct call_path_root; > > struct call_path; > > struct call_return; > > > > @@ -64,6 +65,7 @@ struct db_export { > > int (*export_call_return)(struct db_export *dbe, > > struct call_return *cr); > > struct call_return_processor *crp; > > + struct call_path_root *cpr; > > u64 evsel_last_db_id; > > u64 machine_last_db_id; > > u64 thread_last_db_id; > >
