| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 6 May 2016 10:07:12 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Chris Phlipot <cphlipot0@...il.com>, jolsa@...nel.org,
peterz@...radead.org, mingo@...hat.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] perf script: enable db export to output sampled
callchains
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 <cphlipot0@...il.com>
>
> Looks good. A few of nitpicks below. But apart from those:
Agreed, will fix those.
- Arnaldo
> Acked-by: Adrian Hunter <adrian.hunter@...el.com>
>
> > ---
> > 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;
> >
Powered by blists - more mailing lists