Messages in this thread | | | From | Namhyung Kim <> | Subject | Re: [PATCH 6/6] perf timechart: add backtrace support | Date | Tue, 29 Oct 2013 17:41:54 +0900 |
| |
On Tue, 22 Oct 2013 14:56:52 +0400, Stanislav Fomichev wrote: > Add -g flag to `perf timechart record` which saves callchain info in > the perf.data. > When generating SVG, add backtrace information to the figure details, > so now it's possible to see which code path woke up the task and why some > task went to sleep.
[SNIP]
> +static const char *cat_backtrace(union perf_event *event, > + struct perf_sample *sample, > + struct machine *machine) > +{ > + struct addr_location al; > + unsigned int i; > + char *p = NULL; > + size_t p_len; > + u8 cpumode = PERF_RECORD_MISC_USER; > + struct addr_location tal; > + struct ip_callchain *chain = sample->callchain; > + FILE *f = open_memstream(&p, &p_len);
It is safer to check the return value.
> + > + if (!chain) > + return NULL; > + > + if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) { > + fprintf(stderr, "problem processing %d event, skipping it.\n", > + event->header.type); > + return NULL; > + }
Above two returns should close 'f'.
> + > + for (i = 0; i < chain->nr; i++) { > + u64 ip; > + > + if (callchain_param.order == ORDER_CALLEE) > + ip = chain->ips[i]; > + else > + ip = chain->ips[chain->nr - i - 1];
The ip might carry context information rather than the actual instruction pointer. Please see machine__resolve_callchain_sample().
> + > + tal.filtered = false; > + thread__find_addr_location(al.thread, machine, cpumode, > + MAP__FUNCTION, ip, &tal); > + > + if (tal.sym) > + fprintf(f, "..... %016" PRIx64 " %s\n", ip, > + tal.sym->name); > + else > + fprintf(f, "..... %016" PRIx64 "\n", ip); > + } > + > + fclose(f); > + > + return p; > +} > + > typedef int (*tracepoint_handler)(struct perf_evsel *evsel, > - struct perf_sample *sample); > + struct perf_sample *sample, > + const char *backtrace); > > static int process_sample_event(struct perf_tool *tool __maybe_unused, > union perf_event *event __maybe_unused, > @@ -485,7 +545,8 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused, > > if (evsel->handler.func != NULL) { > tracepoint_handler f = evsel->handler.func; > - return f(evsel, sample); > + return f(evsel, sample, > + cat_backtrace(event, sample, machine)); > } > > return 0; > @@ -493,7 +554,8 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused, > > static int > process_sample_cpu_idle(struct perf_evsel *evsel __maybe_unused, > - struct perf_sample *sample) > + struct perf_sample *sample, > + const char *backtrace __maybe_unused) > { > struct power_processor_entry *ppe = sample->raw_data; > > @@ -506,7 +568,8 @@ process_sample_cpu_idle(struct perf_evsel *evsel __maybe_unused, > > static int > process_sample_cpu_frequency(struct perf_evsel *evsel __maybe_unused, > - struct perf_sample *sample) > + struct perf_sample *sample, > + const char *backtrace __maybe_unused) > { > struct power_processor_entry *ppe = sample->raw_data; > > @@ -516,28 +579,31 @@ process_sample_cpu_frequency(struct perf_evsel *evsel __maybe_unused, > > static int > process_sample_sched_wakeup(struct perf_evsel *evsel __maybe_unused, > - struct perf_sample *sample) > + struct perf_sample *sample, > + const char *backtrace __maybe_unused)
You don't need to add __maybe_unused if it's used. :)
> { > struct trace_entry *te = sample->raw_data; > > - sched_wakeup(sample->cpu, sample->time, sample->pid, te); > + sched_wakeup(sample->cpu, sample->time, sample->pid, te, backtrace); > return 0; > } > [SNIP]
> @@ -1166,6 +1255,7 @@ int cmd_timechart(int argc, const char **argv, > "record power data only", parse_power), > OPT_CALLBACK_NOOPT('T', "tasks-only", NULL, NULL, > "record processes data only", parse_tasks), > + OPT_BOOLEAN('g', "callchain", &with_backtrace, "record callchain"),
Please update the doc also.
Thanks, Namhyung
> OPT_END() > }; > const char * const record_usage[] = {
| |