Re: [PATCH] perf: implement recording/reporting per-cpu samples

From: Arnaldo Carvalho de Melo
Date: Thu May 27 2010 - 19:16:24 EST


Em Thu, May 27, 2010 at 06:53:33PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, May 27, 2010 at 01:54:46PM -0700, Arun Sharma escreveu:
> > Thanks for taking care of the second part.
>
> Will try to get it done now and will send for review.

event__preprocess_sample should eventually digest event__parse_sample,
I guess, goal being to separate out what is common to evergrowing set of
perf commands.

Can you see any holes in this one?

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 96db524..9542295 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -65,7 +65,7 @@ static int process_sample_event(event_t *event, struct perf_session *session)
dump_printf("(IP, %d): %d: %#Lx\n", event->header.misc,
event->ip.pid, event->ip.ip);

- if (event__preprocess_sample(event, session, &al, NULL) < 0) {
+ if (event__preprocess_sample(event, NULL, session, &al, NULL) < 0) {
pr_warning("problem processing %d event, skipping it.\n",
event->header.type);
return -1;
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index a6e2fdc..1608629 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -38,7 +38,9 @@ static int diff__process_sample_event(event_t *event, struct perf_session *sessi
dump_printf("(IP, %d): %d: %#Lx\n", event->header.misc,
event->ip.pid, event->ip.ip);

- if (event__preprocess_sample(event, session, &al, NULL) < 0) {
+ event__parse_sample(event, session->sample_type, &data);
+
+ if (event__preprocess_sample(event, &data, session, &al, NULL) < 0) {
pr_warning("problem processing %d event, skipping it.\n",
event->header.type);
return -1;
@@ -47,8 +49,6 @@ static int diff__process_sample_event(event_t *event, struct perf_session *sessi
if (al.filtered || al.sym == NULL)
return 0;

- event__parse_sample(event, session->sample_type, &data);
-
if (hists__add_entry(&session->hists, &al, data.period)) {
pr_warning("problem incrementing symbol period, skipping event\n");
return -1;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 9bc8905..73f8122 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -274,6 +274,9 @@ static void create_counter(int counter, int cpu)
if (call_graph)
attr->sample_type |= PERF_SAMPLE_CALLCHAIN;

+ if (system_wide)
+ attr->sample_type |= PERF_SAMPLE_CPU;
+
if (raw_samples) {
attr->sample_type |= PERF_SAMPLE_TIME;
attr->sample_type |= PERF_SAMPLE_RAW;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3592057..bb0859f 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -157,8 +157,8 @@ static int process_sample_event(event_t *event, struct perf_session *session)

event__parse_sample(event, session->sample_type, &data);

- dump_printf("(IP, %d): %d/%d: %#Lx period: %Ld\n", event->header.misc,
- data.pid, data.tid, data.ip, data.period);
+ dump_printf("(IP, %d): %d %d/%d: %#Lx period: %Ld\n", event->header.misc,
+ data.cpu, data.pid, data.tid, data.ip, data.period);

if (session->sample_type & PERF_SAMPLE_CALLCHAIN) {
unsigned int i;
@@ -178,7 +178,7 @@ static int process_sample_event(event_t *event, struct perf_session *session)
}
}

- if (event__preprocess_sample(event, session, &al, NULL) < 0) {
+ if (event__preprocess_sample(event, &data, session, &al, NULL) < 0) {
fprintf(stderr, "problem processing %d event, skipping it.\n",
event->header.type);
return -1;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index a66f427..fc9849a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1024,7 +1024,7 @@ static void event__process_sample(const event_t *self,
if (self->header.misc & PERF_RECORD_MISC_EXACT_IP)
exact_samples++;

- if (event__preprocess_sample(self, session, &al, symbol_filter) < 0 ||
+ if (event__preprocess_sample(self, NULL, session, &al, symbol_filter) < 0 ||
al.filtered)
return;

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 50771b5..cd8bf65 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -659,7 +659,8 @@ static void dso__calc_col_width(struct dso *self)
self->slen_calculated = 1;
}

-int event__preprocess_sample(const event_t *self, struct perf_session *session,
+int event__preprocess_sample(const event_t *self, const struct sample_data *data,
+ struct perf_session *session,
struct addr_location *al, symbol_filter_t filter)
{
u8 cpumode = self->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
@@ -672,6 +673,9 @@ int event__preprocess_sample(const event_t *self, struct perf_session *session,
!strlist__has_entry(symbol_conf.comm_list, thread->comm))
goto out_filtered;

+ if (data != NULL)
+ al->cpu = data->cpu;
+
dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
/*
* Have we already created the kernel maps for the host machine?
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 8577085..baedf02 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -156,7 +156,8 @@ int event__process_mmap(event_t *self, struct perf_session *session);
int event__process_task(event_t *self, struct perf_session *session);

struct addr_location;
-int event__preprocess_sample(const event_t *self, struct perf_session *session,
+int event__preprocess_sample(const event_t *self, const struct sample_data *data,
+ struct perf_session *session,
struct addr_location *al, symbol_filter_t filter);
int event__parse_sample(event_t *event, u64 type, struct sample_data *data);

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 2316cb5..4f8e0bf 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -13,6 +13,7 @@ enum sort_type sort__first_dimension;
unsigned int dsos__col_width;
unsigned int comms__col_width;
unsigned int threads__col_width;
+unsigned int cpu__col_width;
static unsigned int parent_symbol__col_width;
char * field_sep;

@@ -28,6 +29,8 @@ static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
size_t size, unsigned int width);
static int hist_entry__parent_snprintf(struct hist_entry *self, char *bf,
size_t size, unsigned int width);
+static int hist_entry__cpu_snprintf(struct hist_entry *self, char *bf,
+ size_t size, unsigned int width);

struct sort_entry sort_thread = {
.se_header = "Command: Pid",
@@ -64,6 +67,13 @@ struct sort_entry sort_parent = {
.se_width = &parent_symbol__col_width,
};

+struct sort_entry sort_cpu = {
+ .se_header = "CPU",
+ .se_cmp = sort__cpu_cmp,
+ .se_snprintf = hist_entry__cpu_snprintf,
+ .se_width = &cpu__col_width,
+};
+
struct sort_dimension {
const char *name;
struct sort_entry *entry;
@@ -76,6 +86,7 @@ static struct sort_dimension sort_dimensions[] = {
{ .name = "dso", .entry = &sort_dso, },
{ .name = "symbol", .entry = &sort_sym, },
{ .name = "parent", .entry = &sort_parent, },
+ { .name = "cpu", .entry = &sort_cpu, },
};

int64_t cmp_null(void *l, void *r)
@@ -242,6 +253,20 @@ static int hist_entry__parent_snprintf(struct hist_entry *self, char *bf,
self->parent ? self->parent->name : "[other]");
}

+/* --sort cpu */
+
+int64_t
+sort__cpu_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+ return right->cpu - left->cpu;
+}
+
+static int hist_entry__cpu_snprintf(struct hist_entry *self, char *bf,
+ size_t size, unsigned int width)
+{
+ return repsep_snprintf(bf, size, "%-*d", width, self->cpu);
+}
+
int sort_dimension__add(const char *tok)
{
unsigned int i;
@@ -281,6 +306,8 @@ int sort_dimension__add(const char *tok)
sort__first_dimension = SORT_SYM;
else if (!strcmp(sd->name, "parent"))
sort__first_dimension = SORT_PARENT;
+ else if (!strcmp(sd->name, "cpu"))
+ sort__first_dimension = SORT_CPU;
}

list_add_tail(&sd->entry->list, &hist_entry__sort_list);
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 0d61c40..dcc6f42 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -55,6 +55,7 @@ struct hist_entry {
char level;
u8 filtered;
struct symbol *parent;
+ s32 cpu;
union {
unsigned long position;
struct hist_entry *pair;
@@ -68,7 +69,8 @@ enum sort_type {
SORT_COMM,
SORT_DSO,
SORT_SYM,
- SORT_PARENT
+ SORT_PARENT,
+ SORT_CPU
};

/*
@@ -97,6 +99,8 @@ extern size_t sort__thread_print(FILE *, struct hist_entry *, unsigned int);
extern size_t sort__comm_print(FILE *, struct hist_entry *, unsigned int);
extern size_t sort__dso_print(FILE *, struct hist_entry *, unsigned int);
extern size_t sort__sym_print(FILE *, struct hist_entry *, unsigned int __used);
+extern size_t sort__parent_print(FILE *, struct hist_entry *, unsigned int);
+extern size_t sort__cpu_print(FILE *, struct hist_entry *, unsigned int);
extern int64_t cmp_null(void *, void *);
extern int64_t sort__thread_cmp(struct hist_entry *, struct hist_entry *);
extern int64_t sort__comm_cmp(struct hist_entry *, struct hist_entry *);
@@ -104,7 +108,7 @@ extern int64_t sort__comm_collapse(struct hist_entry *, struct hist_entry *);
extern int64_t sort__dso_cmp(struct hist_entry *, struct hist_entry *);
extern int64_t sort__sym_cmp(struct hist_entry *, struct hist_entry *);
extern int64_t sort__parent_cmp(struct hist_entry *, struct hist_entry *);
-extern size_t sort__parent_print(FILE *, struct hist_entry *, unsigned int);
+extern int64_t sort__cpu_cmp(struct hist_entry *, struct hist_entry *);
extern int sort_dimension__add(const char *);
void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
const char *list_name, FILE *fp);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 5e02d2c..e9a5848 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -113,6 +113,7 @@ struct addr_location {
char level;
bool filtered;
unsigned int cpumode;
+ s32 cpu;
};

enum dso_kernel_type {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/