Re: [PATCH 6/6] perf timechart: add backtrace support

From: Namhyung Kim
Date: Tue Oct 29 2013 - 04:42:04 EST


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[] = {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/