2011-03-09 18:31:55

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 00/10] perf/core fixes and improvements

Hi Ingo,

Please consider pulling from:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core

Regards,

- Arnaldo

Arnaldo Carvalho de Melo (3):
perf session: Simplify evlist creation from perf.data header
perf evsel: Assume rest of perf_header_attr functions
perf header: Stop using 'self'

David Ahern (5):
perf script: Change process_event prototype
perf script: Move printing of 'common' data from print_event and rename
perf script: Support custom field selection for output
perf script: Add support for dumping symbols
perf script: Add support for H/W and S/W events

Jiri Olsa (2):
perf top: Fix events overflow in top command
perf top: Don't let events to eat up whole header line

tools/perf/Documentation/perf-script.txt | 22 +
tools/perf/builtin-record.c | 94 ++---
tools/perf/builtin-report.c | 4 +-
tools/perf/builtin-script.c | 295 +++++++++++++--
tools/perf/util/evlist.c | 26 +-
tools/perf/util/evlist.h | 4 +-
tools/perf/util/evsel.c | 21 +-
tools/perf/util/evsel.h | 9 +-
tools/perf/util/header.c | 422 +++++++++-----------
tools/perf/util/header.h | 48 +--
tools/perf/util/parse-events.c | 22 +
tools/perf/util/parse-events.h | 1 +
.../perf/util/scripting-engines/trace-event-perl.c | 11 +-
.../util/scripting-engines/trace-event-python.c | 11 +-
tools/perf/util/session.c | 124 +++---
tools/perf/util/session.h | 4 +
tools/perf/util/top.c | 48 ++-
tools/perf/util/trace-event-parse.c | 106 +-----
tools/perf/util/trace-event-scripting.c | 9 +-
tools/perf/util/trace-event.h | 10 +-
20 files changed, 730 insertions(+), 561 deletions(-)


2011-03-09 18:31:46

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 04/10] perf top: Fix events overflow in top command

From: Jiri Olsa <[email protected]>

The snprintf function returns number of printed characters even if it
cross the size parameter. So passing enough events via '-e' parameter
will cause segmentation fault.

It's reproduced by following command:

perf top -e `perf list | grep Tracepoint | awk -F'[' '\
{gsub(/[[:space:]]+/,"",$1);array[FNR]=$1}END{outputs=array[1];\
for (i=2;i<=FNR;i++){ outputs=outputs "," array[i];};print outputs}'`

Attached patch is adding SNPRINTF macro that provides the overflow check
and returns actuall number of printed characters.

Reported-by: Han Pingtian <[email protected]>
Cc: Han Pingtian <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/top.c | 30 ++++++++++++++++++------------
1 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index 70a9c13..4f869da 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -61,6 +61,12 @@ static void rb_insert_active_sym(struct rb_root *tree, struct sym_entry *se)
rb_insert_color(&se->rb_node, tree);
}

+#define SNPRINTF(buf, size, fmt, args...) \
+({ \
+ size_t r = snprintf(buf, size, fmt, ## args); \
+ r > size ? size : r; \
+})
+
size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
{
struct perf_evsel *counter;
@@ -70,7 +76,7 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
size_t ret = 0;

if (!perf_guest) {
- ret = snprintf(bf, size,
+ ret = SNPRINTF(bf, size,
" PerfTop:%8.0f irqs/sec kernel:%4.1f%%"
" exact: %4.1f%% [", samples_per_sec,
100.0 - (100.0 * ((samples_per_sec - ksamples_per_sec) /
@@ -81,7 +87,7 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
float guest_kernel_samples_per_sec = top->guest_kernel_samples / top->delay_secs;
float guest_us_samples_per_sec = top->guest_us_samples / top->delay_secs;

- ret = snprintf(bf, size,
+ ret = SNPRINTF(bf, size,
" PerfTop:%8.0f irqs/sec kernel:%4.1f%% us:%4.1f%%"
" guest kernel:%4.1f%% guest us:%4.1f%%"
" exact: %4.1f%% [", samples_per_sec,
@@ -101,38 +107,38 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
if (top->evlist->nr_entries == 1 || !top->display_weighted) {
struct perf_evsel *first;
first = list_entry(top->evlist->entries.next, struct perf_evsel, node);
- ret += snprintf(bf + ret, size - ret, "%" PRIu64 "%s ",
+ ret += SNPRINTF(bf + ret, size - ret, "%" PRIu64 "%s ",
(uint64_t)first->attr.sample_period,
top->freq ? "Hz" : "");
}

if (!top->display_weighted) {
- ret += snprintf(bf + ret, size - ret, "%s",
+ ret += SNPRINTF(bf + ret, size - ret, "%s",
event_name(top->sym_evsel));
} else list_for_each_entry(counter, &top->evlist->entries, node) {
- ret += snprintf(bf + ret, size - ret, "%s%s",
+ ret += SNPRINTF(bf + ret, size - ret, "%s%s",
counter->idx ? "/" : "", event_name(counter));
}

- ret += snprintf(bf + ret, size - ret, "], ");
+ ret += SNPRINTF(bf + ret, size - ret, "], ");

if (top->target_pid != -1)
- ret += snprintf(bf + ret, size - ret, " (target_pid: %d",
+ ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %d",
top->target_pid);
else if (top->target_tid != -1)
- ret += snprintf(bf + ret, size - ret, " (target_tid: %d",
+ ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %d",
top->target_tid);
else
- ret += snprintf(bf + ret, size - ret, " (all");
+ ret += SNPRINTF(bf + ret, size - ret, " (all");

if (top->cpu_list)
- ret += snprintf(bf + ret, size - ret, ", CPU%s: %s)",
+ ret += SNPRINTF(bf + ret, size - ret, ", CPU%s: %s)",
top->evlist->cpus->nr > 1 ? "s" : "", top->cpu_list);
else {
if (top->target_tid != -1)
- ret += snprintf(bf + ret, size - ret, ")");
+ ret += SNPRINTF(bf + ret, size - ret, ")");
else
- ret += snprintf(bf + ret, size - ret, ", %d CPU%s)",
+ ret += SNPRINTF(bf + ret, size - ret, ", %d CPU%s)",
top->evlist->cpus->nr,
top->evlist->cpus->nr > 1 ? "s" : "");
}
--
1.6.2.5

2011-03-09 18:31:52

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 10/10] perf script: Add support for H/W and S/W events

From: David Ahern <[email protected]>

Custom fields set for each type by prepending field argument with type. For
file with multiple event types (e.g., trace and S/W) display of an event type
suppressed by setting output fields to "".

e.g.,
perf record -ga -e sched:sched_switch -e cpu-clock -c 10000000 -R -- sleep 1
perf script

openssl 11496 [000] 9711.807107: cpu-clock-msecs:
ffffffff810c22dc arch_local_irq_restore ([kernel.kallsyms])
ffffffff810c518c __alloc_pages_nodemask ([kernel.kallsyms])
ffffffff810297b2 pte_alloc_one ([kernel.kallsyms])
ffffffff810d8b98 __pte_alloc ([kernel.kallsyms])
ffffffff810daf07 handle_mm_fault ([kernel.kallsyms])
ffffffff8138763a do_page_fault ([kernel.kallsyms])
ffffffff81384a65 page_fault ([kernel.kallsyms])
7f6130507d70 asn1_check_tlen (/lib64/libcrypto.so.1.0.0c)
0 ()

openssl 11496 [000] 9711.808042: sched_switch: prev_comm=openssl ...
kworker/0:0 4 [000] 9711.808067: sched_switch: prev_comm=kworker/...
swapper 0 [001] 9711.808090: sched_switch: prev_comm=kworker/...
sshd 11451 [001] 9711.808185: sched_switch: prev_comm=sshd pre...
swapper 0 [001] 9711.816155: cpu-clock-msecs:
ffffffff81023609 native_safe_halt ([kernel.kallsyms])
ffffffff8100132a cpu_idle ([kernel.kallsyms])
ffffffff8137cf9b start_secondary ([kernel.kallsyms])

openssl 11496 [000] 9711.817104: cpu-clock-msecs:
7f61304ad723 AES_cbc_encrypt (/lib64/libcrypto.so.1.0.0c)
7fff3402f950 ()
12f0debc9a785634 ()

swapper 0 [001] 9711.826155: cpu-clock-msecs:
ffffffff81023609 native_safe_halt ([kernel.kallsyms])
ffffffff8100132a cpu_idle ([kernel.kallsyms])
ffffffff8137cf9b start_secondary ([kernel.kallsyms])

To suppress trace events within the file and use default output for S/W events:
perf script -f trace:

or to suppress S/W events and do default display for trace events:
perf script -f sw:

Custom field selections:
perf script -f sw:comm,tid,time -f trace:time,trace

openssl 11496 9711.797162:
swapper 0 9711.807071:
openssl 11496 9711.807107:
9711.808042: prev_comm=openssl prev_pid=11496 prev_prio=120 prev_state=R ...
9711.808067: prev_comm=kworker/0:0 prev_pid=4 prev_prio=120 prev_state=S ...
9711.808090: prev_comm=kworker/0:0 prev_pid=0 prev_prio=120 prev_state=R ...
9711.808185: prev_comm=sshd prev_pid=11451 prev_prio=120 prev_state=S ==>...
swapper 0 9711.816155:
openssl 11496 9711.817104:
swapper 0 9711.826155:

Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LPU-Reference: <[email protected]>
Signed-off-by: David Ahern <[email protected]>
[ committer note: removed trailing whitespace ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf-script.txt | 5 +-
tools/perf/builtin-script.c | 149 +++++++++++++++++++++++-------
tools/perf/util/parse-events.c | 22 +++++
tools/perf/util/parse-events.h | 1 +
4 files changed, 142 insertions(+), 35 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index c64118a..66f040b 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -115,7 +115,10 @@ OPTIONS
-f::
--fields
Comma separated list of fields to print. Options are:
- comm, tid, pid, time, cpu, event, trace, sym
+ comm, tid, pid, time, cpu, event, trace, sym. Field
+ list must be prepended with the type, trace, sw or hw,
+ to indicate to which event type the field list applies.
+ e.g., -f sw:comm,tid,time,sym and -f trace:time,cpu,trace

-k::
--vmlinux=<file>::
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 31ead6e..57d694c 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -12,6 +12,8 @@
#include "util/trace-event.h"
#include "util/parse-options.h"
#include "util/util.h"
+#include "util/evlist.h"
+#include "util/evsel.h"

static char const *script_name;
static char const *generate_script_lang;
@@ -47,16 +49,61 @@ struct output_option {
};

/* default set to maintain compatibility with current format */
-static u64 output_fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID | \
- PERF_OUTPUT_CPU | PERF_OUTPUT_TIME | \
- PERF_OUTPUT_EVNAME | PERF_OUTPUT_TRACE;
+static u64 output_fields[PERF_TYPE_MAX] = {
+ [PERF_TYPE_HARDWARE] = PERF_OUTPUT_COMM | PERF_OUTPUT_TID | \
+ PERF_OUTPUT_CPU | PERF_OUTPUT_TIME | \
+ PERF_OUTPUT_EVNAME | PERF_OUTPUT_SYM,
+
+ [PERF_TYPE_SOFTWARE] = PERF_OUTPUT_COMM | PERF_OUTPUT_TID | \
+ PERF_OUTPUT_CPU | PERF_OUTPUT_TIME | \
+ PERF_OUTPUT_EVNAME | PERF_OUTPUT_SYM,
+
+ [PERF_TYPE_TRACEPOINT] = PERF_OUTPUT_COMM | PERF_OUTPUT_TID | \
+ PERF_OUTPUT_CPU | PERF_OUTPUT_TIME | \
+ PERF_OUTPUT_EVNAME | PERF_OUTPUT_TRACE,
+};

static bool output_set_by_user;

-#define PRINT_FIELD(x) (output_fields & PERF_OUTPUT_##x)
+#define PRINT_FIELD(x) (output_fields[attr->type] & PERF_OUTPUT_##x)
+
+static int perf_session__check_attr(struct perf_session *session,
+ struct perf_event_attr *attr)
+{
+ if (PRINT_FIELD(TRACE) &&
+ !perf_session__has_traces(session, "record -R"))
+ return -EINVAL;
+
+ if (PRINT_FIELD(SYM) &&
+ !(session->sample_type & PERF_SAMPLE_IP)) {
+ pr_err("Samples do not contain IP data.\n");
+ return -EINVAL;
+ }
+
+ if ((PRINT_FIELD(PID) || PRINT_FIELD(TID)) &&
+ !(session->sample_type & PERF_SAMPLE_TID)) {
+ pr_err("Samples do not contain TID/PID data.\n");
+ return -EINVAL;
+ }
+
+ if (PRINT_FIELD(TIME) &&
+ !(session->sample_type & PERF_SAMPLE_TIME)) {
+ pr_err("Samples do not contain timestamps.\n");
+ return -EINVAL;
+ }
+
+ if (PRINT_FIELD(CPU) &&
+ !(session->sample_type & PERF_SAMPLE_CPU)) {
+ pr_err("Samples do not contain cpu.\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}

static void print_sample_start(struct perf_sample *sample,
- struct thread *thread)
+ struct thread *thread,
+ struct perf_event_attr *attr)
{
int type;
struct event *event;
@@ -97,10 +144,13 @@ static void print_sample_start(struct perf_sample *sample,
}

if (PRINT_FIELD(EVNAME)) {
- type = trace_parse_common_type(sample->raw_data);
- event = trace_find_event(type);
- if (event)
- evname = event->name;
+ if (attr->type == PERF_TYPE_TRACEPOINT) {
+ type = trace_parse_common_type(sample->raw_data);
+ event = trace_find_event(type);
+ if (event)
+ evname = event->name;
+ } else
+ evname = __event_name(attr->type, attr->config);

printf("%s: ", evname ? evname : "(unknown)");
}
@@ -108,10 +158,27 @@ static void print_sample_start(struct perf_sample *sample,

static void process_event(union perf_event *event __unused,
struct perf_sample *sample,
- struct perf_session *session __unused,
+ struct perf_session *session,
struct thread *thread)
{
- print_sample_start(sample, thread);
+ struct perf_event_attr *attr;
+ struct perf_evsel *evsel;
+
+ evsel = perf_evlist__id2evsel(session->evlist, sample->id);
+ if (evsel == NULL) {
+ pr_err("Invalid data. Contains samples with id not in "
+ "its header!\n");
+ return;
+ }
+ attr = &evsel->attr;
+
+ if (output_fields[attr->type] == 0)
+ return;
+
+ if (perf_session__check_attr(session, attr) < 0)
+ return;
+
+ print_sample_start(sample, thread, attr);

if (PRINT_FIELD(TRACE))
print_trace_event(sample->cpu, sample->raw_data,
@@ -183,19 +250,17 @@ static int process_sample_event(union perf_event *event,
return -1;
}

- if (session->sample_type & PERF_SAMPLE_RAW) {
- if (debug_mode) {
- if (sample->time < last_timestamp) {
- pr_err("Samples misordered, previous: %" PRIu64
- " this: %" PRIu64 "\n", last_timestamp,
- sample->time);
- nr_unordered++;
- }
- last_timestamp = sample->time;
- return 0;
+ if (debug_mode) {
+ if (sample->time < last_timestamp) {
+ pr_err("Samples misordered, previous: %" PRIu64
+ " this: %" PRIu64 "\n", last_timestamp,
+ sample->time);
+ nr_unordered++;
}
- scripting_ops->process_event(event, sample, session, thread);
+ last_timestamp = sample->time;
+ return 0;
}
+ scripting_ops->process_event(event, sample, session, thread);

session->hists.stats.total_period += sample->period;
return 0;
@@ -391,21 +456,40 @@ static int parse_output_fields(const struct option *opt __used,
int i, imax = sizeof(all_output_options) / sizeof(struct output_option);
int rc = 0;
char *str = strdup(arg);
+ int type = -1;

if (!str)
return -ENOMEM;

- tok = strtok(str, ",");
+ tok = strtok(str, ":");
if (!tok) {
- fprintf(stderr, "Invalid field string.");
+ fprintf(stderr,
+ "Invalid field string - not prepended with type.");
+ return -EINVAL;
+ }
+
+ /* first word should state which event type user
+ * is specifying the fields
+ */
+ if (!strcmp(tok, "hw"))
+ type = PERF_TYPE_HARDWARE;
+ else if (!strcmp(tok, "sw"))
+ type = PERF_TYPE_SOFTWARE;
+ else if (!strcmp(tok, "trace"))
+ type = PERF_TYPE_TRACEPOINT;
+ else {
+ fprintf(stderr, "Invalid event type in field string.");
return -EINVAL;
}

- output_fields = 0;
+ output_fields[type] = 0;
while (1) {
+ tok = strtok(NULL, ",");
+ if (!tok)
+ break;
for (i = 0; i < imax; ++i) {
if (strcmp(tok, all_output_options[i].str) == 0) {
- output_fields |= all_output_options[i].field;
+ output_fields[type] |= all_output_options[i].field;
break;
}
}
@@ -414,10 +498,11 @@ static int parse_output_fields(const struct option *opt __used,
rc = -EINVAL;
break;
}
+ }

- tok = strtok(NULL, ",");
- if (!tok)
- break;
+ if (output_fields[type] == 0) {
+ pr_debug("No fields requested for %s type. "
+ "Events will not be displayed\n", event_type(type));
}

output_set_by_user = true;
@@ -747,7 +832,7 @@ static const struct option options[] = {
OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",
"Look for files with symbols relative to this directory"),
OPT_CALLBACK('f', "fields", NULL, "str",
- "comma separated output fields. Options: comm,tid,pid,time,cpu,event,trace,sym",
+ "comma separated output fields prepend with 'type:'. Valid types: hw,sw,trace. Fields: comm,tid,pid,time,cpu,event,trace,sym",
parse_output_fields),

OPT_END()
@@ -931,10 +1016,6 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
if (session == NULL)
return -ENOMEM;

- if (strcmp(input_name, "-") &&
- !perf_session__has_traces(session, "record -R"))
- return -EINVAL;
-
if (generate_script_lang) {
struct stat perf_stat;
int input;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 54a7e26..952b4ae 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -263,6 +263,28 @@ static char *event_cache_name(u8 cache_type, u8 cache_op, u8 cache_result)
return name;
}

+const char *event_type(int type)
+{
+ switch (type) {
+ case PERF_TYPE_HARDWARE:
+ return "hardware";
+
+ case PERF_TYPE_SOFTWARE:
+ return "software";
+
+ case PERF_TYPE_TRACEPOINT:
+ return "tracepoint";
+
+ case PERF_TYPE_HW_CACHE:
+ return "hardware-cache";
+
+ default:
+ break;
+ }
+
+ return "unknown";
+}
+
const char *event_name(struct perf_evsel *evsel)
{
u64 config = evsel->attr.config;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 212f88e..746d3fc 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -20,6 +20,7 @@ struct tracepoint_path {
extern struct tracepoint_path *tracepoint_id_to_path(u64 config);
extern bool have_tracepoints(struct list_head *evlist);

+const char *event_type(int type);
const char *event_name(struct perf_evsel *event);
extern const char *__event_name(int type, u64 config);

--
1.6.2.5

2011-03-09 18:32:09

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 08/10] perf script: Support custom field selection for output

From: David Ahern <[email protected]>

Allow a user to select which fields to print to stdout for event data. Options
include comm (command name), tid (thread id), pid (process id), time (perf
timestamp), cpu, event (for event name), and trace (for trace data).

Default is set to maintain compatibility with current output; this feature does
alter output format slightly -- no '-' between command and pid/tid.

Thanks to Frederic Weisbecker for detailed suggestions on this approach.

Examples (output compressed)

1. trace, default format

perf record -ga -e sched:sched_switch
perf script

swapper 0 [000] 537.037184: sched_switch: prev_comm=swapper prev_pid=0...
sshd 1675 [000] 537.037309: sched_switch: prev_comm=sshd prev_pid=1675...
netstat 1692 [001] 537.038664: sched_switch: prev_comm=netstat prev_pid=1692...

2. trace, custom format

perf record -ga -e sched:sched_switch
perf script -f comm,pid,time,trace <--- omitting cpu and event name

swapper 0 537.037184: prev_comm=swapper prev_pid=0 prev_prio=120 ...
sshd 1675 537.037309: prev_comm=sshd prev_pid=1675 prev_prio=120 ...
netstat 1692 537.038664: prev_comm=netstat prev_pid=1692 prev_prio=120 ...

Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LPU-Reference: <[email protected]>
Signed-off-by: David Ahern <[email protected]>
[ committer note: removed several trailing whitespace ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf-script.txt | 5 +
tools/perf/builtin-script.c | 141 ++++++++++++++++++++++++++----
2 files changed, 130 insertions(+), 16 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 29ad942..b73cf58 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -112,6 +112,11 @@ OPTIONS
--debug-mode::
Do various checks like samples ordering and lost events.

+-f::
+--fields
+ Comma separated list of fields to print. Options are:
+ comm, tid, pid, time, cpu, event, trace
+
SEE ALSO
--------
linkperf:perf-record[1], linkperf:perf-script-perl[1],
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 0a79da2..089bdb9 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -20,6 +20,38 @@ static u64 last_timestamp;
static u64 nr_unordered;
extern const struct option record_options[];

+enum perf_output_field {
+ PERF_OUTPUT_COMM = 1U << 0,
+ PERF_OUTPUT_TID = 1U << 1,
+ PERF_OUTPUT_PID = 1U << 2,
+ PERF_OUTPUT_TIME = 1U << 3,
+ PERF_OUTPUT_CPU = 1U << 4,
+ PERF_OUTPUT_EVNAME = 1U << 5,
+ PERF_OUTPUT_TRACE = 1U << 6,
+};
+
+struct output_option {
+ const char *str;
+ enum perf_output_field field;
+} all_output_options [] = {
+ {.str = "comm", .field = PERF_OUTPUT_COMM},
+ {.str = "tid", .field = PERF_OUTPUT_TID},
+ {.str = "pid", .field = PERF_OUTPUT_PID},
+ {.str = "time", .field = PERF_OUTPUT_TIME},
+ {.str = "cpu", .field = PERF_OUTPUT_CPU},
+ {.str = "event", .field = PERF_OUTPUT_EVNAME},
+ {.str = "trace", .field = PERF_OUTPUT_TRACE},
+};
+
+/* default set to maintain compatibility with current format */
+static u64 output_fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID | \
+ PERF_OUTPUT_CPU | PERF_OUTPUT_TIME | \
+ PERF_OUTPUT_EVNAME | PERF_OUTPUT_TRACE;
+
+static bool output_set_by_user;
+
+#define PRINT_FIELD(x) (output_fields & PERF_OUTPUT_##x)
+
static void print_sample_start(struct perf_sample *sample,
struct thread *thread)
{
@@ -28,24 +60,45 @@ static void print_sample_start(struct perf_sample *sample,
const char *evname = NULL;
unsigned long secs;
unsigned long usecs;
- unsigned long long nsecs = sample->time;
+ unsigned long long nsecs;
+
+ if (PRINT_FIELD(COMM)) {
+ if (latency_format)
+ printf("%8.8s ", thread->comm);
+ else
+ printf("%16s ", thread->comm);
+ }

- if (latency_format)
- printf("%8.8s-%-5d %3d", thread->comm, sample->tid, sample->cpu);
- else
- printf("%16s-%-5d [%03d]", thread->comm, sample->tid, sample->cpu);
+ if (PRINT_FIELD(PID) && PRINT_FIELD(TID))
+ printf("%5d/%-5d ", sample->pid, sample->tid);
+ else if (PRINT_FIELD(PID))
+ printf("%5d ", sample->pid);
+ else if (PRINT_FIELD(TID))
+ printf("%5d ", sample->tid);
+
+ if (PRINT_FIELD(CPU)) {
+ if (latency_format)
+ printf("%3d ", sample->cpu);
+ else
+ printf("[%03d] ", sample->cpu);
+ }

- secs = nsecs / NSECS_PER_SEC;
- nsecs -= secs * NSECS_PER_SEC;
- usecs = nsecs / NSECS_PER_USEC;
- printf(" %5lu.%06lu: ", secs, usecs);
+ if (PRINT_FIELD(TIME)) {
+ nsecs = sample->time;
+ secs = nsecs / NSECS_PER_SEC;
+ nsecs -= secs * NSECS_PER_SEC;
+ usecs = nsecs / NSECS_PER_USEC;
+ printf("%5lu.%06lu: ", secs, usecs);
+ }

- type = trace_parse_common_type(sample->raw_data);
- event = trace_find_event(type);
- if (event)
- evname = event->name;
+ if (PRINT_FIELD(EVNAME)) {
+ type = trace_parse_common_type(sample->raw_data);
+ event = trace_find_event(type);
+ if (event)
+ evname = event->name;

- printf("%s: ", evname ? evname : "(unknown)");
+ printf("%s: ", evname ? evname : "(unknown)");
+ }
}

static void process_event(union perf_event *event __unused,
@@ -54,7 +107,11 @@ static void process_event(union perf_event *event __unused,
struct thread *thread)
{
print_sample_start(sample, thread);
- print_trace_event(sample->cpu, sample->raw_data, sample->raw_size);
+
+ if (PRINT_FIELD(TRACE))
+ print_trace_event(sample->cpu, sample->raw_data,
+ sample->raw_size);
+
printf("\n");
}

@@ -311,6 +368,48 @@ static int parse_scriptname(const struct option *opt __used,
return 0;
}

+static int parse_output_fields(const struct option *opt __used,
+ const char *arg, int unset __used)
+{
+ char *tok;
+ int i, imax = sizeof(all_output_options) / sizeof(struct output_option);
+ int rc = 0;
+ char *str = strdup(arg);
+
+ if (!str)
+ return -ENOMEM;
+
+ tok = strtok(str, ",");
+ if (!tok) {
+ fprintf(stderr, "Invalid field string.");
+ return -EINVAL;
+ }
+
+ output_fields = 0;
+ while (1) {
+ for (i = 0; i < imax; ++i) {
+ if (strcmp(tok, all_output_options[i].str) == 0) {
+ output_fields |= all_output_options[i].field;
+ break;
+ }
+ }
+ if (i == imax) {
+ fprintf(stderr, "Invalid field requested.");
+ rc = -EINVAL;
+ break;
+ }
+
+ tok = strtok(NULL, ",");
+ if (!tok)
+ break;
+ }
+
+ output_set_by_user = true;
+
+ free(str);
+ return rc;
+}
+
/* Helper function for filesystems that return a dent->d_type DT_UNKNOWN */
static int is_directory(const char *base_path, const struct dirent *dent)
{
@@ -623,6 +722,9 @@ static const struct option options[] = {
"input file name"),
OPT_BOOLEAN('d', "debug-mode", &debug_mode,
"do various checks like samples ordering and lost events"),
+ OPT_CALLBACK('f', "fields", NULL, "str",
+ "comma separated output fields. Options: comm,tid,pid,time,cpu,event,trace",
+ parse_output_fields),

OPT_END()
};
@@ -809,8 +911,15 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)

if (generate_script_lang) {
struct stat perf_stat;
+ int input;
+
+ if (output_set_by_user) {
+ fprintf(stderr,
+ "custom fields not supported for generated scripts");
+ return -1;
+ }

- int input = open(input_name, O_RDONLY);
+ input = open(input_name, O_RDONLY);
if (input < 0) {
perror("failed to open file");
exit(-1);
--
1.6.2.5

2011-03-09 18:31:50

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 01/10] perf session: Simplify evlist creation from perf.data header

From: Arnaldo Carvalho de Melo <[email protected]>

We don't need to do that perf_header__add_attr +
perf_header_attr__add_id to then create the evsels and hash its ids.

Next csets will finish this perf_header and perf_header_attr stuff,
using the evsels for its purpose.

Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-record.c | 2 +-
tools/perf/builtin-report.c | 4 +-
tools/perf/util/header.c | 66 +++++++++++++++++++++++++-----------------
tools/perf/util/header.h | 6 ++--
tools/perf/util/session.c | 53 ++--------------------------------
5 files changed, 49 insertions(+), 82 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d40a81e..9d236e8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -524,7 +524,7 @@ static int __cmd_record(int argc, const char **argv)
perf_header__set_feat(&session->header, HEADER_BUILD_ID);

if (!file_new) {
- err = perf_header__read(session, output);
+ err = perf_session__read_header(session, output);
if (err < 0)
goto out_delete_session;
}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index e9b5d51..b1b8200 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -70,8 +70,8 @@ static int perf_session__add_hist_entry(struct perf_session *session,
* FIXME: Propagate this back, but at least we're in a builtin,
* where exit() is allowed. ;-)
*/
- ui__warning("Invalid %s file, contains samples with id not in "
- "its header!\n", input_name);
+ ui__warning("Invalid %s file, contains samples with id %" PRIu64 " not in "
+ "its header!\n", input_name, sample->id);
exit_browser(0);
exit(1);
}
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 108b0db..3457ec6 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -9,6 +9,7 @@
#include <linux/kernel.h>

#include "evlist.h"
+#include "evsel.h"
#include "util.h"
#include "header.h"
#include "../perf.h"
@@ -861,7 +862,7 @@ static int perf_header__read_pipe(struct perf_session *session, int fd)
return 0;
}

-int perf_header__read(struct perf_session *session, int fd)
+int perf_session__read_header(struct perf_session *session, int fd)
{
struct perf_header *self = &session->header;
struct perf_file_header f_header;
@@ -869,6 +870,10 @@ int perf_header__read(struct perf_session *session, int fd)
u64 f_id;
int nr_attrs, nr_ids, i, j;

+ session->evlist = perf_evlist__new(NULL, NULL);
+ if (session->evlist == NULL)
+ return -ENOMEM;
+
if (session->fd_pipe)
return perf_header__read_pipe(session, fd);

@@ -881,33 +886,39 @@ int perf_header__read(struct perf_session *session, int fd)
lseek(fd, f_header.attrs.offset, SEEK_SET);

for (i = 0; i < nr_attrs; i++) {
- struct perf_header_attr *attr;
+ struct perf_evsel *evsel;
off_t tmp;

if (perf_header__getbuffer64(self, fd, &f_attr, sizeof(f_attr)))
goto out_errno;

tmp = lseek(fd, 0, SEEK_CUR);
+ evsel = perf_evsel__new(&f_attr.attr, i);

- attr = perf_header_attr__new(&f_attr.attr);
- if (attr == NULL)
- return -ENOMEM;
+ if (evsel == NULL)
+ goto out_delete_evlist;
+ /*
+ * Do it before so that if perf_evsel__alloc_id fails, this
+ * entry gets purged too at perf_evlist__delete().
+ */
+ perf_evlist__add(session->evlist, evsel);

nr_ids = f_attr.ids.size / sizeof(u64);
+ /*
+ * We don't have the cpu and thread maps on the header, so
+ * for allocating the perf_sample_id table we fake 1 cpu and
+ * hattr->ids threads.
+ */
+ if (perf_evsel__alloc_id(evsel, 1, nr_ids))
+ goto out_delete_evlist;
+
lseek(fd, f_attr.ids.offset, SEEK_SET);

for (j = 0; j < nr_ids; j++) {
if (perf_header__getbuffer64(self, fd, &f_id, sizeof(f_id)))
goto out_errno;

- if (perf_header_attr__add_id(attr, f_id) < 0) {
- perf_header_attr__delete(attr);
- return -ENOMEM;
- }
- }
- if (perf_header__add_attr(self, attr) < 0) {
- perf_header_attr__delete(attr);
- return -ENOMEM;
+ perf_evlist__id_hash(session->evlist, evsel, 0, j, f_id);
}

lseek(fd, tmp, SEEK_SET);
@@ -932,37 +943,38 @@ int perf_header__read(struct perf_session *session, int fd)
return 0;
out_errno:
return -errno;
+
+out_delete_evlist:
+ perf_evlist__delete(session->evlist);
+ session->evlist = NULL;
+ return -ENOMEM;
}

-u64 perf_header__sample_type(struct perf_header *header)
+u64 perf_evlist__sample_type(struct perf_evlist *evlist)
{
+ struct perf_evsel *pos;
u64 type = 0;
- int i;
-
- for (i = 0; i < header->attrs; i++) {
- struct perf_header_attr *attr = header->attr[i];

+ list_for_each_entry(pos, &evlist->entries, node) {
if (!type)
- type = attr->attr.sample_type;
- else if (type != attr->attr.sample_type)
+ type = pos->attr.sample_type;
+ else if (type != pos->attr.sample_type)
die("non matching sample_type");
}

return type;
}

-bool perf_header__sample_id_all(const struct perf_header *header)
+bool perf_evlist__sample_id_all(const struct perf_evlist *evlist)
{
bool value = false, first = true;
- int i;
-
- for (i = 0; i < header->attrs; i++) {
- struct perf_header_attr *attr = header->attr[i];
+ struct perf_evsel *pos;

+ list_for_each_entry(pos, &evlist->entries, node) {
if (first) {
- value = attr->attr.sample_id_all;
+ value = pos->attr.sample_id_all;
first = false;
- } else if (value != attr->attr.sample_id_all)
+ } else if (value != pos->attr.sample_id_all)
die("non matching sample_id_all");
}

diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 2fab133..73b84eb 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -67,7 +67,7 @@ void perf_header__exit(struct perf_header *self);

struct perf_evlist;

-int perf_header__read(struct perf_session *session, int fd);
+int perf_session__read_header(struct perf_session *session, int fd);
int perf_header__write(struct perf_header *self, struct perf_evlist *evlist,
int fd, bool at_exit);
int perf_header__write_pipe(int fd);
@@ -83,8 +83,8 @@ void perf_header_attr__delete(struct perf_header_attr *self);

int perf_header_attr__add_id(struct perf_header_attr *self, u64 id);

-u64 perf_header__sample_type(struct perf_header *header);
-bool perf_header__sample_id_all(const struct perf_header *header);
+u64 perf_evlist__sample_type(struct perf_evlist *evlist);
+bool perf_evlist__sample_id_all(const struct perf_evlist *evlist);
void perf_header__set_feat(struct perf_header *self, int feat);
void perf_header__clear_feat(struct perf_header *self, int feat);
bool perf_header__has_feat(const struct perf_header *self, int feat);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 0d41419..26b24c5 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -13,46 +13,6 @@
#include "sort.h"
#include "util.h"

-static int perf_session__read_evlist(struct perf_session *session)
-{
- int i, j;
-
- session->evlist = perf_evlist__new(NULL, NULL);
- if (session->evlist == NULL)
- return -ENOMEM;
-
- for (i = 0; i < session->header.attrs; ++i) {
- struct perf_header_attr *hattr = session->header.attr[i];
- struct perf_evsel *evsel = perf_evsel__new(&hattr->attr, i);
-
- if (evsel == NULL)
- goto out_delete_evlist;
- /*
- * Do it before so that if perf_evsel__alloc_id fails, this
- * entry gets purged too at perf_evlist__delete().
- */
- perf_evlist__add(session->evlist, evsel);
- /*
- * We don't have the cpu and thread maps on the header, so
- * for allocating the perf_sample_id table we fake 1 cpu and
- * hattr->ids threads.
- */
- if (perf_evsel__alloc_id(evsel, 1, hattr->ids))
- goto out_delete_evlist;
-
- for (j = 0; j < hattr->ids; ++j)
- perf_evlist__id_hash(session->evlist, evsel, 0, j,
- hattr->id[j]);
- }
-
- return 0;
-
-out_delete_evlist:
- perf_evlist__delete(session->evlist);
- session->evlist = NULL;
- return -ENOMEM;
-}
-
static int perf_session__open(struct perf_session *self, bool force)
{
struct stat input_stat;
@@ -61,7 +21,7 @@ static int perf_session__open(struct perf_session *self, bool force)
self->fd_pipe = true;
self->fd = STDIN_FILENO;

- if (perf_header__read(self, self->fd) < 0)
+ if (perf_session__read_header(self, self->fd) < 0)
pr_err("incompatible file format");

return 0;
@@ -93,16 +53,11 @@ static int perf_session__open(struct perf_session *self, bool force)
goto out_close;
}

- if (perf_header__read(self, self->fd) < 0) {
+ if (perf_session__read_header(self, self->fd) < 0) {
pr_err("incompatible file format");
goto out_close;
}

- if (perf_session__read_evlist(self) < 0) {
- pr_err("Not enough memory to read the event selector list\n");
- goto out_close;
- }
-
self->size = input_stat.st_size;
return 0;

@@ -152,8 +107,8 @@ void perf_session__set_sample_type(struct perf_session *session, u64 type)

void perf_session__update_sample_type(struct perf_session *self)
{
- self->sample_type = perf_header__sample_type(&self->header);
- self->sample_id_all = perf_header__sample_id_all(&self->header);
+ self->sample_type = perf_evlist__sample_type(self->evlist);
+ self->sample_id_all = perf_evlist__sample_id_all(self->evlist);
perf_session__id_header_size(self);
}

--
1.6.2.5

2011-03-09 18:32:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 03/10] perf header: Stop using 'self'

From: Arnaldo Carvalho de Melo <[email protected]>

Stop using this python/OOP convention, doesn't really helps. Will do
more from time to time till we get it cleaned up in all of tools/perf.

Suggested-by: Thomas Gleixner <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/header.c | 199 +++++++++++++++++++++++-----------------------
tools/perf/util/header.h | 12 ++--
2 files changed, 105 insertions(+), 106 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 40b10e4..5a72d42 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -66,19 +66,19 @@ struct perf_file_attr {
struct perf_file_section ids;
};

-void perf_header__set_feat(struct perf_header *self, int feat)
+void perf_header__set_feat(struct perf_header *header, int feat)
{
- set_bit(feat, self->adds_features);
+ set_bit(feat, header->adds_features);
}

-void perf_header__clear_feat(struct perf_header *self, int feat)
+void perf_header__clear_feat(struct perf_header *header, int feat)
{
- clear_bit(feat, self->adds_features);
+ clear_bit(feat, header->adds_features);
}

-bool perf_header__has_feat(const struct perf_header *self, int feat)
+bool perf_header__has_feat(const struct perf_header *header, int feat)
{
- return test_bit(feat, self->adds_features);
+ return test_bit(feat, header->adds_features);
}

static int do_write(int fd, const void *buf, size_t size)
@@ -147,22 +147,22 @@ static int __dsos__write_buildid_table(struct list_head *head, pid_t pid,
return 0;
}

-static int machine__write_buildid_table(struct machine *self, int fd)
+static int machine__write_buildid_table(struct machine *machine, int fd)
{
int err;
u16 kmisc = PERF_RECORD_MISC_KERNEL,
umisc = PERF_RECORD_MISC_USER;

- if (!machine__is_host(self)) {
+ if (!machine__is_host(machine)) {
kmisc = PERF_RECORD_MISC_GUEST_KERNEL;
umisc = PERF_RECORD_MISC_GUEST_USER;
}

- err = __dsos__write_buildid_table(&self->kernel_dsos, self->pid,
+ err = __dsos__write_buildid_table(&machine->kernel_dsos, machine->pid,
kmisc, fd);
if (err == 0)
- err = __dsos__write_buildid_table(&self->user_dsos,
- self->pid, umisc, fd);
+ err = __dsos__write_buildid_table(&machine->user_dsos,
+ machine->pid, umisc, fd);
return err;
}

@@ -280,12 +280,12 @@ out_free:
return err;
}

-static int dso__cache_build_id(struct dso *self, const char *debugdir)
+static int dso__cache_build_id(struct dso *dso, const char *debugdir)
{
- bool is_kallsyms = self->kernel && self->long_name[0] != '/';
+ bool is_kallsyms = dso->kernel && dso->long_name[0] != '/';

- return build_id_cache__add_b(self->build_id, sizeof(self->build_id),
- self->long_name, debugdir, is_kallsyms);
+ return build_id_cache__add_b(dso->build_id, sizeof(dso->build_id),
+ dso->long_name, debugdir, is_kallsyms);
}

static int __dsos__cache_build_ids(struct list_head *head, const char *debugdir)
@@ -300,14 +300,14 @@ static int __dsos__cache_build_ids(struct list_head *head, const char *debugdir)
return err;
}

-static int machine__cache_build_ids(struct machine *self, const char *debugdir)
+static int machine__cache_build_ids(struct machine *machine, const char *debugdir)
{
- int ret = __dsos__cache_build_ids(&self->kernel_dsos, debugdir);
- ret |= __dsos__cache_build_ids(&self->user_dsos, debugdir);
+ int ret = __dsos__cache_build_ids(&machine->kernel_dsos, debugdir);
+ ret |= __dsos__cache_build_ids(&machine->user_dsos, debugdir);
return ret;
}

-static int perf_session__cache_build_ids(struct perf_session *self)
+static int perf_session__cache_build_ids(struct perf_session *session)
{
struct rb_node *nd;
int ret;
@@ -318,28 +318,28 @@ static int perf_session__cache_build_ids(struct perf_session *self)
if (mkdir(debugdir, 0755) != 0 && errno != EEXIST)
return -1;

- ret = machine__cache_build_ids(&self->host_machine, debugdir);
+ ret = machine__cache_build_ids(&session->host_machine, debugdir);

- for (nd = rb_first(&self->machines); nd; nd = rb_next(nd)) {
+ for (nd = rb_first(&session->machines); nd; nd = rb_next(nd)) {
struct machine *pos = rb_entry(nd, struct machine, rb_node);
ret |= machine__cache_build_ids(pos, debugdir);
}
return ret ? -1 : 0;
}

-static bool machine__read_build_ids(struct machine *self, bool with_hits)
+static bool machine__read_build_ids(struct machine *machine, bool with_hits)
{
- bool ret = __dsos__read_build_ids(&self->kernel_dsos, with_hits);
- ret |= __dsos__read_build_ids(&self->user_dsos, with_hits);
+ bool ret = __dsos__read_build_ids(&machine->kernel_dsos, with_hits);
+ ret |= __dsos__read_build_ids(&machine->user_dsos, with_hits);
return ret;
}

-static bool perf_session__read_build_ids(struct perf_session *self, bool with_hits)
+static bool perf_session__read_build_ids(struct perf_session *session, bool with_hits)
{
struct rb_node *nd;
- bool ret = machine__read_build_ids(&self->host_machine, with_hits);
+ bool ret = machine__read_build_ids(&session->host_machine, with_hits);

- for (nd = rb_first(&self->machines); nd; nd = rb_next(nd)) {
+ for (nd = rb_first(&session->machines); nd; nd = rb_next(nd)) {
struct machine *pos = rb_entry(nd, struct machine, rb_node);
ret |= machine__read_build_ids(pos, with_hits);
}
@@ -347,7 +347,7 @@ static bool perf_session__read_build_ids(struct perf_session *self, bool with_hi
return ret;
}

-static int perf_header__adds_write(struct perf_header *self,
+static int perf_header__adds_write(struct perf_header *header,
struct perf_evlist *evlist, int fd)
{
int nr_sections;
@@ -357,13 +357,13 @@ static int perf_header__adds_write(struct perf_header *self,
u64 sec_start;
int idx = 0, err;

- session = container_of(self, struct perf_session, header);
+ session = container_of(header, struct perf_session, header);

- if (perf_header__has_feat(self, HEADER_BUILD_ID &&
+ if (perf_header__has_feat(header, HEADER_BUILD_ID &&
!perf_session__read_build_ids(session, true)))
- perf_header__clear_feat(self, HEADER_BUILD_ID);
+ perf_header__clear_feat(header, HEADER_BUILD_ID);

- nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
+ nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS);
if (!nr_sections)
return 0;

@@ -373,10 +373,10 @@ static int perf_header__adds_write(struct perf_header *self,

sec_size = sizeof(*feat_sec) * nr_sections;

- sec_start = self->data_offset + self->data_size;
+ sec_start = header->data_offset + header->data_size;
lseek(fd, sec_start + sec_size, SEEK_SET);

- if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
+ if (perf_header__has_feat(header, HEADER_TRACE_INFO)) {
struct perf_file_section *trace_sec;

trace_sec = &feat_sec[idx++];
@@ -387,14 +387,14 @@ static int perf_header__adds_write(struct perf_header *self,
trace_sec->size = lseek(fd, 0, SEEK_CUR) - trace_sec->offset;
}

- if (perf_header__has_feat(self, HEADER_BUILD_ID)) {
+ if (perf_header__has_feat(header, HEADER_BUILD_ID)) {
struct perf_file_section *buildid_sec;

buildid_sec = &feat_sec[idx++];

/* Write build-ids */
buildid_sec->offset = lseek(fd, 0, SEEK_CUR);
- err = dsos__write_buildid_table(self, fd);
+ err = dsos__write_buildid_table(header, fd);
if (err < 0) {
pr_debug("failed to write buildid table\n");
goto out_free;
@@ -439,7 +439,7 @@ int perf_session__write_header(struct perf_session *session,
{
struct perf_file_header f_header;
struct perf_file_attr f_attr;
- struct perf_header *self = &session->header;
+ struct perf_header *header = &session->header;
struct perf_evsel *attr, *pair = NULL;
int err;

@@ -465,7 +465,7 @@ out_err_write:
}
}

- self->attr_offset = lseek(fd, 0, SEEK_CUR);
+ header->attr_offset = lseek(fd, 0, SEEK_CUR);

list_for_each_entry(attr, &evlist->entries, node) {
f_attr = (struct perf_file_attr){
@@ -482,20 +482,20 @@ out_err_write:
}
}

- self->event_offset = lseek(fd, 0, SEEK_CUR);
- self->event_size = event_count * sizeof(struct perf_trace_event_type);
+ header->event_offset = lseek(fd, 0, SEEK_CUR);
+ header->event_size = event_count * sizeof(struct perf_trace_event_type);
if (events) {
- err = do_write(fd, events, self->event_size);
+ err = do_write(fd, events, header->event_size);
if (err < 0) {
pr_debug("failed to write perf header events\n");
return err;
}
}

- self->data_offset = lseek(fd, 0, SEEK_CUR);
+ header->data_offset = lseek(fd, 0, SEEK_CUR);

if (at_exit) {
- err = perf_header__adds_write(self, evlist, fd);
+ err = perf_header__adds_write(header, evlist, fd);
if (err < 0)
return err;
}
@@ -505,20 +505,20 @@ out_err_write:
.size = sizeof(f_header),
.attr_size = sizeof(f_attr),
.attrs = {
- .offset = self->attr_offset,
+ .offset = header->attr_offset,
.size = evlist->nr_entries * sizeof(f_attr),
},
.data = {
- .offset = self->data_offset,
- .size = self->data_size,
+ .offset = header->data_offset,
+ .size = header->data_size,
},
.event_types = {
- .offset = self->event_offset,
- .size = self->event_size,
+ .offset = header->event_offset,
+ .size = header->event_size,
},
};

- memcpy(&f_header.adds_features, &self->adds_features, sizeof(self->adds_features));
+ memcpy(&f_header.adds_features, &header->adds_features, sizeof(header->adds_features));

lseek(fd, 0, SEEK_SET);
err = do_write(fd, &f_header, sizeof(f_header));
@@ -526,26 +526,26 @@ out_err_write:
pr_debug("failed to write perf header\n");
return err;
}
- lseek(fd, self->data_offset + self->data_size, SEEK_SET);
+ lseek(fd, header->data_offset + header->data_size, SEEK_SET);

- self->frozen = 1;
+ header->frozen = 1;
return 0;
}

-static int perf_header__getbuffer64(struct perf_header *self,
+static int perf_header__getbuffer64(struct perf_header *header,
int fd, void *buf, size_t size)
{
if (readn(fd, buf, size) <= 0)
return -1;

- if (self->needs_swap)
+ if (header->needs_swap)
mem_bswap_64(buf, size);

return 0;
}

-int perf_header__process_sections(struct perf_header *self, int fd,
- int (*process)(struct perf_file_section *self,
+int perf_header__process_sections(struct perf_header *header, int fd,
+ int (*process)(struct perf_file_section *section,
struct perf_header *ph,
int feat, int fd))
{
@@ -555,7 +555,7 @@ int perf_header__process_sections(struct perf_header *self, int fd,
int idx = 0;
int err = -1, feat = 1;

- nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
+ nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS);
if (!nr_sections)
return 0;

@@ -565,17 +565,17 @@ int perf_header__process_sections(struct perf_header *self, int fd,

sec_size = sizeof(*feat_sec) * nr_sections;

- lseek(fd, self->data_offset + self->data_size, SEEK_SET);
+ lseek(fd, header->data_offset + header->data_size, SEEK_SET);

- if (perf_header__getbuffer64(self, fd, feat_sec, sec_size))
+ if (perf_header__getbuffer64(header, fd, feat_sec, sec_size))
goto out_free;

err = 0;
while (idx < nr_sections && feat < HEADER_LAST_FEATURE) {
- if (perf_header__has_feat(self, feat)) {
+ if (perf_header__has_feat(header, feat)) {
struct perf_file_section *sec = &feat_sec[idx++];

- err = process(sec, self, feat, fd);
+ err = process(sec, header, feat, fd);
if (err < 0)
break;
}
@@ -586,35 +586,35 @@ out_free:
return err;
}

-int perf_file_header__read(struct perf_file_header *self,
+int perf_file_header__read(struct perf_file_header *header,
struct perf_header *ph, int fd)
{
lseek(fd, 0, SEEK_SET);

- if (readn(fd, self, sizeof(*self)) <= 0 ||
- memcmp(&self->magic, __perf_magic, sizeof(self->magic)))
+ if (readn(fd, header, sizeof(*header)) <= 0 ||
+ memcmp(&header->magic, __perf_magic, sizeof(header->magic)))
return -1;

- if (self->attr_size != sizeof(struct perf_file_attr)) {
- u64 attr_size = bswap_64(self->attr_size);
+ if (header->attr_size != sizeof(struct perf_file_attr)) {
+ u64 attr_size = bswap_64(header->attr_size);

if (attr_size != sizeof(struct perf_file_attr))
return -1;

- mem_bswap_64(self, offsetof(struct perf_file_header,
+ mem_bswap_64(header, offsetof(struct perf_file_header,
adds_features));
ph->needs_swap = true;
}

- if (self->size != sizeof(*self)) {
+ if (header->size != sizeof(*header)) {
/* Support the previous format */
- if (self->size == offsetof(typeof(*self), adds_features))
- bitmap_zero(self->adds_features, HEADER_FEAT_BITS);
+ if (header->size == offsetof(typeof(*header), adds_features))
+ bitmap_zero(header->adds_features, HEADER_FEAT_BITS);
else
return -1;
}

- memcpy(&ph->adds_features, &self->adds_features,
+ memcpy(&ph->adds_features, &header->adds_features,
sizeof(ph->adds_features));
/*
* FIXME: hack that assumes that if we need swap the perf.data file
@@ -628,10 +628,10 @@ int perf_file_header__read(struct perf_file_header *self,
perf_header__set_feat(ph, HEADER_BUILD_ID);
}

- ph->event_offset = self->event_types.offset;
- ph->event_size = self->event_types.size;
- ph->data_offset = self->data.offset;
- ph->data_size = self->data.size;
+ ph->event_offset = header->event_types.offset;
+ ph->event_size = header->event_types.size;
+ ph->data_offset = header->data.offset;
+ ph->data_size = header->data.size;
return 0;
}

@@ -690,11 +690,10 @@ out:
return err;
}

-static int perf_header__read_build_ids(struct perf_header *self,
- int input, u64 offset, u64 size)
+static int perf_header__read_build_ids(struct perf_header *header,
+ int input, u64 offset, u64 size)
{
- struct perf_session *session = container_of(self,
- struct perf_session, header);
+ struct perf_session *session = container_of(header, struct perf_session, header);
struct build_id_event bev;
char filename[PATH_MAX];
u64 limit = offset + size;
@@ -706,7 +705,7 @@ static int perf_header__read_build_ids(struct perf_header *self,
if (read(input, &bev, sizeof(bev)) != sizeof(bev))
goto out;

- if (self->needs_swap)
+ if (header->needs_swap)
perf_event_header__bswap(&bev.header);

len = bev.header.size - sizeof(bev);
@@ -722,13 +721,13 @@ out:
return err;
}

-static int perf_file_section__process(struct perf_file_section *self,
+static int perf_file_section__process(struct perf_file_section *section,
struct perf_header *ph,
int feat, int fd)
{
- if (lseek(fd, self->offset, SEEK_SET) == (off_t)-1) {
+ if (lseek(fd, section->offset, SEEK_SET) == (off_t)-1) {
pr_debug("Failed to lseek to %" PRIu64 " offset for feature "
- "%d, continuing...\n", self->offset, feat);
+ "%d, continuing...\n", section->offset, feat);
return 0;
}

@@ -738,7 +737,7 @@ static int perf_file_section__process(struct perf_file_section *self,
break;

case HEADER_BUILD_ID:
- if (perf_header__read_build_ids(ph, fd, self->offset, self->size))
+ if (perf_header__read_build_ids(ph, fd, section->offset, section->size))
pr_debug("Failed to read buildids, continuing...\n");
break;
default:
@@ -748,21 +747,21 @@ static int perf_file_section__process(struct perf_file_section *self,
return 0;
}

-static int perf_file_header__read_pipe(struct perf_pipe_file_header *self,
+static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
struct perf_header *ph, int fd,
bool repipe)
{
- if (readn(fd, self, sizeof(*self)) <= 0 ||
- memcmp(&self->magic, __perf_magic, sizeof(self->magic)))
+ if (readn(fd, header, sizeof(*header)) <= 0 ||
+ memcmp(&header->magic, __perf_magic, sizeof(header->magic)))
return -1;

- if (repipe && do_write(STDOUT_FILENO, self, sizeof(*self)) < 0)
+ if (repipe && do_write(STDOUT_FILENO, header, sizeof(*header)) < 0)
return -1;

- if (self->size != sizeof(*self)) {
- u64 size = bswap_64(self->size);
+ if (header->size != sizeof(*header)) {
+ u64 size = bswap_64(header->size);

- if (size != sizeof(*self))
+ if (size != sizeof(*header))
return -1;

ph->needs_swap = true;
@@ -773,10 +772,10 @@ static int perf_file_header__read_pipe(struct perf_pipe_file_header *self,

static int perf_header__read_pipe(struct perf_session *session, int fd)
{
- struct perf_header *self = &session->header;
+ struct perf_header *header = &session->header;
struct perf_pipe_file_header f_header;

- if (perf_file_header__read_pipe(&f_header, self, fd,
+ if (perf_file_header__read_pipe(&f_header, header, fd,
session->repipe) < 0) {
pr_debug("incompatible file format\n");
return -EINVAL;
@@ -789,7 +788,7 @@ static int perf_header__read_pipe(struct perf_session *session, int fd)

int perf_session__read_header(struct perf_session *session, int fd)
{
- struct perf_header *self = &session->header;
+ struct perf_header *header = &session->header;
struct perf_file_header f_header;
struct perf_file_attr f_attr;
u64 f_id;
@@ -802,7 +801,7 @@ int perf_session__read_header(struct perf_session *session, int fd)
if (session->fd_pipe)
return perf_header__read_pipe(session, fd);

- if (perf_file_header__read(&f_header, self, fd) < 0) {
+ if (perf_file_header__read(&f_header, header, fd) < 0) {
pr_debug("incompatible file format\n");
return -EINVAL;
}
@@ -814,7 +813,7 @@ int perf_session__read_header(struct perf_session *session, int fd)
struct perf_evsel *evsel;
off_t tmp;

- if (perf_header__getbuffer64(self, fd, &f_attr, sizeof(f_attr)))
+ if (perf_header__getbuffer64(header, fd, &f_attr, sizeof(f_attr)))
goto out_errno;

tmp = lseek(fd, 0, SEEK_CUR);
@@ -840,7 +839,7 @@ int perf_session__read_header(struct perf_session *session, int fd)
lseek(fd, f_attr.ids.offset, SEEK_SET);

for (j = 0; j < nr_ids; j++) {
- if (perf_header__getbuffer64(self, fd, &f_id, sizeof(f_id)))
+ if (perf_header__getbuffer64(header, fd, &f_id, sizeof(f_id)))
goto out_errno;

perf_evlist__id_add(session->evlist, evsel, 0, j, f_id);
@@ -854,17 +853,17 @@ int perf_session__read_header(struct perf_session *session, int fd)
events = malloc(f_header.event_types.size);
if (events == NULL)
return -ENOMEM;
- if (perf_header__getbuffer64(self, fd, events,
+ if (perf_header__getbuffer64(header, fd, events,
f_header.event_types.size))
goto out_errno;
event_count = f_header.event_types.size / sizeof(struct perf_trace_event_type);
}

- perf_header__process_sections(self, fd, perf_file_section__process);
+ perf_header__process_sections(header, fd, perf_file_section__process);

- lseek(fd, self->data_offset, SEEK_SET);
+ lseek(fd, header->data_offset, SEEK_SET);

- self->frozen = 1;
+ header->frozen = 1;
return 0;
out_errno:
return -errno;
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 4cc2675..456661d 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -39,7 +39,7 @@ struct perf_pipe_file_header {

struct perf_header;

-int perf_file_header__read(struct perf_file_header *self,
+int perf_file_header__read(struct perf_file_header *header,
struct perf_header *ph, int fd);

struct perf_header {
@@ -66,12 +66,12 @@ char *perf_header__find_event(u64 id);

u64 perf_evlist__sample_type(struct perf_evlist *evlist);
bool perf_evlist__sample_id_all(const struct perf_evlist *evlist);
-void perf_header__set_feat(struct perf_header *self, int feat);
-void perf_header__clear_feat(struct perf_header *self, int feat);
-bool perf_header__has_feat(const struct perf_header *self, int feat);
+void perf_header__set_feat(struct perf_header *header, int feat);
+void perf_header__clear_feat(struct perf_header *header, int feat);
+bool perf_header__has_feat(const struct perf_header *header, int feat);

-int perf_header__process_sections(struct perf_header *self, int fd,
- int (*process)(struct perf_file_section *self,
+int perf_header__process_sections(struct perf_header *header, int fd,
+ int (*process)(struct perf_file_section *section,
struct perf_header *ph,
int feat, int fd));

--
1.6.2.5

2011-03-09 18:32:44

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 06/10] perf script: Change process_event prototype

From: David Ahern <[email protected]>

Prepare for handling of samples for any event type.

Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LPU-Reference: <[email protected]>
Signed-off-by: David Ahern <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-script.c | 25 ++++++++++++-------
.../perf/util/scripting-engines/trace-event-perl.c | 11 ++++++--
.../util/scripting-engines/trace-event-python.c | 11 ++++++--
tools/perf/util/trace-event-scripting.c | 9 +++----
tools/perf/util/trace-event.h | 7 ++++-
5 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 5f40df6..b2bdd55 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -20,6 +20,20 @@ static u64 last_timestamp;
static u64 nr_unordered;
extern const struct option record_options[];

+static void process_event(union perf_event *event __unused,
+ struct perf_sample *sample,
+ struct perf_session *session __unused,
+ struct thread *thread)
+{
+ /*
+ * FIXME: better resolve from pid from the struct trace_entry
+ * field, although it should be the same than this perf
+ * event pid
+ */
+ print_event(sample->cpu, sample->raw_data, sample->raw_size,
+ sample->time, thread->comm);
+}
+
static int default_start_script(const char *script __unused,
int argc __unused,
const char **argv __unused)
@@ -40,7 +54,7 @@ static int default_generate_script(const char *outfile __unused)
static struct scripting_ops default_scripting_ops = {
.start_script = default_start_script,
.stop_script = default_stop_script,
- .process_event = print_event,
+ .process_event = process_event,
.generate_script = default_generate_script,
};

@@ -86,14 +100,7 @@ static int process_sample_event(union perf_event *event,
last_timestamp = sample->time;
return 0;
}
- /*
- * FIXME: better resolve from pid from the struct trace_entry
- * field, although it should be the same than this perf
- * event pid
- */
- scripting_ops->process_event(sample->cpu, sample->raw_data,
- sample->raw_size,
- sample->time, thread->comm);
+ scripting_ops->process_event(event, sample, session, thread);
}

session->hists.stats.total_period += sample->period;
diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
index 9368081..6214272 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -245,9 +245,10 @@ static inline struct event *find_cache_event(int type)
return event;
}

-static void perl_process_event(int cpu, void *data,
- int size __unused,
- unsigned long long nsecs, char *comm)
+static void perl_process_event(union perf_event *pevent __unused,
+ struct perf_sample *sample,
+ struct perf_session *session __unused,
+ struct thread *thread)
{
struct format_field *field;
static char handler[256];
@@ -256,6 +257,10 @@ static void perl_process_event(int cpu, void *data,
struct event *event;
int type;
int pid;
+ int cpu = sample->cpu;
+ void *data = sample->raw_data;
+ unsigned long long nsecs = sample->time;
+ char *comm = thread->comm;

dSP;

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 2040b85..1b85d60 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -204,9 +204,10 @@ static inline struct event *find_cache_event(int type)
return event;
}

-static void python_process_event(int cpu, void *data,
- int size __unused,
- unsigned long long nsecs, char *comm)
+static void python_process_event(union perf_event *pevent __unused,
+ struct perf_sample *sample,
+ struct perf_session *session __unused,
+ struct thread *thread)
{
PyObject *handler, *retval, *context, *t, *obj, *dict = NULL;
static char handler_name[256];
@@ -217,6 +218,10 @@ static void python_process_event(int cpu, void *data,
unsigned n = 0;
int type;
int pid;
+ int cpu = sample->cpu;
+ void *data = sample->raw_data;
+ unsigned long long nsecs = sample->time;
+ char *comm = thread->comm;

t = PyTuple_New(MAX_FIELDS);
if (!t)
diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c
index f7af2fc..66f4b78 100644
--- a/tools/perf/util/trace-event-scripting.c
+++ b/tools/perf/util/trace-event-scripting.c
@@ -36,11 +36,10 @@ static int stop_script_unsupported(void)
return 0;
}

-static void process_event_unsupported(int cpu __unused,
- void *data __unused,
- int size __unused,
- unsigned long long nsecs __unused,
- char *comm __unused)
+static void process_event_unsupported(union perf_event *event __unused,
+ struct perf_sample *sample __unused,
+ struct perf_session *session __unused,
+ struct thread *thread __unused)
{
}

diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index b5f12ca..5f7b513 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -3,6 +3,7 @@

#include <stdbool.h>
#include "parse-events.h"
+#include "session.h"

#define __unused __attribute__((unused))

@@ -278,8 +279,10 @@ struct scripting_ops {
const char *name;
int (*start_script) (const char *script, int argc, const char **argv);
int (*stop_script) (void);
- void (*process_event) (int cpu, void *data, int size,
- unsigned long long nsecs, char *comm);
+ void (*process_event) (union perf_event *event,
+ struct perf_sample *sample,
+ struct perf_session *session,
+ struct thread *thread);
int (*generate_script) (const char *outfile);
};

--
1.6.2.5

2011-03-09 18:32:43

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename

From: David Ahern <[email protected]>

This change does impact output: latency data is trace specific and is now
printed after the common data - comm, tid, cpu, time and event name.

Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LPU-Reference: <[email protected]>
Signed-off-by: David Ahern <[email protected]>
[ committer note: Added space after print_lat_fmt() ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-script.c | 38 ++++++++++--
tools/perf/util/trace-event-parse.c | 106 +++--------------------------------
tools/perf/util/trace-event.h | 3 +-
3 files changed, 41 insertions(+), 106 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b2bdd55..0a79da2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -20,18 +20,42 @@ static u64 last_timestamp;
static u64 nr_unordered;
extern const struct option record_options[];

+static void print_sample_start(struct perf_sample *sample,
+ struct thread *thread)
+{
+ int type;
+ struct event *event;
+ const char *evname = NULL;
+ unsigned long secs;
+ unsigned long usecs;
+ unsigned long long nsecs = sample->time;
+
+ if (latency_format)
+ printf("%8.8s-%-5d %3d", thread->comm, sample->tid, sample->cpu);
+ else
+ printf("%16s-%-5d [%03d]", thread->comm, sample->tid, sample->cpu);
+
+ secs = nsecs / NSECS_PER_SEC;
+ nsecs -= secs * NSECS_PER_SEC;
+ usecs = nsecs / NSECS_PER_USEC;
+ printf(" %5lu.%06lu: ", secs, usecs);
+
+ type = trace_parse_common_type(sample->raw_data);
+ event = trace_find_event(type);
+ if (event)
+ evname = event->name;
+
+ printf("%s: ", evname ? evname : "(unknown)");
+}
+
static void process_event(union perf_event *event __unused,
struct perf_sample *sample,
struct perf_session *session __unused,
struct thread *thread)
{
- /*
- * FIXME: better resolve from pid from the struct trace_entry
- * field, although it should be the same than this perf
- * event pid
- */
- print_event(sample->cpu, sample->raw_data, sample->raw_size,
- sample->time, thread->comm);
+ print_sample_start(sample, thread);
+ print_trace_event(sample->cpu, sample->raw_data, sample->raw_size);
+ printf("\n");
}

static int default_start_script(const char *script __unused,
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index d8e622d..ceb6b19 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -2648,63 +2648,8 @@ static void print_lat_fmt(void *data, int size __unused)
printf("%d", lock_depth);
}

-/* taken from Linux, written by Frederic Weisbecker */
-static void print_graph_cpu(int cpu)
-{
- int i;
- int log10_this = log10_cpu(cpu);
- int log10_all = log10_cpu(cpus);
-
-
- /*
- * Start with a space character - to make it stand out
- * to the right a bit when trace output is pasted into
- * email:
- */
- printf(" ");
-
- /*
- * Tricky - we space the CPU field according to the max
- * number of online CPUs. On a 2-cpu system it would take
- * a maximum of 1 digit - on a 128 cpu system it would
- * take up to 3 digits:
- */
- for (i = 0; i < log10_all - log10_this; i++)
- printf(" ");
-
- printf("%d) ", cpu);
-}
-
-#define TRACE_GRAPH_PROCINFO_LENGTH 14
#define TRACE_GRAPH_INDENT 2

-static void print_graph_proc(int pid, const char *comm)
-{
- /* sign + log10(MAX_INT) + '\0' */
- char pid_str[11];
- int spaces = 0;
- int len;
- int i;
-
- sprintf(pid_str, "%d", pid);
-
- /* 1 stands for the "-" character */
- len = strlen(comm) + strlen(pid_str) + 1;
-
- if (len < TRACE_GRAPH_PROCINFO_LENGTH)
- spaces = TRACE_GRAPH_PROCINFO_LENGTH - len;
-
- /* First spaces to align center */
- for (i = 0; i < spaces / 2; i++)
- printf(" ");
-
- printf("%s-%s", comm, pid_str);
-
- /* Last spaces to align center */
- for (i = 0; i < spaces - (spaces / 2); i++)
- printf(" ");
-}
-
static struct record *
get_return_for_leaf(int cpu, int cur_pid, unsigned long long cur_func,
struct record *next)
@@ -2876,21 +2821,13 @@ static void print_graph_nested(struct event *event, void *data)

static void
pretty_print_func_ent(void *data, int size, struct event *event,
- int cpu, int pid, const char *comm,
- unsigned long secs, unsigned long usecs)
+ int cpu, int pid)
{
struct format_field *field;
struct record *rec;
void *copy_data;
unsigned long val;

- printf("%5lu.%06lu | ", secs, usecs);
-
- print_graph_cpu(cpu);
- print_graph_proc(pid, comm);
-
- printf(" | ");
-
if (latency_format) {
print_lat_fmt(data, size);
printf(" | ");
@@ -2923,22 +2860,13 @@ out_free:
}

static void
-pretty_print_func_ret(void *data, int size __unused, struct event *event,
- int cpu, int pid, const char *comm,
- unsigned long secs, unsigned long usecs)
+pretty_print_func_ret(void *data, int size __unused, struct event *event)
{
unsigned long long rettime, calltime;
unsigned long long duration, depth;
struct format_field *field;
int i;

- printf("%5lu.%06lu | ", secs, usecs);
-
- print_graph_cpu(cpu);
- print_graph_proc(pid, comm);
-
- printf(" | ");
-
if (latency_format) {
print_lat_fmt(data, size);
printf(" | ");
@@ -2976,31 +2904,21 @@ pretty_print_func_ret(void *data, int size __unused, struct event *event,

static void
pretty_print_func_graph(void *data, int size, struct event *event,
- int cpu, int pid, const char *comm,
- unsigned long secs, unsigned long usecs)
+ int cpu, int pid)
{
if (event->flags & EVENT_FL_ISFUNCENT)
- pretty_print_func_ent(data, size, event,
- cpu, pid, comm, secs, usecs);
+ pretty_print_func_ent(data, size, event, cpu, pid);
else if (event->flags & EVENT_FL_ISFUNCRET)
- pretty_print_func_ret(data, size, event,
- cpu, pid, comm, secs, usecs);
+ pretty_print_func_ret(data, size, event);
printf("\n");
}

-void print_event(int cpu, void *data, int size, unsigned long long nsecs,
- char *comm)
+void print_trace_event(int cpu, void *data, int size)
{
struct event *event;
- unsigned long secs;
- unsigned long usecs;
int type;
int pid;

- secs = nsecs / NSECS_PER_SEC;
- nsecs -= secs * NSECS_PER_SEC;
- usecs = nsecs / NSECS_PER_USEC;
-
type = trace_parse_common_type(data);

event = trace_find_event(type);
@@ -3012,17 +2930,12 @@ void print_event(int cpu, void *data, int size, unsigned long long nsecs,
pid = trace_parse_common_pid(data);

if (event->flags & (EVENT_FL_ISFUNCENT | EVENT_FL_ISFUNCRET))
- return pretty_print_func_graph(data, size, event, cpu,
- pid, comm, secs, usecs);
+ return pretty_print_func_graph(data, size, event, cpu, pid);

if (latency_format) {
- printf("%8.8s-%-5d %3d",
- comm, pid, cpu);
print_lat_fmt(data, size);
- } else
- printf("%16s-%-5d [%03d]", comm, pid, cpu);
-
- printf(" %5lu.%06lu: %s: ", secs, usecs, event->name);
+ putchar(' ');
+ }

if (event->flags & EVENT_FL_FAILED) {
printf("EVENT '%s' FAILED TO PARSE\n",
@@ -3031,7 +2944,6 @@ void print_event(int cpu, void *data, int size, unsigned long long nsecs,
}

pretty_print(data, size, event);
- printf("\n");
}

static void print_fields(struct print_flag_sym *field)
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index 5f7b513..b04da57 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -177,8 +177,7 @@ void print_printk(void);

int parse_ftrace_file(char *buf, unsigned long size);
int parse_event_file(char *buf, unsigned long size, char *sys);
-void print_event(int cpu, void *data, int size, unsigned long long nsecs,
- char *comm);
+void print_trace_event(int cpu, void *data, int size);

extern int file_bigendian;
extern int host_bigendian;
--
1.6.2.5

2011-03-09 18:33:14

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 09/10] perf script: Add support for dumping symbols

From: David Ahern <[email protected]>

e.g., perf script -f comm,pid,tid,time,trace,sym

swapper 0/0 537.037184: prev_comm=swapper prev_pid=0 prev_prio=120...
ffffffff81030350 perf_trace_sched_switch ([kernel.kallsyms])
ffffffff81382ac5 schedule ([kernel.kallsyms])
ffffffff8100134a cpu_idle ([kernel.kallsyms])
ffffffff81370b39 rest_init ([kernel.kallsyms])
ffffffff81696c23 start_kernel ([kernel.kallsyms].init.text)
ffffffff816962af x86_64_start_reservations ([kernel.kallsyms].init.text)
ffffffff816963b9 x86_64_start_kernel ([kernel.kallsyms].init.text)

sshd 1675/1675 537.037309: prev_comm=sshd prev_pid=1675 prev_prio=120...
ffffffff81030350 perf_trace_sched_switch ([kernel.kallsyms])
ffffffff81382ac5 schedule ([kernel.kallsyms])
ffffffff813837aa schedule_hrtimeout_range_clock ([kernel.kallsyms])
ffffffff81383886 schedule_hrtimeout_range ([kernel.kallsyms])
ffffffff8110c4f9 poll_schedule_timeout ([kernel.kallsyms])
ffffffff8110cd20 do_select ([kernel.kallsyms])
ffffffff8110ced8 core_sys_select ([kernel.kallsyms])
ffffffff8110d00d sys_select ([kernel.kallsyms])
ffffffff81002bc2 system_call ([kernel.kallsyms])
7f1647e56e93 __GI_select (/lib64/libc-2.12.90.so)

netstat 1692/1692 537.038664: prev_comm=netstat prev_pid=1692 prev_prio=...
ffffffff81030350 perf_trace_sched_switch ([kernel.kallsyms])
ffffffff81382ac5 schedule ([kernel.kallsyms])
ffffffff81002c3a sysret_careful ([kernel.kallsyms])
7f7a6cd1b210 __GI___libc_read (/lib64/libc-2.12.90.so)

Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LPU-Reference: <[email protected]>
Signed-off-by: David Ahern <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf-script.txt | 16 +++++++-
tools/perf/builtin-script.c | 28 +++++++++++++-
tools/perf/util/session.c | 61 ++++++++++++++++++++++++++++++
tools/perf/util/session.h | 4 ++
4 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index b73cf58..c64118a 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -115,7 +115,21 @@ OPTIONS
-f::
--fields
Comma separated list of fields to print. Options are:
- comm, tid, pid, time, cpu, event, trace
+ comm, tid, pid, time, cpu, event, trace, sym
+
+-k::
+--vmlinux=<file>::
+ vmlinux pathname
+
+--kallsyms=<file>::
+ kallsyms pathname
+
+--symfs=<directory>::
+ Look for files with symbols relative to this directory.
+
+-G::
+--hide-call-graph::
+ When printing symbols do not display call chain.

SEE ALSO
--------
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 089bdb9..31ead6e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -19,6 +19,7 @@ static bool debug_mode;
static u64 last_timestamp;
static u64 nr_unordered;
extern const struct option record_options[];
+static bool no_callchain;

enum perf_output_field {
PERF_OUTPUT_COMM = 1U << 0,
@@ -28,6 +29,7 @@ enum perf_output_field {
PERF_OUTPUT_CPU = 1U << 4,
PERF_OUTPUT_EVNAME = 1U << 5,
PERF_OUTPUT_TRACE = 1U << 6,
+ PERF_OUTPUT_SYM = 1U << 7,
};

struct output_option {
@@ -41,6 +43,7 @@ struct output_option {
{.str = "cpu", .field = PERF_OUTPUT_CPU},
{.str = "event", .field = PERF_OUTPUT_EVNAME},
{.str = "trace", .field = PERF_OUTPUT_TRACE},
+ {.str = "sym", .field = PERF_OUTPUT_SYM},
};

/* default set to maintain compatibility with current format */
@@ -65,6 +68,8 @@ static void print_sample_start(struct perf_sample *sample,
if (PRINT_FIELD(COMM)) {
if (latency_format)
printf("%8.8s ", thread->comm);
+ else if (PRINT_FIELD(SYM) && !no_callchain)
+ printf("%s ", thread->comm);
else
printf("%16s ", thread->comm);
}
@@ -112,6 +117,14 @@ static void process_event(union perf_event *event __unused,
print_trace_event(sample->cpu, sample->raw_data,
sample->raw_size);

+ if (PRINT_FIELD(SYM)) {
+ if (no_callchain)
+ printf(" ");
+ else
+ printf("\n");
+ perf_session__print_symbols(event, sample, session);
+ }
+
printf("\n");
}

@@ -190,7 +203,10 @@ static int process_sample_event(union perf_event *event,

static struct perf_event_ops event_ops = {
.sample = process_sample_event,
+ .mmap = perf_event__process_mmap,
.comm = perf_event__process_comm,
+ .exit = perf_event__process_task,
+ .fork = perf_event__process_task,
.attr = perf_event__process_attr,
.event_type = perf_event__process_event_type,
.tracing_data = perf_event__process_tracing_data,
@@ -722,8 +738,16 @@ static const struct option options[] = {
"input file name"),
OPT_BOOLEAN('d', "debug-mode", &debug_mode,
"do various checks like samples ordering and lost events"),
+ OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
+ "file", "vmlinux pathname"),
+ OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
+ "file", "kallsyms pathname"),
+ OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
+ "When printing symbols do not display call chain"),
+ OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",
+ "Look for files with symbols relative to this directory"),
OPT_CALLBACK('f', "fields", NULL, "str",
- "comma separated output fields. Options: comm,tid,pid,time,cpu,event,trace",
+ "comma separated output fields. Options: comm,tid,pid,time,cpu,event,trace,sym",
parse_output_fields),

OPT_END()
@@ -868,6 +892,8 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
exit(-1);
}

+ symbol_conf.use_callchain = no_callchain ? false : true;
+
if (rec_script_path)
script_path = rec_script_path;
if (rep_script_path)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f642615..51fac44 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1145,3 +1145,64 @@ size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp)

return ret;
}
+
+void perf_session__print_symbols(union perf_event *event,
+ struct perf_sample *sample,
+ struct perf_session *session)
+{
+ struct addr_location al;
+ const char *symname, *dsoname;
+ struct callchain_cursor *cursor = &session->callchain_cursor;
+ struct callchain_cursor_node *node;
+
+ if (perf_event__preprocess_sample(event, session, &al, sample,
+ NULL) < 0) {
+ error("problem processing %d event, skipping it.\n",
+ event->header.type);
+ return;
+ }
+
+ if (symbol_conf.use_callchain && sample->callchain) {
+
+ if (perf_session__resolve_callchain(session, al.thread,
+ sample->callchain, NULL) != 0) {
+ if (verbose)
+ error("Failed to resolve callchain. Skipping\n");
+ return;
+ }
+ callchain_cursor_commit(cursor);
+
+ while (1) {
+ node = callchain_cursor_current(cursor);
+ if (!node)
+ break;
+
+ if (node->sym && node->sym->name)
+ symname = node->sym->name;
+ else
+ symname = "";
+
+ if (node->map && node->map->dso && node->map->dso->name)
+ dsoname = node->map->dso->name;
+ else
+ dsoname = "";
+
+ printf("\t%16" PRIx64 " %s (%s)\n", node->ip, symname, dsoname);
+
+ callchain_cursor_advance(cursor);
+ }
+
+ } else {
+ if (al.sym && al.sym->name)
+ symname = al.sym->name;
+ else
+ symname = "";
+
+ if (al.map && al.map->dso && al.map->dso->name)
+ dsoname = al.map->dso->name;
+ else
+ dsoname = "";
+
+ printf("%16" PRIx64 " %s (%s)", al.addr, symname, dsoname);
+ }
+}
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 05dd7bc..64369d3 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -161,4 +161,8 @@ static inline int perf_session__parse_sample(struct perf_session *session,
session->sample_id_all, sample);
}

+void perf_session__print_symbols(union perf_event *event,
+ struct perf_sample *sample,
+ struct perf_session *session);
+
#endif /* __PERF_SESSION_H */
--
1.6.2.5

2011-03-09 18:33:30

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 02/10] perf evsel: Assume rest of perf_header_attr functions

From: Arnaldo Carvalho de Melo <[email protected]>

The append code in record can be made smarter, i.e. allowing merging of
different sets of events, but that is left for the future.

Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-record.c | 92 +++++++++----------------
tools/perf/util/evlist.c | 26 +++++---
tools/perf/util/evlist.h | 4 +-
tools/perf/util/evsel.c | 21 +++++-
tools/perf/util/evsel.h | 9 ++-
tools/perf/util/header.c | 161 ++++++++++++------------------------------
tools/perf/util/header.h | 30 ++-------
tools/perf/util/session.c | 10 +---
8 files changed, 128 insertions(+), 225 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 9d236e8..81dbe27 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -31,7 +31,6 @@
#include <sys/mman.h>

#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
-#define SID(e, x, y) xyarray__entry(e->id, x, y)

enum write_mode_t {
WRITE_FORCE,
@@ -160,54 +159,6 @@ static void sig_atexit(void)
kill(getpid(), signr);
}

-static struct perf_header_attr *get_header_attr(struct perf_event_attr *a, int nr)
-{
- struct perf_header_attr *h_attr;
-
- if (nr < session->header.attrs) {
- h_attr = session->header.attr[nr];
- } else {
- h_attr = perf_header_attr__new(a);
- if (h_attr != NULL)
- if (perf_header__add_attr(&session->header, h_attr) < 0) {
- perf_header_attr__delete(h_attr);
- h_attr = NULL;
- }
- }
-
- return h_attr;
-}
-
-static void create_counter(struct perf_evsel *evsel, int cpu)
-{
- struct perf_event_attr *attr = &evsel->attr;
- struct perf_header_attr *h_attr;
- struct perf_sample_id *sid;
- int thread_index;
-
- for (thread_index = 0; thread_index < evsel_list->threads->nr; thread_index++) {
- h_attr = get_header_attr(attr, evsel->idx);
- if (h_attr == NULL)
- die("nomem\n");
-
- if (!file_new) {
- if (memcmp(&h_attr->attr, attr, sizeof(*attr))) {
- fprintf(stderr, "incompatible append\n");
- exit(-1);
- }
- }
-
- sid = SID(evsel, cpu, thread_index);
- if (perf_header_attr__add_id(h_attr, sid->id) < 0) {
- pr_warning("Not enough memory to add id\n");
- exit(-1);
- }
- }
-
- if (!sample_type)
- sample_type = attr->sample_type;
-}
-
static void config_attr(struct perf_evsel *evsel, struct perf_evlist *evlist)
{
struct perf_event_attr *attr = &evsel->attr;
@@ -278,10 +229,28 @@ static void config_attr(struct perf_evsel *evsel, struct perf_evlist *evlist)
}
}

+static bool perf_evlist__equal(struct perf_evlist *evlist,
+ struct perf_evlist *other)
+{
+ struct perf_evsel *pos, *pair;
+
+ if (evlist->nr_entries != other->nr_entries)
+ return false;
+
+ pair = list_entry(other->entries.next, struct perf_evsel, node);
+
+ list_for_each_entry(pos, &evlist->entries, node) {
+ if (memcmp(&pos->attr, &pair->attr, sizeof(pos->attr) != 0))
+ return false;
+ pair = list_entry(pair->node.next, struct perf_evsel, node);
+ }
+
+ return true;
+}
+
static void open_counters(struct perf_evlist *evlist)
{
struct perf_evsel *pos;
- int cpu;

list_for_each_entry(pos, &evlist->entries, node) {
struct perf_event_attr *attr = &pos->attr;
@@ -364,10 +333,16 @@ try_again:
if (perf_evlist__mmap(evlist, mmap_pages, false) < 0)
die("failed to mmap with %d (%s)\n", errno, strerror(errno));

- for (cpu = 0; cpu < evsel_list->cpus->nr; ++cpu) {
- list_for_each_entry(pos, &evlist->entries, node)
- create_counter(pos, cpu);
+ if (file_new)
+ session->evlist = evlist;
+ else {
+ if (!perf_evlist__equal(session->evlist, evlist)) {
+ fprintf(stderr, "incompatible append\n");
+ exit(-1);
+ }
}
+
+ sample_type = pos->attr.sample_type;
}

static int process_buildids(void)
@@ -390,7 +365,7 @@ static void atexit_header(void)

if (!no_buildid)
process_buildids();
- perf_header__write(&session->header, evsel_list, output, true);
+ perf_session__write_header(session, evsel_list, output, true);
perf_session__delete(session);
perf_evlist__delete(evsel_list);
symbol__exit();
@@ -600,8 +575,8 @@ static int __cmd_record(int argc, const char **argv)
if (err < 0)
return err;
} else if (file_new) {
- err = perf_header__write(&session->header, evsel_list,
- output, false);
+ err = perf_session__write_header(session, evsel_list,
+ output, false);
if (err < 0)
return err;
}
@@ -611,9 +586,8 @@ static int __cmd_record(int argc, const char **argv)
perf_session__set_sample_id_all(session, sample_id_all_avail);

if (pipe_output) {
- err = perf_event__synthesize_attrs(&session->header,
- process_synthesized_event,
- session);
+ err = perf_session__synthesize_attrs(session,
+ process_synthesized_event);
if (err < 0) {
pr_err("Couldn't synthesize attrs.\n");
return err;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 190c64c..d852cef 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -19,7 +19,7 @@
#include <linux/hash.h>

#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
-#define SID(e, x, y) xyarray__entry(e->id, x, y)
+#define SID(e, x, y) xyarray__entry(e->sample_id, x, y)

void perf_evlist__init(struct perf_evlist *evlist, struct cpu_map *cpus,
struct thread_map *threads)
@@ -106,8 +106,9 @@ void perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd)
evlist->nr_fds++;
}

-void perf_evlist__id_hash(struct perf_evlist *evlist, struct perf_evsel *evsel,
- int cpu, int thread, u64 id)
+static void perf_evlist__id_hash(struct perf_evlist *evlist,
+ struct perf_evsel *evsel,
+ int cpu, int thread, u64 id)
{
int hash;
struct perf_sample_id *sid = SID(evsel, cpu, thread);
@@ -118,9 +119,16 @@ void perf_evlist__id_hash(struct perf_evlist *evlist, struct perf_evsel *evsel,
hlist_add_head(&sid->node, &evlist->heads[hash]);
}

-static int perf_evlist__id_hash_fd(struct perf_evlist *evlist,
- struct perf_evsel *evsel,
- int cpu, int thread, int fd)
+void perf_evlist__id_add(struct perf_evlist *evlist, struct perf_evsel *evsel,
+ int cpu, int thread, u64 id)
+{
+ perf_evlist__id_hash(evlist, evsel, cpu, thread, id);
+ evsel->id[evsel->ids++] = id;
+}
+
+static int perf_evlist__id_add_fd(struct perf_evlist *evlist,
+ struct perf_evsel *evsel,
+ int cpu, int thread, int fd)
{
u64 read_data[4] = { 0, };
int id_idx = 1; /* The first entry is the counter value */
@@ -134,7 +142,7 @@ static int perf_evlist__id_hash_fd(struct perf_evlist *evlist,
if (evsel->attr.read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
++id_idx;

- perf_evlist__id_hash(evlist, evsel, cpu, thread, read_data[id_idx]);
+ perf_evlist__id_add(evlist, evsel, cpu, thread, read_data[id_idx]);
return 0;
}

@@ -292,7 +300,7 @@ int perf_evlist__mmap(struct perf_evlist *evlist, int pages, bool overwrite)

list_for_each_entry(evsel, &evlist->entries, node) {
if ((evsel->attr.read_format & PERF_FORMAT_ID) &&
- evsel->id == NULL &&
+ evsel->sample_id == NULL &&
perf_evsel__alloc_id(evsel, cpus->nr, threads->nr) < 0)
return -ENOMEM;

@@ -308,7 +316,7 @@ int perf_evlist__mmap(struct perf_evlist *evlist, int pages, bool overwrite)
goto out_unmap;

if ((evsel->attr.read_format & PERF_FORMAT_ID) &&
- perf_evlist__id_hash_fd(evlist, evsel, cpu, thread, fd) < 0)
+ perf_evlist__id_add_fd(evlist, evsel, cpu, thread, fd) < 0)
goto out_unmap;
}
}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 078d512..8b1cb7a 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -38,8 +38,8 @@ void perf_evlist__delete(struct perf_evlist *evlist);
void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry);
int perf_evlist__add_default(struct perf_evlist *evlist);

-void perf_evlist__id_hash(struct perf_evlist *evlist, struct perf_evsel *evsel,
- int cpu, int thread, u64 id);
+void perf_evlist__id_add(struct perf_evlist *evlist, struct perf_evsel *evsel,
+ int cpu, int thread, u64 id);

int perf_evlist__alloc_pollfd(struct perf_evlist *evlist);
void perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 8083d51..662596a 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -41,8 +41,18 @@ int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)

int perf_evsel__alloc_id(struct perf_evsel *evsel, int ncpus, int nthreads)
{
- evsel->id = xyarray__new(ncpus, nthreads, sizeof(struct perf_sample_id));
- return evsel->id != NULL ? 0 : -ENOMEM;
+ evsel->sample_id = xyarray__new(ncpus, nthreads, sizeof(struct perf_sample_id));
+ if (evsel->sample_id == NULL)
+ return -ENOMEM;
+
+ evsel->id = zalloc(ncpus * nthreads * sizeof(u64));
+ if (evsel->id == NULL) {
+ xyarray__delete(evsel->sample_id);
+ evsel->sample_id = NULL;
+ return -ENOMEM;
+ }
+
+ return 0;
}

int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus)
@@ -60,7 +70,9 @@ void perf_evsel__free_fd(struct perf_evsel *evsel)

void perf_evsel__free_id(struct perf_evsel *evsel)
{
- xyarray__delete(evsel->id);
+ xyarray__delete(evsel->sample_id);
+ evsel->sample_id = NULL;
+ free(evsel->id);
evsel->id = NULL;
}

@@ -79,7 +91,8 @@ void perf_evsel__exit(struct perf_evsel *evsel)
{
assert(list_empty(&evsel->node));
xyarray__delete(evsel->fd);
- xyarray__delete(evsel->id);
+ xyarray__delete(evsel->sample_id);
+ free(evsel->id);
}

void perf_evsel__delete(struct perf_evsel *evsel)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 281b60e..6710ab5 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -49,12 +49,17 @@ struct perf_evsel {
struct perf_event_attr attr;
char *filter;
struct xyarray *fd;
- struct xyarray *id;
+ struct xyarray *sample_id;
+ u64 *id;
struct perf_counts *counts;
int idx;
+ int ids;
struct hists hists;
char *name;
- void *priv;
+ union {
+ void *priv;
+ off_t id_offset;
+ };
struct cgroup_sel *cgrp;
};

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 3457ec6..40b10e4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -20,89 +20,6 @@

static bool no_buildid_cache = false;

-/*
- * Create new perf.data header attribute:
- */
-struct perf_header_attr *perf_header_attr__new(struct perf_event_attr *attr)
-{
- struct perf_header_attr *self = malloc(sizeof(*self));
-
- if (self != NULL) {
- self->attr = *attr;
- self->ids = 0;
- self->size = 1;
- self->id = malloc(sizeof(u64));
- if (self->id == NULL) {
- free(self);
- self = NULL;
- }
- }
-
- return self;
-}
-
-void perf_header_attr__delete(struct perf_header_attr *self)
-{
- free(self->id);
- free(self);
-}
-
-int perf_header_attr__add_id(struct perf_header_attr *self, u64 id)
-{
- int pos = self->ids;
-
- self->ids++;
- if (self->ids > self->size) {
- int nsize = self->size * 2;
- u64 *nid = realloc(self->id, nsize * sizeof(u64));
-
- if (nid == NULL)
- return -1;
-
- self->size = nsize;
- self->id = nid;
- }
- self->id[pos] = id;
- return 0;
-}
-
-int perf_header__init(struct perf_header *self)
-{
- self->size = 1;
- self->attr = malloc(sizeof(void *));
- return self->attr == NULL ? -ENOMEM : 0;
-}
-
-void perf_header__exit(struct perf_header *self)
-{
- int i;
- for (i = 0; i < self->attrs; ++i)
- perf_header_attr__delete(self->attr[i]);
- free(self->attr);
-}
-
-int perf_header__add_attr(struct perf_header *self,
- struct perf_header_attr *attr)
-{
- if (self->frozen)
- return -1;
-
- if (self->attrs == self->size) {
- int nsize = self->size * 2;
- struct perf_header_attr **nattr;
-
- nattr = realloc(self->attr, nsize * sizeof(void *));
- if (nattr == NULL)
- return -1;
-
- self->size = nsize;
- self->attr = nattr;
- }
-
- self->attr[self->attrs++] = attr;
- return 0;
-}
-
static int event_count;
static struct perf_trace_event_type *events;

@@ -516,33 +433,41 @@ int perf_header__write_pipe(int fd)
return 0;
}

-int perf_header__write(struct perf_header *self, struct perf_evlist *evlist,
- int fd, bool at_exit)
+int perf_session__write_header(struct perf_session *session,
+ struct perf_evlist *evlist,
+ int fd, bool at_exit)
{
struct perf_file_header f_header;
struct perf_file_attr f_attr;
- struct perf_header_attr *attr;
- int i, err;
+ struct perf_header *self = &session->header;
+ struct perf_evsel *attr, *pair = NULL;
+ int err;

lseek(fd, sizeof(f_header), SEEK_SET);

- for (i = 0; i < self->attrs; i++) {
- attr = self->attr[i];
+ if (session->evlist != evlist)
+ pair = list_entry(session->evlist->entries.next, struct perf_evsel, node);

+ list_for_each_entry(attr, &evlist->entries, node) {
attr->id_offset = lseek(fd, 0, SEEK_CUR);
err = do_write(fd, attr->id, attr->ids * sizeof(u64));
if (err < 0) {
+out_err_write:
pr_debug("failed to write perf header\n");
return err;
}
+ if (session->evlist != evlist) {
+ err = do_write(fd, pair->id, pair->ids * sizeof(u64));
+ if (err < 0)
+ goto out_err_write;
+ attr->ids += pair->ids;
+ pair = list_entry(pair->node.next, struct perf_evsel, node);
+ }
}

-
self->attr_offset = lseek(fd, 0, SEEK_CUR);

- for (i = 0; i < self->attrs; i++) {
- attr = self->attr[i];
-
+ list_for_each_entry(attr, &evlist->entries, node) {
f_attr = (struct perf_file_attr){
.attr = attr->attr,
.ids = {
@@ -581,7 +506,7 @@ int perf_header__write(struct perf_header *self, struct perf_evlist *evlist,
.attr_size = sizeof(f_attr),
.attrs = {
.offset = self->attr_offset,
- .size = self->attrs * sizeof(f_attr),
+ .size = evlist->nr_entries * sizeof(f_attr),
},
.data = {
.offset = self->data_offset,
@@ -918,7 +843,7 @@ int perf_session__read_header(struct perf_session *session, int fd)
if (perf_header__getbuffer64(self, fd, &f_id, sizeof(f_id)))
goto out_errno;

- perf_evlist__id_hash(session->evlist, evsel, 0, j, f_id);
+ perf_evlist__id_add(session->evlist, evsel, 0, j, f_id);
}

lseek(fd, tmp, SEEK_SET);
@@ -1012,16 +937,13 @@ int perf_event__synthesize_attr(struct perf_event_attr *attr, u16 ids, u64 *id,
return err;
}

-int perf_event__synthesize_attrs(struct perf_header *self,
- perf_event__handler_t process,
- struct perf_session *session)
+int perf_session__synthesize_attrs(struct perf_session *session,
+ perf_event__handler_t process)
{
- struct perf_header_attr *attr;
- int i, err = 0;
-
- for (i = 0; i < self->attrs; i++) {
- attr = self->attr[i];
+ struct perf_evsel *attr;
+ int err = 0;

+ list_for_each_entry(attr, &session->evlist->entries, node) {
err = perf_event__synthesize_attr(&attr->attr, attr->ids,
attr->id, process, session);
if (err) {
@@ -1036,27 +958,36 @@ int perf_event__synthesize_attrs(struct perf_header *self,
int perf_event__process_attr(union perf_event *event,
struct perf_session *session)
{
- struct perf_header_attr *attr;
unsigned int i, ids, n_ids;
+ struct perf_evsel *evsel;

- attr = perf_header_attr__new(&event->attr.attr);
- if (attr == NULL)
+ if (session->evlist == NULL) {
+ session->evlist = perf_evlist__new(NULL, NULL);
+ if (session->evlist == NULL)
+ return -ENOMEM;
+ }
+
+ evsel = perf_evsel__new(&event->attr.attr,
+ session->evlist->nr_entries);
+ if (evsel == NULL)
return -ENOMEM;

+ perf_evlist__add(session->evlist, evsel);
+
ids = event->header.size;
ids -= (void *)&event->attr.id - (void *)event;
n_ids = ids / sizeof(u64);
+ /*
+ * We don't have the cpu and thread maps on the header, so
+ * for allocating the perf_sample_id table we fake 1 cpu and
+ * hattr->ids threads.
+ */
+ if (perf_evsel__alloc_id(evsel, 1, n_ids))
+ return -ENOMEM;

for (i = 0; i < n_ids; i++) {
- if (perf_header_attr__add_id(attr, event->attr.id[i]) < 0) {
- perf_header_attr__delete(attr);
- return -ENOMEM;
- }
- }
-
- if (perf_header__add_attr(&session->header, attr) < 0) {
- perf_header_attr__delete(attr);
- return -ENOMEM;
+ perf_evlist__id_add(session->evlist, evsel, 0, i,
+ event->attr.id[i]);
}

perf_session__update_sample_type(session);
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 73b84eb..4cc2675 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -9,13 +9,6 @@

#include <linux/bitmap.h>

-struct perf_header_attr {
- struct perf_event_attr attr;
- int ids, size;
- u64 *id;
- off_t id_offset;
-};
-
enum {
HEADER_TRACE_INFO = 1,
HEADER_BUILD_ID,
@@ -51,9 +44,7 @@ int perf_file_header__read(struct perf_file_header *self,

struct perf_header {
int frozen;
- int attrs, size;
bool needs_swap;
- struct perf_header_attr **attr;
s64 attr_offset;
u64 data_offset;
u64 data_size;
@@ -62,27 +53,17 @@ struct perf_header {
DECLARE_BITMAP(adds_features, HEADER_FEAT_BITS);
};

-int perf_header__init(struct perf_header *self);
-void perf_header__exit(struct perf_header *self);
-
struct perf_evlist;

int perf_session__read_header(struct perf_session *session, int fd);
-int perf_header__write(struct perf_header *self, struct perf_evlist *evlist,
- int fd, bool at_exit);
+int perf_session__write_header(struct perf_session *session,
+ struct perf_evlist *evlist,
+ int fd, bool at_exit);
int perf_header__write_pipe(int fd);

-int perf_header__add_attr(struct perf_header *self,
- struct perf_header_attr *attr);
-
int perf_header__push_event(u64 id, const char *name);
char *perf_header__find_event(u64 id);

-struct perf_header_attr *perf_header_attr__new(struct perf_event_attr *attr);
-void perf_header_attr__delete(struct perf_header_attr *self);
-
-int perf_header_attr__add_id(struct perf_header_attr *self, u64 id);
-
u64 perf_evlist__sample_type(struct perf_evlist *evlist);
bool perf_evlist__sample_id_all(const struct perf_evlist *evlist);
void perf_header__set_feat(struct perf_header *self, int feat);
@@ -101,9 +82,8 @@ int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir);
int perf_event__synthesize_attr(struct perf_event_attr *attr, u16 ids, u64 *id,
perf_event__handler_t process,
struct perf_session *session);
-int perf_event__synthesize_attrs(struct perf_header *self,
- perf_event__handler_t process,
- struct perf_session *session);
+int perf_session__synthesize_attrs(struct perf_session *session,
+ perf_event__handler_t process);
int perf_event__process_attr(union perf_event *event, struct perf_session *session);

int perf_event__synthesize_event_type(u64 event_id, char *name,
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 26b24c5..f642615 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -137,9 +137,6 @@ struct perf_session *perf_session__new(const char *filename, int mode,
if (self == NULL)
goto out;

- if (perf_header__init(&self->header) < 0)
- goto out_free;
-
memcpy(self->filename, filename, len);
self->threads = RB_ROOT;
INIT_LIST_HEAD(&self->dead_threads);
@@ -163,6 +160,7 @@ struct perf_session *perf_session__new(const char *filename, int mode,
if (mode == O_RDONLY) {
if (perf_session__open(self, force) < 0)
goto out_delete;
+ perf_session__update_sample_type(self);
} else if (mode == O_WRONLY) {
/*
* In O_RDONLY mode this will be performed when reading the
@@ -172,8 +170,6 @@ struct perf_session *perf_session__new(const char *filename, int mode,
goto out_delete;
}

- perf_session__update_sample_type(self);
-
if (ops && ops->ordering_requires_timestamps &&
ops->ordered_samples && !self->sample_id_all) {
dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
@@ -182,9 +178,6 @@ struct perf_session *perf_session__new(const char *filename, int mode,

out:
return self;
-out_free:
- free(self);
- return NULL;
out_delete:
perf_session__delete(self);
return NULL;
@@ -215,7 +208,6 @@ static void perf_session__delete_threads(struct perf_session *self)

void perf_session__delete(struct perf_session *self)
{
- perf_header__exit(&self->header);
perf_session__destroy_kernel_maps(self);
perf_session__delete_dead_threads(self);
perf_session__delete_threads(self);
--
1.6.2.5

2011-03-09 18:33:49

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 05/10] perf top: Don't let events to eat up whole header line

From: Jiri Olsa <[email protected]>

Passing multiple events might force out information about pid/tid/cpu.
Attached patch leaves 30 characters for this info at the expense of the
events' names.

Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Han Pingtian <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/top.c | 20 +++++++++++++++++---
1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index 4f869da..75cfe4d 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -115,9 +115,23 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
if (!top->display_weighted) {
ret += SNPRINTF(bf + ret, size - ret, "%s",
event_name(top->sym_evsel));
- } else list_for_each_entry(counter, &top->evlist->entries, node) {
- ret += SNPRINTF(bf + ret, size - ret, "%s%s",
- counter->idx ? "/" : "", event_name(counter));
+ } else {
+ /*
+ * Don't let events eat all the space. Leaving 30 bytes
+ * for the rest should be enough.
+ */
+ size_t last_pos = size - 30;
+
+ list_for_each_entry(counter, &top->evlist->entries, node) {
+ ret += SNPRINTF(bf + ret, size - ret, "%s%s",
+ counter->idx ? "/" : "",
+ event_name(counter));
+ if (ret > last_pos) {
+ sprintf(bf + last_pos - 3, "..");
+ ret = last_pos - 1;
+ break;
+ }
+ }
}

ret += SNPRINTF(bf + ret, size - ret, "], ");
--
1.6.2.5

2011-03-09 23:30:46

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 09/10] perf script: Add support for dumping symbols

On Wed, Mar 09, 2011 at 03:31:30PM -0300, Arnaldo Carvalho de Melo wrote:
> @@ -868,6 +892,8 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
> exit(-1);
> }
>
> + symbol_conf.use_callchain = no_callchain ? false : true;
> +

That also depend on (sample_type & PERF_SAMPLE_CALLCHAIN)...

> if (rec_script_path)
> script_path = rec_script_path;
> if (rep_script_path)
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index f642615..51fac44 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1145,3 +1145,64 @@ size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp)
>
> return ret;
> }
> +
> +void perf_session__print_symbols(union perf_event *event,
> + struct perf_sample *sample,
> + struct perf_session *session)
> +{
> + struct addr_location al;
> + const char *symname, *dsoname;
> + struct callchain_cursor *cursor = &session->callchain_cursor;
> + struct callchain_cursor_node *node;
> +
> + if (perf_event__preprocess_sample(event, session, &al, sample,
> + NULL) < 0) {
> + error("problem processing %d event, skipping it.\n",
> + event->header.type);
> + return;
> + }
> +
> + if (symbol_conf.use_callchain && sample->callchain) {

...otherwise you may deref some crap there. sample->callchain is
random when there is actually no callchain.

> +
> + if (perf_session__resolve_callchain(session, al.thread,
> + sample->callchain, NULL) != 0) {
> + if (verbose)
> + error("Failed to resolve callchain. Skipping\n");
> + return;
> + }

2011-03-09 23:50:19

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename

On Wed, Mar 09, 2011 at 03:31:28PM -0300, Arnaldo Carvalho de Melo wrote:
> From: David Ahern <[email protected]>
>
> This change does impact output: latency data is trace specific and is now
> printed after the common data - comm, tid, cpu, time and event name.
>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> LPU-Reference: <[email protected]>
> Signed-off-by: David Ahern <[email protected]>
> [ committer note: Added space after print_lat_fmt() ]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/perf/builtin-script.c | 38 ++++++++++--
> tools/perf/util/trace-event-parse.c | 106 +++--------------------------------
> tools/perf/util/trace-event.h | 3 +-
> 3 files changed, 41 insertions(+), 106 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index b2bdd55..0a79da2 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -20,18 +20,42 @@ static u64 last_timestamp;
> static u64 nr_unordered;
> extern const struct option record_options[];
>
> +static void print_sample_start(struct perf_sample *sample,
> + struct thread *thread)
> +{
> + int type;
> + struct event *event;
> + const char *evname = NULL;
> + unsigned long secs;
> + unsigned long usecs;
> + unsigned long long nsecs = sample->time;
> +
> + if (latency_format)

I believe we don't yet make use of the latency format with perf.
Like the function graph output, it was included in that code coming
from trace-cmd.

We might use it in the future though, so it looks fine to keep it
and handle it like you did.

> + printf("%8.8s-%-5d %3d", thread->comm, sample->tid, sample->cpu);
> + else
> + printf("%16s-%-5d [%03d]", thread->comm, sample->tid, sample->cpu);
> +
> + secs = nsecs / NSECS_PER_SEC;
> + nsecs -= secs * NSECS_PER_SEC;
> + usecs = nsecs / NSECS_PER_USEC;
> + printf(" %5lu.%06lu: ", secs, usecs);
> +
> + type = trace_parse_common_type(sample->raw_data);
> + event = trace_find_event(type);
> + if (event)
> + evname = event->name;
> +
> + printf("%s: ", evname ? evname : "(unknown)");
> +}
> +
[...]
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -2648,63 +2648,8 @@ static void print_lat_fmt(void *data, int size __unused)
> printf("%d", lock_depth);
> }
>
> -/* taken from Linux, written by Frederic Weisbecker */
> -static void print_graph_cpu(int cpu)
> -{
> - int i;
> - int log10_this = log10_cpu(cpu);
> - int log10_all = log10_cpu(cpus);
> -
> -
> - /*
> - * Start with a space character - to make it stand out
> - * to the right a bit when trace output is pasted into
> - * email:
> - */
> - printf(" ");
> -
> - /*
> - * Tricky - we space the CPU field according to the max
> - * number of online CPUs. On a 2-cpu system it would take
> - * a maximum of 1 digit - on a 128 cpu system it would
> - * take up to 3 digits:
> - */
> - for (i = 0; i < log10_all - log10_this; i++)
> - printf(" ");
> -
> - printf("%d) ", cpu);
> -}

So, we indeed don't use the function graph tracer with perf yet.
But there are fair chances we will in the future.

So if we remove such code, I would prefer this to be made as
a seperate commit. Something we can easily retrieve and revert
in the future.

Other than that and the callchain bug, the whole series looks
pretty good now.

Thanks a lot!

2011-03-10 00:04:40

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename



On 03/09/11 16:50, Frederic Weisbecker wrote:

>> +++ b/tools/perf/util/trace-event-parse.c
>> @@ -2648,63 +2648,8 @@ static void print_lat_fmt(void *data, int size __unused)
>> printf("%d", lock_depth);
>> }
>>
>> -/* taken from Linux, written by Frederic Weisbecker */
>> -static void print_graph_cpu(int cpu)
>> -{
>> - int i;
>> - int log10_this = log10_cpu(cpu);
>> - int log10_all = log10_cpu(cpus);
>> -
>> -
>> - /*
>> - * Start with a space character - to make it stand out
>> - * to the right a bit when trace output is pasted into
>> - * email:
>> - */
>> - printf(" ");
>> -
>> - /*
>> - * Tricky - we space the CPU field according to the max
>> - * number of online CPUs. On a 2-cpu system it would take
>> - * a maximum of 1 digit - on a 128 cpu system it would
>> - * take up to 3 digits:
>> - */
>> - for (i = 0; i < log10_all - log10_this; i++)
>> - printf(" ");
>> -
>> - printf("%d) ", cpu);
>> -}
>
> So, we indeed don't use the function graph tracer with perf yet.
> But there are fair chances we will in the future.
>
> So if we remove such code, I would prefer this to be made as
> a seperate commit. Something we can easily retrieve and revert
> in the future.

Once the references to the functions are removed, compile fails --
functions defined without a user.

I left the cpu arg into print_trace_event to avoid having to delete even
more code because of that.

David


>
> Other than that and the callchain bug, the whole series looks
> pretty good now.
>
> Thanks a lot!

2011-03-10 00:10:48

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename

On Wed, Mar 09, 2011 at 05:04:31PM -0700, David Ahern wrote:
>
>
> On 03/09/11 16:50, Frederic Weisbecker wrote:
>
> >> +++ b/tools/perf/util/trace-event-parse.c
> >> @@ -2648,63 +2648,8 @@ static void print_lat_fmt(void *data, int size __unused)
> >> printf("%d", lock_depth);
> >> }
> >>
> >> -/* taken from Linux, written by Frederic Weisbecker */
> >> -static void print_graph_cpu(int cpu)
> >> -{
> >> - int i;
> >> - int log10_this = log10_cpu(cpu);
> >> - int log10_all = log10_cpu(cpus);
> >> -
> >> -
> >> - /*
> >> - * Start with a space character - to make it stand out
> >> - * to the right a bit when trace output is pasted into
> >> - * email:
> >> - */
> >> - printf(" ");
> >> -
> >> - /*
> >> - * Tricky - we space the CPU field according to the max
> >> - * number of online CPUs. On a 2-cpu system it would take
> >> - * a maximum of 1 digit - on a 128 cpu system it would
> >> - * take up to 3 digits:
> >> - */
> >> - for (i = 0; i < log10_all - log10_this; i++)
> >> - printf(" ");
> >> -
> >> - printf("%d) ", cpu);
> >> -}
> >
> > So, we indeed don't use the function graph tracer with perf yet.
> > But there are fair chances we will in the future.
> >
> > So if we remove such code, I would prefer this to be made as
> > a seperate commit. Something we can easily retrieve and revert
> > in the future.
>
> Once the references to the functions are removed, compile fails --
> functions defined without a user.
>
> I left the cpu arg into print_trace_event to avoid having to delete even
> more code because of that.

And if you actually keep those functions in place?

2011-03-10 00:11:21

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename



On 03/09/11 17:10, Frederic Weisbecker wrote:
> On Wed, Mar 09, 2011 at 05:04:31PM -0700, David Ahern wrote:
>>
>>
>> On 03/09/11 16:50, Frederic Weisbecker wrote:
>>
>>>> +++ b/tools/perf/util/trace-event-parse.c
>>>> @@ -2648,63 +2648,8 @@ static void print_lat_fmt(void *data, int size __unused)
>>>> printf("%d", lock_depth);
>>>> }
>>>>
>>>> -/* taken from Linux, written by Frederic Weisbecker */
>>>> -static void print_graph_cpu(int cpu)
>>>> -{
>>>> - int i;
>>>> - int log10_this = log10_cpu(cpu);
>>>> - int log10_all = log10_cpu(cpus);
>>>> -
>>>> -
>>>> - /*
>>>> - * Start with a space character - to make it stand out
>>>> - * to the right a bit when trace output is pasted into
>>>> - * email:
>>>> - */
>>>> - printf(" ");
>>>> -
>>>> - /*
>>>> - * Tricky - we space the CPU field according to the max
>>>> - * number of online CPUs. On a 2-cpu system it would take
>>>> - * a maximum of 1 digit - on a 128 cpu system it would
>>>> - * take up to 3 digits:
>>>> - */
>>>> - for (i = 0; i < log10_all - log10_this; i++)
>>>> - printf(" ");
>>>> -
>>>> - printf("%d) ", cpu);
>>>> -}
>>>
>>> So, we indeed don't use the function graph tracer with perf yet.
>>> But there are fair chances we will in the future.
>>>
>>> So if we remove such code, I would prefer this to be made as
>>> a seperate commit. Something we can easily retrieve and revert
>>> in the future.
>>
>> Once the references to the functions are removed, compile fails --
>> functions defined without a user.
>>
>> I left the cpu arg into print_trace_event to avoid having to delete even
>> more code because of that.
>
> And if you actually keep those functions in place?

Compile fails.

2011-03-10 00:15:01

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename

On Wed, Mar 09, 2011 at 05:11:55PM -0700, David Ahern wrote:
>
>
> On 03/09/11 17:10, Frederic Weisbecker wrote:
> > On Wed, Mar 09, 2011 at 05:04:31PM -0700, David Ahern wrote:
> >>
> >>
> >> On 03/09/11 16:50, Frederic Weisbecker wrote:
> >>
> >>>> +++ b/tools/perf/util/trace-event-parse.c
> >>>> @@ -2648,63 +2648,8 @@ static void print_lat_fmt(void *data, int size __unused)
> >>>> printf("%d", lock_depth);
> >>>> }
> >>>>
> >>>> -/* taken from Linux, written by Frederic Weisbecker */
> >>>> -static void print_graph_cpu(int cpu)
> >>>> -{
> >>>> - int i;
> >>>> - int log10_this = log10_cpu(cpu);
> >>>> - int log10_all = log10_cpu(cpus);
> >>>> -
> >>>> -
> >>>> - /*
> >>>> - * Start with a space character - to make it stand out
> >>>> - * to the right a bit when trace output is pasted into
> >>>> - * email:
> >>>> - */
> >>>> - printf(" ");
> >>>> -
> >>>> - /*
> >>>> - * Tricky - we space the CPU field according to the max
> >>>> - * number of online CPUs. On a 2-cpu system it would take
> >>>> - * a maximum of 1 digit - on a 128 cpu system it would
> >>>> - * take up to 3 digits:
> >>>> - */
> >>>> - for (i = 0; i < log10_all - log10_this; i++)
> >>>> - printf(" ");
> >>>> -
> >>>> - printf("%d) ", cpu);
> >>>> -}
> >>>
> >>> So, we indeed don't use the function graph tracer with perf yet.
> >>> But there are fair chances we will in the future.
> >>>
> >>> So if we remove such code, I would prefer this to be made as
> >>> a seperate commit. Something we can easily retrieve and revert
> >>> in the future.
> >>
> >> Once the references to the functions are removed, compile fails --
> >> functions defined without a user.
> >>
> >> I left the cpu arg into print_trace_event to avoid having to delete even
> >> more code because of that.
> >
> > And if you actually keep those functions in place?
>
> Compile fails.

Ok, but why does it fail? :)

You can probably keep their callsites, like before they simply won't happen.

2011-03-10 00:21:24

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH 09/10] perf script: Add support for dumping symbols



On 03/09/11 16:30, Frederic Weisbecker wrote:
> On Wed, Mar 09, 2011 at 03:31:30PM -0300, Arnaldo Carvalho de Melo wrote:
>> @@ -868,6 +892,8 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
>> exit(-1);
>> }
>>
>> + symbol_conf.use_callchain = no_callchain ? false : true;
>> +
>
> That also depend on (sample_type & PERF_SAMPLE_CALLCHAIN)...
>
>> if (rec_script_path)
>> script_path = rec_script_path;
>> if (rep_script_path)
>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> index f642615..51fac44 100644
>> --- a/tools/perf/util/session.c
>> +++ b/tools/perf/util/session.c
>> @@ -1145,3 +1145,64 @@ size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp)
>>
>> return ret;
>> }
>> +
>> +void perf_session__print_symbols(union perf_event *event,
>> + struct perf_sample *sample,
>> + struct perf_session *session)
>> +{
>> + struct addr_location al;
>> + const char *symname, *dsoname;
>> + struct callchain_cursor *cursor = &session->callchain_cursor;
>> + struct callchain_cursor_node *node;
>> +
>> + if (perf_event__preprocess_sample(event, session, &al, sample,
>> + NULL) < 0) {
>> + error("problem processing %d event, skipping it.\n",
>> + event->header.type);
>> + return;
>> + }
>> +
>> + if (symbol_conf.use_callchain && sample->callchain) {
>
> ...otherwise you may deref some crap there. sample->callchain is
> random when there is actually no callchain.

Doh. I'll add a check to perf_session__check_attr for
PERF_SAMPLE_CALLCHAIN and reset use_callchain if necessary.

David

2011-03-10 00:22:47

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename

On Wed, Mar 09, 2011 at 05:15:00PM -0700, David Ahern wrote:
>
>
> On 03/09/11 17:10, Frederic Weisbecker wrote:
> >
> > And if you actually keep those functions in place?
>
> CC /tmp/build-perf/util/trace-event-read.o
> cc1: warnings being treated as errors
> util/trace-event-parse.c:2652:13: error: 'print_graph_cpu' defined but
> not used
> util/trace-event-parse.c:2681:13: error: 'print_graph_proc' defined but
> not used
> make: *** [/tmp/build-perf/util/trace-event-parse.o] Error 1
> make: *** Waiting for unfinished jobs....

All right, that's because you removed their calls in pretty_print_func_ent().
So either you remove the whole in a specific patch, pretty_print_func_ent()
included and other related functions, or you keep them.

But I prefer we don't do something halfway, and in particular not in a
semi-hidden way inside a patch that is not particularly focused on that
purpose.

2011-03-10 00:31:46

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename



On 03/09/11 17:22, Frederic Weisbecker wrote:
> On Wed, Mar 09, 2011 at 05:15:00PM -0700, David Ahern wrote:
>>
>>
>> On 03/09/11 17:10, Frederic Weisbecker wrote:
>>>
>>> And if you actually keep those functions in place?
>>
>> CC /tmp/build-perf/util/trace-event-read.o
>> cc1: warnings being treated as errors
>> util/trace-event-parse.c:2652:13: error: 'print_graph_cpu' defined but
>> not used
>> util/trace-event-parse.c:2681:13: error: 'print_graph_proc' defined but
>> not used
>> make: *** [/tmp/build-perf/util/trace-event-parse.o] Error 1
>> make: *** Waiting for unfinished jobs....
>
> All right, that's because you removed their calls in pretty_print_func_ent().
> So either you remove the whole in a specific patch, pretty_print_func_ent()
> included and other related functions, or you keep them.
>
> But I prefer we don't do something halfway, and in particular not in a
> semi-hidden way inside a patch that is not particularly focused on that
> purpose.

You lost me on the halfway part.

So you want a separate patch that removes the code for an incomplete
feature -- which means changing the references to the functions in that
patch? The intent being a patch that can be reverted later?

What about the cpu argument which was only kept in print_trace_event to
avoid the removal of more code? Leave it and the additional code or take
it out?

David

2011-03-10 00:50:10

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 07/10] perf script: Move printing of 'common' data from print_event and rename

On Wed, Mar 09, 2011 at 05:32:22PM -0700, David Ahern wrote:
>
>
> On 03/09/11 17:22, Frederic Weisbecker wrote:
> > On Wed, Mar 09, 2011 at 05:15:00PM -0700, David Ahern wrote:
> >>
> >>
> >> On 03/09/11 17:10, Frederic Weisbecker wrote:
> >>>
> >>> And if you actually keep those functions in place?
> >>
> >> CC /tmp/build-perf/util/trace-event-read.o
> >> cc1: warnings being treated as errors
> >> util/trace-event-parse.c:2652:13: error: 'print_graph_cpu' defined but
> >> not used
> >> util/trace-event-parse.c:2681:13: error: 'print_graph_proc' defined but
> >> not used
> >> make: *** [/tmp/build-perf/util/trace-event-parse.o] Error 1
> >> make: *** Waiting for unfinished jobs....
> >
> > All right, that's because you removed their calls in pretty_print_func_ent().
> > So either you remove the whole in a specific patch, pretty_print_func_ent()
> > included and other related functions, or you keep them.
> >
> > But I prefer we don't do something halfway, and in particular not in a
> > semi-hidden way inside a patch that is not particularly focused on that
> > purpose.
>
> You lost me on the halfway part.
>
> So you want a separate patch that removes the code for an incomplete
> feature -- which means changing the references to the functions in that
> patch?

Yep.

> The intent being a patch that can be reverted later?

That can be reverted or something on top of which we can refer to
get that feature later.

In any case it is deadcode right now. If you remove it it should be
better done seperately. For all the reasons we always prefer to have
patches that do only one logic thing: easier to review, understand,
bisect, revert, find a bug, explain it, etc...

Ok, in fact you can actually keep it like you did: remove the cpu and
proc displays and let the rest of the function graph features. But
at least do it in an isolated patch because that should not be
an unnoticeable change, and for other reasons to make it a standalone
patch quoted above.