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: 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; >
