Re: [PATCH 1/8] perf db-export: Add calls parent_id

From: Arnaldo Carvalho de Melo
Date: Fri Mar 01 2019 - 12:52:09 EST

Next message: Will Deacon: "Will is away until April"
Previous message: Arnaldo Carvalho de Melo: "Re: [PATCH] perf intel-pt: Fix divide by zero"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Thu, Feb 28, 2019 at 03:00:24PM +0200, Adrian Hunter escreveu:
> The call_path can be used to find the parent symbol for a call but not the
> exact parent call. To do that add parent_id to the call_return export. This
> enables the creation of a call tree from the exported data.

Thanks, applied.

- Arnaldo

> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
> tools/perf/util/db-export.c | 15 ++++++++++-----
> tools/perf/util/db-export.h | 3 ++-
> .../util/scripting-engines/trace-event-python.c | 8 +++++---
> tools/perf/util/thread-stack.c | 16 ++++++++++++++--
> tools/perf/util/thread-stack.h | 6 ++++--
> 5 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index de9b4769d06c..d7315a00c731 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -510,18 +510,23 @@ int db_export__call_path(struct db_export *dbe, struct call_path *cp)
> return 0;
> }
>
> -int db_export__call_return(struct db_export *dbe, struct call_return *cr)
> +int db_export__call_return(struct db_export *dbe, struct call_return *cr,
> + u64 *parent_db_id)
> {
> int err;
>
> - if (cr->db_id)
> - return 0;
> -
> err = db_export__call_path(dbe, cr->cp);
> if (err)
> return err;
>
> - cr->db_id = ++dbe->call_return_last_db_id;
> + if (!cr->db_id)
> + cr->db_id = ++dbe->call_return_last_db_id;
> +
> + if (parent_db_id) {
> + if (!*parent_db_id)
> + *parent_db_id = ++dbe->call_return_last_db_id;
> + cr->parent_db_id = *parent_db_id;
> + }
>
> if (dbe->export_call_return)
> return dbe->export_call_return(dbe, cr);
> diff --git a/tools/perf/util/db-export.h b/tools/perf/util/db-export.h
> index 67bc6b8ad2d6..4e2424c89df9 100644
> --- a/tools/perf/util/db-export.h
> +++ b/tools/perf/util/db-export.h
> @@ -104,6 +104,7 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
> int db_export__branch_types(struct db_export *dbe);
>
> int db_export__call_path(struct db_export *dbe, struct call_path *cp);
> -int db_export__call_return(struct db_export *dbe, struct call_return *cr);
> +int db_export__call_return(struct db_export *dbe, struct call_return *cr,
> + u64 *parent_db_id);
>
> #endif
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 0e17db41b49b..09604c6508f0 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -1173,7 +1173,7 @@ static int python_export_call_return(struct db_export *dbe,
> u64 comm_db_id = cr->comm ? cr->comm->db_id : 0;
> PyObject *t;
>
> - t = tuple_new(11);
> + t = tuple_new(12);
>
> tuple_set_u64(t, 0, cr->db_id);
> tuple_set_u64(t, 1, cr->thread->db_id);
> @@ -1186,6 +1186,7 @@ static int python_export_call_return(struct db_export *dbe,
> tuple_set_u64(t, 8, cr->return_ref);
> tuple_set_u64(t, 9, cr->cp->parent->db_id);
> tuple_set_s32(t, 10, cr->flags);
> + tuple_set_u64(t, 11, cr->parent_db_id);
>
> call_object(tables->call_return_handler, t, "call_return_table");
>
> @@ -1194,11 +1195,12 @@ static int python_export_call_return(struct db_export *dbe,
> return 0;
> }
>
> -static int python_process_call_return(struct call_return *cr, void *data)
> +static int python_process_call_return(struct call_return *cr, u64 *parent_db_id,
> + void *data)
> {
> struct db_export *dbe = data;
>
> - return db_export__call_return(dbe, cr);
> + return db_export__call_return(dbe, cr, parent_db_id);
> }
>
> static void python_process_general_event(struct perf_sample *sample,
> diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
> index a8b45168513c..41942c2aaa18 100644
> --- a/tools/perf/util/thread-stack.c
> +++ b/tools/perf/util/thread-stack.c
> @@ -49,6 +49,7 @@ enum retpoline_state_t {
> * @timestamp: timestamp (if known)
> * @ref: external reference (e.g. db_id of sample)
> * @branch_count: the branch count when the entry was created
> + * @db_id: id used for db-export
> * @cp: call path
> * @no_call: a 'call' was not seen
> * @trace_end: a 'call' but trace ended
> @@ -59,6 +60,7 @@ struct thread_stack_entry {
> u64 timestamp;
> u64 ref;
> u64 branch_count;
> + u64 db_id;
> struct call_path *cp;
> bool no_call;
> bool trace_end;
> @@ -280,12 +282,14 @@ static int thread_stack__call_return(struct thread *thread,
> .comm = ts->comm,
> .db_id = 0,
> };
> + u64 *parent_db_id;
>
> tse = &ts->stack[idx];
> cr.cp = tse->cp;
> cr.call_time = tse->timestamp;
> cr.return_time = timestamp;
> cr.branch_count = ts->branch_count - tse->branch_count;
> + cr.db_id = tse->db_id;
> cr.call_ref = tse->ref;
> cr.return_ref = ref;
> if (tse->no_call)
> @@ -295,7 +299,14 @@ static int thread_stack__call_return(struct thread *thread,
> if (tse->non_call)
> cr.flags |= CALL_RETURN_NON_CALL;
>
> - return crp->process(&cr, crp->data);
> + /*
> + * The parent db_id must be assigned before exporting the child. Note
> + * it is not possible to export the parent first because its information
> + * is not yet complete because its 'return' has not yet been processed.
> + */
> + parent_db_id = idx ? &(tse - 1)->db_id : NULL;
> +
> + return crp->process(&cr, parent_db_id, crp->data);
> }
>
> static int __thread_stack__flush(struct thread *thread, struct thread_stack *ts)
> @@ -484,7 +495,7 @@ void thread_stack__sample(struct thread *thread, int cpu,
> }
>
> struct call_return_processor *
> -call_return_processor__new(int (*process)(struct call_return *cr, void *data),
> +call_return_processor__new(int (*process)(struct call_return *cr, u64 *parent_db_id, void *data),
> void *data)
> {
> struct call_return_processor *crp;
> @@ -537,6 +548,7 @@ static int thread_stack__push_cp(struct thread_stack *ts, u64 ret_addr,
> tse->no_call = no_call;
> tse->trace_end = trace_end;
> tse->non_call = false;
> + tse->db_id = 0;
>
> return 0;
> }
> diff --git a/tools/perf/util/thread-stack.h b/tools/perf/util/thread-stack.h
> index b7c04e19ad41..9c45f947f5a9 100644
> --- a/tools/perf/util/thread-stack.h
> +++ b/tools/perf/util/thread-stack.h
> @@ -55,6 +55,7 @@ enum {
> * @call_ref: external reference to 'call' sample (e.g. db_id)
> * @return_ref: external reference to 'return' sample (e.g. db_id)
> * @db_id: id used for db-export
> + * @parent_db_id: id of parent call used for db-export
> * @flags: Call/Return flags
> */
> struct call_return {
> @@ -67,6 +68,7 @@ struct call_return {
> u64 call_ref;
> u64 return_ref;
> u64 db_id;
> + u64 parent_db_id;
> u32 flags;
> };
>
> @@ -79,7 +81,7 @@ struct call_return {
> */
> struct call_return_processor {
> struct call_path_root *cpr;
> - int (*process)(struct call_return *cr, void *data);
> + int (*process)(struct call_return *cr, u64 *parent_db_id, void *data);
> void *data;
> };
>
> @@ -93,7 +95,7 @@ void thread_stack__free(struct thread *thread);
> size_t thread_stack__depth(struct thread *thread, int cpu);
>
> struct call_return_processor *
> -call_return_processor__new(int (*process)(struct call_return *cr, void *data),
> +call_return_processor__new(int (*process)(struct call_return *cr, u64 *parent_db_id, void *data),
> void *data);
> void call_return_processor__free(struct call_return_processor *crp);
> int thread_stack__process(struct thread *thread, struct comm *comm,
> --
> 2.17.1

--

- Arnaldo

Next message: Will Deacon: "Will is away until April"
Previous message: Arnaldo Carvalho de Melo: "Re: [PATCH] perf intel-pt: Fix divide by zero"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]