Re: [PATCH 2/2] perf script: Support callindent

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Fri, May 20, 2016 at 12:52:18PM -0700, Andi Kleen escreveu:

> +static void print_sample_callindent(struct perf_sample *sample,
> +				    struct perf_evsel *evsel,
> +				    struct thread *thread,
> +				    struct addr_location *al)
> +{
> +	struct perf_event_attr *attr = &evsel->attr;
> +
> +	if (sample_addr_correlates_sym(attr)) {
> +		static struct call_return_processor *crp;
> +		struct addr_location addr_al;
> +		struct thread *main_thread;
> +		struct comm *comm;
> +
> +		if (!crp)
> +			crp = call_return_processor__new(dummy_call_return,
> +							 NULL);
> +		thread__resolve(thread, &addr_al, sample);
> +		main_thread = db_export__main_thread(al->machine, thread);


What is db_export doing here?

This is specific to machine or thread, you should have moved this from
being a static function in the db_export code to a function, probably:

  struct thread *thread__main(struct thread *thread, struct machine *machine)

And also bear in mind you have to drop the refcount that this function
returns, as follows:

> +		if (main_thread) {
> +			comm = machine__thread_exec_comm(al->machine,
> +							 main_thread);
			thread__put(main_thread);
		}
}

Also do this in a separate, prep patch before this one,

Thanks,

- Arnaldo

> +		thread_stack__process(thread, comm, sample, al, &addr_al,
> +							0, crp);
> +	}
> +	thread_stack__print_indent(thread);
> +}
> +
>  static void print_sample_bts(struct perf_sample *sample,
>  			     struct perf_evsel *evsel,
>  			     struct thread *thread,
> @@ -567,6 +609,9 @@ static void print_sample_bts(struct perf_sample *sample,
>  	struct perf_event_attr *attr = &evsel->attr;
>  	bool print_srcline_last = false;
>  
> +	if (PRINT_FIELD(CALLINDENT))
> +		print_sample_callindent(sample, evsel, thread, al);
> +
>  	/* print branch_from information */
>  	if (PRINT_FIELD(IP)) {
>  		unsigned int print_opts = output[attr->type].print_ip_opts;
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index 049438d51b9a..02f8255a0b01 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -231,7 +231,7 @@ int db_export__symbol(struct db_export *dbe, struct symbol *sym,
>  	return 0;
>  }
>  
> -static struct thread *get_main_thread(struct machine *machine, struct thread *thread)
> +struct thread *db_export__main_thread(struct machine *machine, struct thread *thread)
>  {
>  	if (thread->pid_ == thread->tid)
>  		return thread__get(thread);
> @@ -308,7 +308,7 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>  	if (err)
>  		return err;
>  
> -	main_thread = get_main_thread(al->machine, thread);
> +	main_thread = db_export__main_thread(al->machine, thread);
>  	if (main_thread)
>  		comm = machine__thread_exec_comm(al->machine, main_thread);
>  
> diff --git a/tools/perf/util/db-export.h b/tools/perf/util/db-export.h
> index 25e22fd76aca..8cb3658172d9 100644
> --- a/tools/perf/util/db-export.h
> +++ b/tools/perf/util/db-export.h
> @@ -103,4 +103,6 @@ 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);
>  
> +struct thread *db_export__main_thread(struct machine *machine, struct thread *thread);
> +
>  #endif
> diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
> index 679688e70ae7..abea25eb2ccc 100644
> --- a/tools/perf/util/thread-stack.c
> +++ b/tools/perf/util/thread-stack.c
> @@ -753,3 +753,10 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
>  
>  	return err;
>  }
> +
> +void thread_stack__print_indent(struct thread *thread)
> +{
> +	if (!thread->ts)
> +		return;
> +	printf("%*s", (int)thread->ts->cnt * 4, "");
> +}
> diff --git a/tools/perf/util/thread-stack.h b/tools/perf/util/thread-stack.h
> index e1528f1374c3..d7426bd0510b 100644
> --- a/tools/perf/util/thread-stack.h
> +++ b/tools/perf/util/thread-stack.h
> @@ -98,6 +98,7 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain,
>  			  size_t sz, u64 ip);
>  int thread_stack__flush(struct thread *thread);
>  void thread_stack__free(struct thread *thread);
> +void thread_stack__print_indent(struct thread *thread);
>  
>  struct call_return_processor *
>  call_return_processor__new(int (*process)(struct call_return *cr, void *data),
> -- 
> 2.5.5