2015-04-01 16:26:55

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing


With some feedback from Jolsa, who showed me how to trigger the actual
outputs.

---

Subject: perf, tools: Merge all perf_event_attr print functions
From: Peter Zijlstra <[email protected]>
Date: Tue Mar 31 13:01:54 CEST 2015

Currently there's 3 (that I found) different and incomplete
implementations of printing perf_event_attr.

This is quite silly. Merge the lot.

While this patch does not retain the exact form all printing that I
found is debug output and thus it should not be critical.

Also, I cannot find a single print_event_desc() caller.

Pre:

$ perf record -vv -e cycles -- sleep 1
------------------------------------------------------------
perf_event_attr:
type 0
size 104
config 0
sample_period 4000
sample_freq 4000
sample_type 0x107
read_format 0
disabled 1 inherit 1
pinned 0 exclusive 0
exclude_user 0 exclude_kernel 0
exclude_hv 0 exclude_idle 0
mmap 1 comm 1
mmap2 1 comm_exec 1
freq 1 inherit_stat 0
enable_on_exec 1 task 1
watermark 0 precise_ip 0
mmap_data 0 sample_id_all 1
exclude_host 0 exclude_guest 1
excl.callchain_kern 0 excl.callchain_user 0
wakeup_events 0
wakeup_watermark 0
bp_type 0
bp_addr 0
config1 0
bp_len 0
config2 0
branch_sample_type 0
sample_regs_user 0
sample_stack_user 0
sample_regs_intr 0
------------------------------------------------------------

$ perf evlist -vv
cycles: sample_freq=4000, size: 104, sample_type: IP|TID|TIME|PERIOD,
disabled: 1, inherit: 1, mmap: 1, mmap2: 1, comm: 1, comm_exec: 1,
freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest:
1

Post:

$ ./perf record -vv -e cycles -- sleep 1
------------------------------------------------------------
perf_event_attr:
size 112
{ sample_period, sample_freq } 4000
sample_type IP|TID|TIME|PERIOD
disabled 1
inherit 1
mmap 1
comm 1
freq 1
enable_on_exec 1
task 1
sample_id_all 1
exclude_guest 1
mmap2 1
comm_exec 1
------------------------------------------------------------

$ ./perf evlist -vv
cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type:
IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq:
1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1,
mmap2: 1, comm_exec: 1

Cc: [email protected]
Cc: [email protected]
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
tools/perf/util/Build | 1
tools/perf/util/evsel.c | 181 ++++++-----------------------------------
tools/perf/util/header.c | 34 ++-----
tools/perf/util/print_attr.h | 69 +++++++++++++++
tools/perf/util/print_helper.c | 52 +++++++++++
tools/perf/util/print_helper.h | 7 +
6 files changed, 170 insertions(+), 174 deletions(-)

--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -74,6 +74,7 @@ libperf-y += data.o
libperf-$(CONFIG_X86) += tsc.o
libperf-y += cloexec.o
libperf-y += thread-stack.o
+libperf-y += print_helper.o

libperf-$(CONFIG_LIBELF) += symbol-elf.o
libperf-$(CONFIG_LIBELF) += probe-event.o
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -26,6 +26,7 @@
#include "perf_regs.h"
#include "debug.h"
#include "trace-event.h"
+#include "print_helper.h"

static struct {
bool sample_id_all;
@@ -1011,21 +1012,6 @@ static int get_group_fd(struct perf_evse
return fd;
}

-#define __PRINT_ATTR(fmt, cast, field) \
- fprintf(fp, " %-19s "fmt"\n", #field, cast attr->field)
-
-#define PRINT_ATTR_U32(field) __PRINT_ATTR("%u" , , field)
-#define PRINT_ATTR_X32(field) __PRINT_ATTR("%#x", , field)
-#define PRINT_ATTR_U64(field) __PRINT_ATTR("%" PRIu64, (uint64_t), field)
-#define PRINT_ATTR_X64(field) __PRINT_ATTR("%#"PRIx64, (uint64_t), field)
-
-#define PRINT_ATTR2N(name1, field1, name2, field2) \
- fprintf(fp, " %-19s %u %-19s %u\n", \
- name1, attr->field1, name2, attr->field2)
-
-#define PRINT_ATTR2(field1, field2) \
- PRINT_ATTR2N(#field1, field1, #field2, field2)
-
static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
{
size_t ret = 0;
@@ -1033,42 +1019,18 @@ static size_t perf_event_attr__fprintf(s
ret += fprintf(fp, "%.60s\n", graph_dotted_line);
ret += fprintf(fp, "perf_event_attr:\n");

- ret += PRINT_ATTR_U32(type);
- ret += PRINT_ATTR_U32(size);
- ret += PRINT_ATTR_X64(config);
- ret += PRINT_ATTR_U64(sample_period);
- ret += PRINT_ATTR_U64(sample_freq);
- ret += PRINT_ATTR_X64(sample_type);
- ret += PRINT_ATTR_X64(read_format);
-
- ret += PRINT_ATTR2(disabled, inherit);
- ret += PRINT_ATTR2(pinned, exclusive);
- ret += PRINT_ATTR2(exclude_user, exclude_kernel);
- ret += PRINT_ATTR2(exclude_hv, exclude_idle);
- ret += PRINT_ATTR2(mmap, comm);
- ret += PRINT_ATTR2(freq, inherit_stat);
- ret += PRINT_ATTR2(enable_on_exec, task);
- ret += PRINT_ATTR2(watermark, precise_ip);
- ret += PRINT_ATTR2(mmap_data, sample_id_all);
- ret += PRINT_ATTR2(exclude_host, exclude_guest);
- ret += PRINT_ATTR2N("excl.callchain_kern", exclude_callchain_kernel,
- "excl.callchain_user", exclude_callchain_user);
- ret += PRINT_ATTR2(mmap2, comm_exec);
- ret += __PRINT_ATTR("%u",,use_clockid);
-
-
- ret += PRINT_ATTR_U32(wakeup_events);
- ret += PRINT_ATTR_U32(wakeup_watermark);
- ret += PRINT_ATTR_X32(bp_type);
- ret += PRINT_ATTR_X64(bp_addr);
- ret += PRINT_ATTR_X64(config1);
- ret += PRINT_ATTR_U64(bp_len);
- ret += PRINT_ATTR_X64(config2);
- ret += PRINT_ATTR_X64(branch_sample_type);
- ret += PRINT_ATTR_X64(sample_regs_user);
- ret += PRINT_ATTR_U32(sample_stack_user);
- ret += PRINT_ATTR_U32(clockid);
- ret += PRINT_ATTR_X64(sample_regs_intr);
+#define PRINT_ATTR(_n, _f, _p) \
+do { \
+ if (attr->_f) { \
+ ret += fprintf(fp, " %-32s ", _n); \
+ ret += _p(fp, attr->_f); \
+ ret += fprintf(fp, "\n"); \
+ } \
+} while (0)
+
+#include "util/print_attr.h"
+
+#undef PRINT_ATTR

ret += fprintf(fp, "%.60s\n", graph_dotted_line);

@@ -1996,64 +1958,6 @@ static int comma_fprintf(FILE *fp, bool
return ret;
}

-static int __if_fprintf(FILE *fp, bool *first, const char *field, u64 value)
-{
- if (value == 0)
- return 0;
-
- return comma_fprintf(fp, first, " %s: %" PRIu64, field, value);
-}
-
-#define if_print(field) printed += __if_fprintf(fp, &first, #field, evsel->attr.field)
-
-struct bit_names {
- int bit;
- const char *name;
-};
-
-static int bits__fprintf(FILE *fp, const char *field, u64 value,
- struct bit_names *bits, bool *first)
-{
- int i = 0, printed = comma_fprintf(fp, first, " %s: ", field);
- bool first_bit = true;
-
- do {
- if (value & bits[i].bit) {
- printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
- first_bit = false;
- }
- } while (bits[++i].name != NULL);
-
- return printed;
-}
-
-static int sample_type__fprintf(FILE *fp, bool *first, u64 value)
-{
-#define bit_name(n) { PERF_SAMPLE_##n, #n }
- struct bit_names bits[] = {
- bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
- bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
- bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
- bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
- bit_name(IDENTIFIER), bit_name(REGS_INTR),
- { .name = NULL, }
- };
-#undef bit_name
- return bits__fprintf(fp, "sample_type", value, bits, first);
-}
-
-static int read_format__fprintf(FILE *fp, bool *first, u64 value)
-{
-#define bit_name(n) { PERF_FORMAT_##n, #n }
- struct bit_names bits[] = {
- bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
- bit_name(ID), bit_name(GROUP),
- { .name = NULL, }
- };
-#undef bit_name
- return bits__fprintf(fp, "read_format", value, bits, first);
-}
-
int perf_evsel__fprintf(struct perf_evsel *evsel,
struct perf_attr_details *details, FILE *fp)
{
@@ -2080,51 +1984,24 @@ int perf_evsel__fprintf(struct perf_evse

printed += fprintf(fp, "%s", perf_evsel__name(evsel));

- if (details->verbose || details->freq) {
- printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
- (u64)evsel->attr.sample_freq);
- }

if (details->verbose) {
- if_print(type);
- if_print(config);
- if_print(config1);
- if_print(config2);
- if_print(size);
- printed += sample_type__fprintf(fp, &first, evsel->attr.sample_type);
- if (evsel->attr.read_format)
- printed += read_format__fprintf(fp, &first, evsel->attr.read_format);
- if_print(disabled);
- if_print(inherit);
- if_print(pinned);
- if_print(exclusive);
- if_print(exclude_user);
- if_print(exclude_kernel);
- if_print(exclude_hv);
- if_print(exclude_idle);
- if_print(mmap);
- if_print(comm);
- if_print(freq);
- if_print(inherit_stat);
- if_print(enable_on_exec);
- if_print(task);
- if_print(watermark);
- if_print(precise_ip);
- if_print(mmap_data);
- if_print(sample_id_all);
- if_print(exclude_host);
- if_print(exclude_guest);
- if_print(mmap2);
- if_print(comm_exec);
- if_print(use_clockid);
- if_print(__reserved_1);
- if_print(wakeup_events);
- if_print(bp_type);
- if_print(branch_sample_type);
- if_print(sample_regs_user);
- if_print(sample_stack_user);
- if_print(clockid);
- if_print(sample_regs_intr);
+
+#define PRINT_ATTR(_n, _f, _p) \
+do { \
+ if (evsel->attr._f) { \
+ printed += comma_fprintf(fp, &first, " %s: ", _n); \
+ printed += _p(fp, evsel->attr._f); \
+ } \
+} while (0)
+
+#include "util/print_attr.h"
+
+#undef PRINT_ATTR
+
+ } else if (details->freq) {
+ printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
+ (u64)evsel->attr.sample_freq);
}
out:
fputc('\n', fp);
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -23,6 +23,7 @@
#include "strbuf.h"
#include "build-id.h"
#include "data.h"
+#include "print_helper.h"

static u32 header_argc;
static const char **header_argv;
@@ -1069,26 +1070,6 @@ static void print_event_desc(struct perf
for (evsel = events; evsel->attr.size; evsel++) {
fprintf(fp, "# event : name = %s, ", evsel->name);

- fprintf(fp, "type = %d, config = 0x%"PRIx64
- ", config1 = 0x%"PRIx64", config2 = 0x%"PRIx64,
- evsel->attr.type,
- (u64)evsel->attr.config,
- (u64)evsel->attr.config1,
- (u64)evsel->attr.config2);
-
- fprintf(fp, ", excl_usr = %d, excl_kern = %d",
- evsel->attr.exclude_user,
- evsel->attr.exclude_kernel);
-
- fprintf(fp, ", excl_host = %d, excl_guest = %d",
- evsel->attr.exclude_host,
- evsel->attr.exclude_guest);
-
- fprintf(fp, ", precise_ip = %d", evsel->attr.precise_ip);
-
- fprintf(fp, ", attr_mmap2 = %d", evsel->attr.mmap2);
- fprintf(fp, ", attr_mmap = %d", evsel->attr.mmap);
- fprintf(fp, ", attr_mmap_data = %d", evsel->attr.mmap_data);
if (evsel->ids) {
fprintf(fp, ", id = {");
for (j = 0, id = evsel->id; j < evsel->ids; j++, id++) {
@@ -1098,9 +1079,18 @@ static void print_event_desc(struct perf
}
fprintf(fp, " }");
}
- if (evsel->attr.use_clockid)
- fprintf(fp, ", clockid = %d", evsel->attr.clockid);

+#define PRINT_ATTR(_n, _f, _p) \
+do { \
+ if (evsel->attr._f) { \
+ fprintf(fp, ", %s =", _n); \
+ _p(fp, evsel->attr._f); \
+ } \
+} while (0)
+
+#include "util/print_attr.h"
+
+#undef PRINT_ATTR

fputc('\n', fp);
}
--- /dev/null
+++ b/tools/perf/util/print_attr.h
@@ -0,0 +1,69 @@
+
+#ifndef PRINT_ATTR
+/*
+ * #define PRINT_ATTR(name, field, print) \
+ * do { \
+ * fprintf(fp, " %s = ", name); \
+ * print(fp, attr->field); \
+ * } while (0)
+ */
+#error "General Error and Major Fault yell at you!"
+#endif
+
+#define p_hex(fp, val) fprintf(fp, "%"PRIx64, (uint64_t)(val))
+#define p_unsigned(fp, val) fprintf(fp, "%"PRIu64, (uint64_t)(val))
+#define p_signed(fp, val) fprintf(fp, "%"PRId64, (int64_t)(val))
+#define p_sample_type(fp, val) sample_type__fprintf(fp, val)
+#define p_read_format(fp, val) read_format__fprintf(fp, val)
+
+#define PRINT_ATTRf(field, print) PRINT_ATTR(#field, field, print)
+
+PRINT_ATTRf(type, p_unsigned);
+PRINT_ATTRf(size, p_unsigned);
+PRINT_ATTRf(config, p_hex);
+PRINT_ATTR("{ sample_period, sample_freq }", sample_period, p_unsigned);
+PRINT_ATTRf(sample_type, p_sample_type);
+PRINT_ATTRf(read_format, p_read_format);
+
+PRINT_ATTRf(disabled, p_unsigned);
+PRINT_ATTRf(inherit, p_unsigned);
+PRINT_ATTRf(pinned, p_unsigned);
+PRINT_ATTRf(exclusive, p_unsigned);
+PRINT_ATTRf(exclude_user, p_unsigned);
+PRINT_ATTRf(exclude_kernel, p_unsigned);
+PRINT_ATTRf(exclude_hv, p_unsigned);
+PRINT_ATTRf(exclude_idle, p_unsigned);
+PRINT_ATTRf(mmap, p_unsigned);
+PRINT_ATTRf(comm, p_unsigned);
+PRINT_ATTRf(freq, p_unsigned);
+PRINT_ATTRf(inherit_stat, p_unsigned);
+PRINT_ATTRf(enable_on_exec, p_unsigned);
+PRINT_ATTRf(task, p_unsigned);
+PRINT_ATTRf(watermark, p_unsigned);
+PRINT_ATTRf(precise_ip, p_unsigned);
+PRINT_ATTRf(mmap_data, p_unsigned);
+PRINT_ATTRf(sample_id_all, p_unsigned);
+PRINT_ATTRf(exclude_host, p_unsigned);
+PRINT_ATTRf(exclude_guest, p_unsigned);
+PRINT_ATTRf(exclude_callchain_kernel, p_unsigned);
+PRINT_ATTRf(exclude_callchain_user, p_unsigned);
+PRINT_ATTRf(mmap2, p_unsigned);
+PRINT_ATTRf(comm_exec, p_unsigned);
+PRINT_ATTRf(use_clockid, p_unsigned);
+
+PRINT_ATTR("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
+PRINT_ATTRf(bp_type, p_unsigned);
+PRINT_ATTR("{ bp_addr, config1 }", bp_addr, p_hex);
+PRINT_ATTR("{ bp_len, config2 }", bp_len, p_hex);
+PRINT_ATTRf(sample_regs_user, p_hex);
+PRINT_ATTRf(sample_stack_user, p_unsigned);
+PRINT_ATTRf(clockid, p_signed);
+PRINT_ATTRf(sample_regs_intr, p_hex);
+
+#undef PRINT_ATTRf
+
+#undef p_hex
+#undef p_unsigned
+#undef p_signed
+#undef p_sample_type
+#undef p_read_format
--- /dev/null
+++ b/tools/perf/util/print_helper.c
@@ -0,0 +1,52 @@
+
+#include <linux/perf_event.h>
+#include "util.h"
+#include "print_helper.h"
+
+struct bit_names {
+ int bit;
+ const char *name;
+};
+
+static int bits__fprintf(FILE *fp, u64 value, struct bit_names *bits)
+{
+ int i = 0, printed = 0;
+ bool first_bit = true;
+
+ do {
+ if (value & bits[i].bit) {
+ printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
+ first_bit = false;
+ }
+ } while (bits[++i].name != NULL);
+
+ return printed;
+}
+
+int sample_type__fprintf(FILE *fp, u64 value)
+{
+#define bit_name(n) { PERF_SAMPLE_##n, #n }
+ struct bit_names bits[] = {
+ bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
+ bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
+ bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
+ bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
+ bit_name(IDENTIFIER), bit_name(REGS_INTR),
+ { .name = NULL, }
+ };
+#undef bit_name
+ return bits__fprintf(fp, value, bits);
+}
+
+int read_format__fprintf(FILE *fp, u64 value)
+{
+#define bit_name(n) { PERF_FORMAT_##n, #n }
+ struct bit_names bits[] = {
+ bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
+ bit_name(ID), bit_name(GROUP),
+ { .name = NULL, }
+ };
+#undef bit_name
+ return bits__fprintf(fp, value, bits);
+}
+
--- /dev/null
+++ b/tools/perf/util/print_helper.h
@@ -0,0 +1,7 @@
+#ifndef PRINT_HELPER_H
+#define PRINT_HELPER_H
+
+extern int sample_type__fprintf(FILE *fp, u64 value);
+extern int read_format__fprintf(FILE *fp, u64 value);
+
+#endif


2015-04-01 16:52:58

by Jiri Olsa

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing

On Wed, Apr 01, 2015 at 06:26:38PM +0200, Peter Zijlstra wrote:
>
> With some feedback from Jolsa, who showed me how to trigger the actual
> outputs.

adding Adrian to CC as he's the original author AFAIK

jirka

>
> ---
>
> Subject: perf, tools: Merge all perf_event_attr print functions
> From: Peter Zijlstra <[email protected]>
> Date: Tue Mar 31 13:01:54 CEST 2015
>
> Currently there's 3 (that I found) different and incomplete
> implementations of printing perf_event_attr.
>
> This is quite silly. Merge the lot.
>
> While this patch does not retain the exact form all printing that I
> found is debug output and thus it should not be critical.
>
> Also, I cannot find a single print_event_desc() caller.
>
> Pre:
>
> $ perf record -vv -e cycles -- sleep 1
> ------------------------------------------------------------
> perf_event_attr:
> type 0
> size 104
> config 0
> sample_period 4000
> sample_freq 4000
> sample_type 0x107
> read_format 0
> disabled 1 inherit 1
> pinned 0 exclusive 0
> exclude_user 0 exclude_kernel 0
> exclude_hv 0 exclude_idle 0
> mmap 1 comm 1
> mmap2 1 comm_exec 1
> freq 1 inherit_stat 0
> enable_on_exec 1 task 1
> watermark 0 precise_ip 0
> mmap_data 0 sample_id_all 1
> exclude_host 0 exclude_guest 1
> excl.callchain_kern 0 excl.callchain_user 0
> wakeup_events 0
> wakeup_watermark 0
> bp_type 0
> bp_addr 0
> config1 0
> bp_len 0
> config2 0
> branch_sample_type 0
> sample_regs_user 0
> sample_stack_user 0
> sample_regs_intr 0
> ------------------------------------------------------------
>
> $ perf evlist -vv
> cycles: sample_freq=4000, size: 104, sample_type: IP|TID|TIME|PERIOD,
> disabled: 1, inherit: 1, mmap: 1, mmap2: 1, comm: 1, comm_exec: 1,
> freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest:
> 1
>
> Post:
>
> $ ./perf record -vv -e cycles -- sleep 1
> ------------------------------------------------------------
> perf_event_attr:
> size 112
> { sample_period, sample_freq } 4000
> sample_type IP|TID|TIME|PERIOD
> disabled 1
> inherit 1
> mmap 1
> comm 1
> freq 1
> enable_on_exec 1
> task 1
> sample_id_all 1
> exclude_guest 1
> mmap2 1
> comm_exec 1
> ------------------------------------------------------------
>
> $ ./perf evlist -vv
> cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type:
> IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq:
> 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1,
> mmap2: 1, comm_exec: 1
>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> tools/perf/util/Build | 1
> tools/perf/util/evsel.c | 181 ++++++-----------------------------------
> tools/perf/util/header.c | 34 ++-----
> tools/perf/util/print_attr.h | 69 +++++++++++++++
> tools/perf/util/print_helper.c | 52 +++++++++++
> tools/perf/util/print_helper.h | 7 +
> 6 files changed, 170 insertions(+), 174 deletions(-)
>
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -74,6 +74,7 @@ libperf-y += data.o
> libperf-$(CONFIG_X86) += tsc.o
> libperf-y += cloexec.o
> libperf-y += thread-stack.o
> +libperf-y += print_helper.o
>
> libperf-$(CONFIG_LIBELF) += symbol-elf.o
> libperf-$(CONFIG_LIBELF) += probe-event.o
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -26,6 +26,7 @@
> #include "perf_regs.h"
> #include "debug.h"
> #include "trace-event.h"
> +#include "print_helper.h"
>
> static struct {
> bool sample_id_all;
> @@ -1011,21 +1012,6 @@ static int get_group_fd(struct perf_evse
> return fd;
> }
>
> -#define __PRINT_ATTR(fmt, cast, field) \
> - fprintf(fp, " %-19s "fmt"\n", #field, cast attr->field)
> -
> -#define PRINT_ATTR_U32(field) __PRINT_ATTR("%u" , , field)
> -#define PRINT_ATTR_X32(field) __PRINT_ATTR("%#x", , field)
> -#define PRINT_ATTR_U64(field) __PRINT_ATTR("%" PRIu64, (uint64_t), field)
> -#define PRINT_ATTR_X64(field) __PRINT_ATTR("%#"PRIx64, (uint64_t), field)
> -
> -#define PRINT_ATTR2N(name1, field1, name2, field2) \
> - fprintf(fp, " %-19s %u %-19s %u\n", \
> - name1, attr->field1, name2, attr->field2)
> -
> -#define PRINT_ATTR2(field1, field2) \
> - PRINT_ATTR2N(#field1, field1, #field2, field2)
> -
> static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
> {
> size_t ret = 0;
> @@ -1033,42 +1019,18 @@ static size_t perf_event_attr__fprintf(s
> ret += fprintf(fp, "%.60s\n", graph_dotted_line);
> ret += fprintf(fp, "perf_event_attr:\n");
>
> - ret += PRINT_ATTR_U32(type);
> - ret += PRINT_ATTR_U32(size);
> - ret += PRINT_ATTR_X64(config);
> - ret += PRINT_ATTR_U64(sample_period);
> - ret += PRINT_ATTR_U64(sample_freq);
> - ret += PRINT_ATTR_X64(sample_type);
> - ret += PRINT_ATTR_X64(read_format);
> -
> - ret += PRINT_ATTR2(disabled, inherit);
> - ret += PRINT_ATTR2(pinned, exclusive);
> - ret += PRINT_ATTR2(exclude_user, exclude_kernel);
> - ret += PRINT_ATTR2(exclude_hv, exclude_idle);
> - ret += PRINT_ATTR2(mmap, comm);
> - ret += PRINT_ATTR2(freq, inherit_stat);
> - ret += PRINT_ATTR2(enable_on_exec, task);
> - ret += PRINT_ATTR2(watermark, precise_ip);
> - ret += PRINT_ATTR2(mmap_data, sample_id_all);
> - ret += PRINT_ATTR2(exclude_host, exclude_guest);
> - ret += PRINT_ATTR2N("excl.callchain_kern", exclude_callchain_kernel,
> - "excl.callchain_user", exclude_callchain_user);
> - ret += PRINT_ATTR2(mmap2, comm_exec);
> - ret += __PRINT_ATTR("%u",,use_clockid);
> -
> -
> - ret += PRINT_ATTR_U32(wakeup_events);
> - ret += PRINT_ATTR_U32(wakeup_watermark);
> - ret += PRINT_ATTR_X32(bp_type);
> - ret += PRINT_ATTR_X64(bp_addr);
> - ret += PRINT_ATTR_X64(config1);
> - ret += PRINT_ATTR_U64(bp_len);
> - ret += PRINT_ATTR_X64(config2);
> - ret += PRINT_ATTR_X64(branch_sample_type);
> - ret += PRINT_ATTR_X64(sample_regs_user);
> - ret += PRINT_ATTR_U32(sample_stack_user);
> - ret += PRINT_ATTR_U32(clockid);
> - ret += PRINT_ATTR_X64(sample_regs_intr);
> +#define PRINT_ATTR(_n, _f, _p) \
> +do { \
> + if (attr->_f) { \
> + ret += fprintf(fp, " %-32s ", _n); \
> + ret += _p(fp, attr->_f); \
> + ret += fprintf(fp, "\n"); \
> + } \
> +} while (0)
> +
> +#include "util/print_attr.h"
> +
> +#undef PRINT_ATTR
>
> ret += fprintf(fp, "%.60s\n", graph_dotted_line);
>
> @@ -1996,64 +1958,6 @@ static int comma_fprintf(FILE *fp, bool
> return ret;
> }
>
> -static int __if_fprintf(FILE *fp, bool *first, const char *field, u64 value)
> -{
> - if (value == 0)
> - return 0;
> -
> - return comma_fprintf(fp, first, " %s: %" PRIu64, field, value);
> -}
> -
> -#define if_print(field) printed += __if_fprintf(fp, &first, #field, evsel->attr.field)
> -
> -struct bit_names {
> - int bit;
> - const char *name;
> -};
> -
> -static int bits__fprintf(FILE *fp, const char *field, u64 value,
> - struct bit_names *bits, bool *first)
> -{
> - int i = 0, printed = comma_fprintf(fp, first, " %s: ", field);
> - bool first_bit = true;
> -
> - do {
> - if (value & bits[i].bit) {
> - printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
> - first_bit = false;
> - }
> - } while (bits[++i].name != NULL);
> -
> - return printed;
> -}
> -
> -static int sample_type__fprintf(FILE *fp, bool *first, u64 value)
> -{
> -#define bit_name(n) { PERF_SAMPLE_##n, #n }
> - struct bit_names bits[] = {
> - bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
> - bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
> - bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
> - bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
> - bit_name(IDENTIFIER), bit_name(REGS_INTR),
> - { .name = NULL, }
> - };
> -#undef bit_name
> - return bits__fprintf(fp, "sample_type", value, bits, first);
> -}
> -
> -static int read_format__fprintf(FILE *fp, bool *first, u64 value)
> -{
> -#define bit_name(n) { PERF_FORMAT_##n, #n }
> - struct bit_names bits[] = {
> - bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
> - bit_name(ID), bit_name(GROUP),
> - { .name = NULL, }
> - };
> -#undef bit_name
> - return bits__fprintf(fp, "read_format", value, bits, first);
> -}
> -
> int perf_evsel__fprintf(struct perf_evsel *evsel,
> struct perf_attr_details *details, FILE *fp)
> {
> @@ -2080,51 +1984,24 @@ int perf_evsel__fprintf(struct perf_evse
>
> printed += fprintf(fp, "%s", perf_evsel__name(evsel));
>
> - if (details->verbose || details->freq) {
> - printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
> - (u64)evsel->attr.sample_freq);
> - }
>
> if (details->verbose) {
> - if_print(type);
> - if_print(config);
> - if_print(config1);
> - if_print(config2);
> - if_print(size);
> - printed += sample_type__fprintf(fp, &first, evsel->attr.sample_type);
> - if (evsel->attr.read_format)
> - printed += read_format__fprintf(fp, &first, evsel->attr.read_format);
> - if_print(disabled);
> - if_print(inherit);
> - if_print(pinned);
> - if_print(exclusive);
> - if_print(exclude_user);
> - if_print(exclude_kernel);
> - if_print(exclude_hv);
> - if_print(exclude_idle);
> - if_print(mmap);
> - if_print(comm);
> - if_print(freq);
> - if_print(inherit_stat);
> - if_print(enable_on_exec);
> - if_print(task);
> - if_print(watermark);
> - if_print(precise_ip);
> - if_print(mmap_data);
> - if_print(sample_id_all);
> - if_print(exclude_host);
> - if_print(exclude_guest);
> - if_print(mmap2);
> - if_print(comm_exec);
> - if_print(use_clockid);
> - if_print(__reserved_1);
> - if_print(wakeup_events);
> - if_print(bp_type);
> - if_print(branch_sample_type);
> - if_print(sample_regs_user);
> - if_print(sample_stack_user);
> - if_print(clockid);
> - if_print(sample_regs_intr);
> +
> +#define PRINT_ATTR(_n, _f, _p) \
> +do { \
> + if (evsel->attr._f) { \
> + printed += comma_fprintf(fp, &first, " %s: ", _n); \
> + printed += _p(fp, evsel->attr._f); \
> + } \
> +} while (0)
> +
> +#include "util/print_attr.h"
> +
> +#undef PRINT_ATTR
> +
> + } else if (details->freq) {
> + printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
> + (u64)evsel->attr.sample_freq);
> }
> out:
> fputc('\n', fp);
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -23,6 +23,7 @@
> #include "strbuf.h"
> #include "build-id.h"
> #include "data.h"
> +#include "print_helper.h"
>
> static u32 header_argc;
> static const char **header_argv;
> @@ -1069,26 +1070,6 @@ static void print_event_desc(struct perf
> for (evsel = events; evsel->attr.size; evsel++) {
> fprintf(fp, "# event : name = %s, ", evsel->name);
>
> - fprintf(fp, "type = %d, config = 0x%"PRIx64
> - ", config1 = 0x%"PRIx64", config2 = 0x%"PRIx64,
> - evsel->attr.type,
> - (u64)evsel->attr.config,
> - (u64)evsel->attr.config1,
> - (u64)evsel->attr.config2);
> -
> - fprintf(fp, ", excl_usr = %d, excl_kern = %d",
> - evsel->attr.exclude_user,
> - evsel->attr.exclude_kernel);
> -
> - fprintf(fp, ", excl_host = %d, excl_guest = %d",
> - evsel->attr.exclude_host,
> - evsel->attr.exclude_guest);
> -
> - fprintf(fp, ", precise_ip = %d", evsel->attr.precise_ip);
> -
> - fprintf(fp, ", attr_mmap2 = %d", evsel->attr.mmap2);
> - fprintf(fp, ", attr_mmap = %d", evsel->attr.mmap);
> - fprintf(fp, ", attr_mmap_data = %d", evsel->attr.mmap_data);
> if (evsel->ids) {
> fprintf(fp, ", id = {");
> for (j = 0, id = evsel->id; j < evsel->ids; j++, id++) {
> @@ -1098,9 +1079,18 @@ static void print_event_desc(struct perf
> }
> fprintf(fp, " }");
> }
> - if (evsel->attr.use_clockid)
> - fprintf(fp, ", clockid = %d", evsel->attr.clockid);
>
> +#define PRINT_ATTR(_n, _f, _p) \
> +do { \
> + if (evsel->attr._f) { \
> + fprintf(fp, ", %s =", _n); \
> + _p(fp, evsel->attr._f); \
> + } \
> +} while (0)
> +
> +#include "util/print_attr.h"
> +
> +#undef PRINT_ATTR
>
> fputc('\n', fp);
> }
> --- /dev/null
> +++ b/tools/perf/util/print_attr.h
> @@ -0,0 +1,69 @@
> +
> +#ifndef PRINT_ATTR
> +/*
> + * #define PRINT_ATTR(name, field, print) \
> + * do { \
> + * fprintf(fp, " %s = ", name); \
> + * print(fp, attr->field); \
> + * } while (0)
> + */
> +#error "General Error and Major Fault yell at you!"
> +#endif
> +
> +#define p_hex(fp, val) fprintf(fp, "%"PRIx64, (uint64_t)(val))
> +#define p_unsigned(fp, val) fprintf(fp, "%"PRIu64, (uint64_t)(val))
> +#define p_signed(fp, val) fprintf(fp, "%"PRId64, (int64_t)(val))
> +#define p_sample_type(fp, val) sample_type__fprintf(fp, val)
> +#define p_read_format(fp, val) read_format__fprintf(fp, val)
> +
> +#define PRINT_ATTRf(field, print) PRINT_ATTR(#field, field, print)
> +
> +PRINT_ATTRf(type, p_unsigned);
> +PRINT_ATTRf(size, p_unsigned);
> +PRINT_ATTRf(config, p_hex);
> +PRINT_ATTR("{ sample_period, sample_freq }", sample_period, p_unsigned);
> +PRINT_ATTRf(sample_type, p_sample_type);
> +PRINT_ATTRf(read_format, p_read_format);
> +
> +PRINT_ATTRf(disabled, p_unsigned);
> +PRINT_ATTRf(inherit, p_unsigned);
> +PRINT_ATTRf(pinned, p_unsigned);
> +PRINT_ATTRf(exclusive, p_unsigned);
> +PRINT_ATTRf(exclude_user, p_unsigned);
> +PRINT_ATTRf(exclude_kernel, p_unsigned);
> +PRINT_ATTRf(exclude_hv, p_unsigned);
> +PRINT_ATTRf(exclude_idle, p_unsigned);
> +PRINT_ATTRf(mmap, p_unsigned);
> +PRINT_ATTRf(comm, p_unsigned);
> +PRINT_ATTRf(freq, p_unsigned);
> +PRINT_ATTRf(inherit_stat, p_unsigned);
> +PRINT_ATTRf(enable_on_exec, p_unsigned);
> +PRINT_ATTRf(task, p_unsigned);
> +PRINT_ATTRf(watermark, p_unsigned);
> +PRINT_ATTRf(precise_ip, p_unsigned);
> +PRINT_ATTRf(mmap_data, p_unsigned);
> +PRINT_ATTRf(sample_id_all, p_unsigned);
> +PRINT_ATTRf(exclude_host, p_unsigned);
> +PRINT_ATTRf(exclude_guest, p_unsigned);
> +PRINT_ATTRf(exclude_callchain_kernel, p_unsigned);
> +PRINT_ATTRf(exclude_callchain_user, p_unsigned);
> +PRINT_ATTRf(mmap2, p_unsigned);
> +PRINT_ATTRf(comm_exec, p_unsigned);
> +PRINT_ATTRf(use_clockid, p_unsigned);
> +
> +PRINT_ATTR("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
> +PRINT_ATTRf(bp_type, p_unsigned);
> +PRINT_ATTR("{ bp_addr, config1 }", bp_addr, p_hex);
> +PRINT_ATTR("{ bp_len, config2 }", bp_len, p_hex);
> +PRINT_ATTRf(sample_regs_user, p_hex);
> +PRINT_ATTRf(sample_stack_user, p_unsigned);
> +PRINT_ATTRf(clockid, p_signed);
> +PRINT_ATTRf(sample_regs_intr, p_hex);
> +
> +#undef PRINT_ATTRf
> +
> +#undef p_hex
> +#undef p_unsigned
> +#undef p_signed
> +#undef p_sample_type
> +#undef p_read_format
> --- /dev/null
> +++ b/tools/perf/util/print_helper.c
> @@ -0,0 +1,52 @@
> +
> +#include <linux/perf_event.h>
> +#include "util.h"
> +#include "print_helper.h"
> +
> +struct bit_names {
> + int bit;
> + const char *name;
> +};
> +
> +static int bits__fprintf(FILE *fp, u64 value, struct bit_names *bits)
> +{
> + int i = 0, printed = 0;
> + bool first_bit = true;
> +
> + do {
> + if (value & bits[i].bit) {
> + printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
> + first_bit = false;
> + }
> + } while (bits[++i].name != NULL);
> +
> + return printed;
> +}
> +
> +int sample_type__fprintf(FILE *fp, u64 value)
> +{
> +#define bit_name(n) { PERF_SAMPLE_##n, #n }
> + struct bit_names bits[] = {
> + bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
> + bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
> + bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
> + bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
> + bit_name(IDENTIFIER), bit_name(REGS_INTR),
> + { .name = NULL, }
> + };
> +#undef bit_name
> + return bits__fprintf(fp, value, bits);
> +}
> +
> +int read_format__fprintf(FILE *fp, u64 value)
> +{
> +#define bit_name(n) { PERF_FORMAT_##n, #n }
> + struct bit_names bits[] = {
> + bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
> + bit_name(ID), bit_name(GROUP),
> + { .name = NULL, }
> + };
> +#undef bit_name
> + return bits__fprintf(fp, value, bits);
> +}
> +
> --- /dev/null
> +++ b/tools/perf/util/print_helper.h
> @@ -0,0 +1,7 @@
> +#ifndef PRINT_HELPER_H
> +#define PRINT_HELPER_H
> +
> +extern int sample_type__fprintf(FILE *fp, u64 value);
> +extern int read_format__fprintf(FILE *fp, u64 value);
> +
> +#endif

2015-04-02 08:14:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing


* Peter Zijlstra <[email protected]> wrote:

> With some feedback from Jolsa, who showed me how to trigger the actual
> outputs.
>
> ---
>
> Subject: perf, tools: Merge all perf_event_attr print functions
> From: Peter Zijlstra <[email protected]>
> Date: Tue Mar 31 13:01:54 CEST 2015
>
> Currently there's 3 (that I found) different and incomplete
> implementations of printing perf_event_attr.
>
> This is quite silly. Merge the lot.
>
> While this patch does not retain the exact form all printing that I
> found is debug output and thus it should not be critical.
>
> Also, I cannot find a single print_event_desc() caller.
>
> Pre:
>
> $ perf record -vv -e cycles -- sleep 1
> ------------------------------------------------------------
> perf_event_attr:
> type 0
> size 104
> config 0
> sample_period 4000
> sample_freq 4000
> sample_type 0x107
> read_format 0
> disabled 1 inherit 1
> pinned 0 exclusive 0
> exclude_user 0 exclude_kernel 0
> exclude_hv 0 exclude_idle 0
> mmap 1 comm 1
> mmap2 1 comm_exec 1
> freq 1 inherit_stat 0
> enable_on_exec 1 task 1
> watermark 0 precise_ip 0
> mmap_data 0 sample_id_all 1
> exclude_host 0 exclude_guest 1
> excl.callchain_kern 0 excl.callchain_user 0
> wakeup_events 0
> wakeup_watermark 0
> bp_type 0
> bp_addr 0
> config1 0
> bp_len 0
> config2 0
> branch_sample_type 0
> sample_regs_user 0
> sample_stack_user 0
> sample_regs_intr 0
> ------------------------------------------------------------
>
> $ perf evlist -vv
> cycles: sample_freq=4000, size: 104, sample_type: IP|TID|TIME|PERIOD,
> disabled: 1, inherit: 1, mmap: 1, mmap2: 1, comm: 1, comm_exec: 1,
> freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest:
> 1
>
> Post:
>
> $ ./perf record -vv -e cycles -- sleep 1
> ------------------------------------------------------------
> perf_event_attr:
> size 112
> { sample_period, sample_freq } 4000
> sample_type IP|TID|TIME|PERIOD
> disabled 1
> inherit 1
> mmap 1
> comm 1
> freq 1
> enable_on_exec 1
> task 1
> sample_id_all 1
> exclude_guest 1
> mmap2 1
> comm_exec 1
> ------------------------------------------------------------
>
> $ ./perf evlist -vv
> cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type:
> IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq:
> 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1,
> mmap2: 1, comm_exec: 1
>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> tools/perf/util/Build | 1
> tools/perf/util/evsel.c | 181 ++++++-----------------------------------
> tools/perf/util/header.c | 34 ++-----
> tools/perf/util/print_attr.h | 69 +++++++++++++++
> tools/perf/util/print_helper.c | 52 +++++++++++
> tools/perf/util/print_helper.h | 7 +
> 6 files changed, 170 insertions(+), 174 deletions(-)

Acked-by: Ingo Molnar <[email protected]>

Thanks,

Ingo

2015-04-02 09:04:06

by Adrian Hunter

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing

On 01/04/15 19:52, Jiri Olsa wrote:
> On Wed, Apr 01, 2015 at 06:26:38PM +0200, Peter Zijlstra wrote:
>>
>> With some feedback from Jolsa, who showed me how to trigger the actual
>> outputs.
>
> adding Adrian to CC as he's the original author AFAIK

I wanted a compact format, and the omission of null/zero values achieves that.

But personally I think the "include" approach is too ugly. I would just
add a function instead. Something like:

#define p_unsigned(val) snprintf(value_buf, BUF_SIZE, "%"PRIu64, (uint64_t)(val))

#define PRINT_ATTRN(name, field, print) \
if (!skip_zero || attr->field) { \
snprintf(name_buf, BUF_SIZE, name); \
print(attr->field); \
prt(name_buf, value_buf, priv); \
}

#define PRINT_ATTR(field, print) PRINT_ATTRN(#field, field, print)

typedef int (*attr_print_cb_t)(const char *name, const char *value, void *priv);

int perf_event_attr__print(struct perf_event_attr *attr, attr_print_cb_t prt, bool skip_zero, void *priv)
{
char name_buf[BUF_SIZE];
char value_buf[BUF_SIZE];
int err;

PRINT_ATTR(type, p_unsigned);

etc

return 0;
}


Then it calling it looks like:


static int perf_event_attr__fprintf_one(const char *name, const char *value, void *priv)
{
size_t *ret = priv;

*ret += fprintf(fp, " %-32s %s\n", name, value);
return 0;
}

static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
{
size_t ret = 0;

ret += fprintf(fp, "%.60s\n", graph_dotted_line);
ret += fprintf(fp, "perf_event_attr:\n");

perf_event_attr__print(attr, perf_event_attr__fprintf_one, true, &ret);

ret += fprintf(fp, "%.60s\n", graph_dotted_line);

return ret;
}


>
> jirka
>
>>
>> ---
>>
>> Subject: perf, tools: Merge all perf_event_attr print functions
>> From: Peter Zijlstra <[email protected]>
>> Date: Tue Mar 31 13:01:54 CEST 2015
>>
>> Currently there's 3 (that I found) different and incomplete
>> implementations of printing perf_event_attr.
>>
>> This is quite silly. Merge the lot.
>>
>> While this patch does not retain the exact form all printing that I
>> found is debug output and thus it should not be critical.
>>
>> Also, I cannot find a single print_event_desc() caller.
>>
>> Pre:
>>
>> $ perf record -vv -e cycles -- sleep 1
>> ------------------------------------------------------------
>> perf_event_attr:
>> type 0
>> size 104
>> config 0
>> sample_period 4000
>> sample_freq 4000
>> sample_type 0x107
>> read_format 0
>> disabled 1 inherit 1
>> pinned 0 exclusive 0
>> exclude_user 0 exclude_kernel 0
>> exclude_hv 0 exclude_idle 0
>> mmap 1 comm 1
>> mmap2 1 comm_exec 1
>> freq 1 inherit_stat 0
>> enable_on_exec 1 task 1
>> watermark 0 precise_ip 0
>> mmap_data 0 sample_id_all 1
>> exclude_host 0 exclude_guest 1
>> excl.callchain_kern 0 excl.callchain_user 0
>> wakeup_events 0
>> wakeup_watermark 0
>> bp_type 0
>> bp_addr 0
>> config1 0
>> bp_len 0
>> config2 0
>> branch_sample_type 0
>> sample_regs_user 0
>> sample_stack_user 0
>> sample_regs_intr 0
>> ------------------------------------------------------------
>>
>> $ perf evlist -vv
>> cycles: sample_freq=4000, size: 104, sample_type: IP|TID|TIME|PERIOD,
>> disabled: 1, inherit: 1, mmap: 1, mmap2: 1, comm: 1, comm_exec: 1,
>> freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest:
>> 1
>>
>> Post:
>>
>> $ ./perf record -vv -e cycles -- sleep 1
>> ------------------------------------------------------------
>> perf_event_attr:
>> size 112
>> { sample_period, sample_freq } 4000
>> sample_type IP|TID|TIME|PERIOD
>> disabled 1
>> inherit 1
>> mmap 1
>> comm 1
>> freq 1
>> enable_on_exec 1
>> task 1
>> sample_id_all 1
>> exclude_guest 1
>> mmap2 1
>> comm_exec 1
>> ------------------------------------------------------------
>>
>> $ ./perf evlist -vv
>> cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type:
>> IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq:
>> 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1,
>> mmap2: 1, comm_exec: 1
>>
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
>> ---
>> tools/perf/util/Build | 1
>> tools/perf/util/evsel.c | 181 ++++++-----------------------------------
>> tools/perf/util/header.c | 34 ++-----
>> tools/perf/util/print_attr.h | 69 +++++++++++++++
>> tools/perf/util/print_helper.c | 52 +++++++++++
>> tools/perf/util/print_helper.h | 7 +
>> 6 files changed, 170 insertions(+), 174 deletions(-)
>>
>> --- a/tools/perf/util/Build
>> +++ b/tools/perf/util/Build
>> @@ -74,6 +74,7 @@ libperf-y += data.o
>> libperf-$(CONFIG_X86) += tsc.o
>> libperf-y += cloexec.o
>> libperf-y += thread-stack.o
>> +libperf-y += print_helper.o
>>
>> libperf-$(CONFIG_LIBELF) += symbol-elf.o
>> libperf-$(CONFIG_LIBELF) += probe-event.o
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -26,6 +26,7 @@
>> #include "perf_regs.h"
>> #include "debug.h"
>> #include "trace-event.h"
>> +#include "print_helper.h"
>>
>> static struct {
>> bool sample_id_all;
>> @@ -1011,21 +1012,6 @@ static int get_group_fd(struct perf_evse
>> return fd;
>> }
>>
>> -#define __PRINT_ATTR(fmt, cast, field) \
>> - fprintf(fp, " %-19s "fmt"\n", #field, cast attr->field)
>> -
>> -#define PRINT_ATTR_U32(field) __PRINT_ATTR("%u" , , field)
>> -#define PRINT_ATTR_X32(field) __PRINT_ATTR("%#x", , field)
>> -#define PRINT_ATTR_U64(field) __PRINT_ATTR("%" PRIu64, (uint64_t), field)
>> -#define PRINT_ATTR_X64(field) __PRINT_ATTR("%#"PRIx64, (uint64_t), field)
>> -
>> -#define PRINT_ATTR2N(name1, field1, name2, field2) \
>> - fprintf(fp, " %-19s %u %-19s %u\n", \
>> - name1, attr->field1, name2, attr->field2)
>> -
>> -#define PRINT_ATTR2(field1, field2) \
>> - PRINT_ATTR2N(#field1, field1, #field2, field2)
>> -
>> static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
>> {
>> size_t ret = 0;
>> @@ -1033,42 +1019,18 @@ static size_t perf_event_attr__fprintf(s
>> ret += fprintf(fp, "%.60s\n", graph_dotted_line);
>> ret += fprintf(fp, "perf_event_attr:\n");
>>
>> - ret += PRINT_ATTR_U32(type);
>> - ret += PRINT_ATTR_U32(size);
>> - ret += PRINT_ATTR_X64(config);
>> - ret += PRINT_ATTR_U64(sample_period);
>> - ret += PRINT_ATTR_U64(sample_freq);
>> - ret += PRINT_ATTR_X64(sample_type);
>> - ret += PRINT_ATTR_X64(read_format);
>> -
>> - ret += PRINT_ATTR2(disabled, inherit);
>> - ret += PRINT_ATTR2(pinned, exclusive);
>> - ret += PRINT_ATTR2(exclude_user, exclude_kernel);
>> - ret += PRINT_ATTR2(exclude_hv, exclude_idle);
>> - ret += PRINT_ATTR2(mmap, comm);
>> - ret += PRINT_ATTR2(freq, inherit_stat);
>> - ret += PRINT_ATTR2(enable_on_exec, task);
>> - ret += PRINT_ATTR2(watermark, precise_ip);
>> - ret += PRINT_ATTR2(mmap_data, sample_id_all);
>> - ret += PRINT_ATTR2(exclude_host, exclude_guest);
>> - ret += PRINT_ATTR2N("excl.callchain_kern", exclude_callchain_kernel,
>> - "excl.callchain_user", exclude_callchain_user);
>> - ret += PRINT_ATTR2(mmap2, comm_exec);
>> - ret += __PRINT_ATTR("%u",,use_clockid);
>> -
>> -
>> - ret += PRINT_ATTR_U32(wakeup_events);
>> - ret += PRINT_ATTR_U32(wakeup_watermark);
>> - ret += PRINT_ATTR_X32(bp_type);
>> - ret += PRINT_ATTR_X64(bp_addr);
>> - ret += PRINT_ATTR_X64(config1);
>> - ret += PRINT_ATTR_U64(bp_len);
>> - ret += PRINT_ATTR_X64(config2);
>> - ret += PRINT_ATTR_X64(branch_sample_type);
>> - ret += PRINT_ATTR_X64(sample_regs_user);
>> - ret += PRINT_ATTR_U32(sample_stack_user);
>> - ret += PRINT_ATTR_U32(clockid);
>> - ret += PRINT_ATTR_X64(sample_regs_intr);
>> +#define PRINT_ATTR(_n, _f, _p) \
>> +do { \
>> + if (attr->_f) { \
>> + ret += fprintf(fp, " %-32s ", _n); \
>> + ret += _p(fp, attr->_f); \
>> + ret += fprintf(fp, "\n"); \
>> + } \
>> +} while (0)
>> +
>> +#include "util/print_attr.h"
>> +
>> +#undef PRINT_ATTR
>>
>> ret += fprintf(fp, "%.60s\n", graph_dotted_line);
>>
>> @@ -1996,64 +1958,6 @@ static int comma_fprintf(FILE *fp, bool
>> return ret;
>> }
>>
>> -static int __if_fprintf(FILE *fp, bool *first, const char *field, u64 value)
>> -{
>> - if (value == 0)
>> - return 0;
>> -
>> - return comma_fprintf(fp, first, " %s: %" PRIu64, field, value);
>> -}
>> -
>> -#define if_print(field) printed += __if_fprintf(fp, &first, #field, evsel->attr.field)
>> -
>> -struct bit_names {
>> - int bit;
>> - const char *name;
>> -};
>> -
>> -static int bits__fprintf(FILE *fp, const char *field, u64 value,
>> - struct bit_names *bits, bool *first)
>> -{
>> - int i = 0, printed = comma_fprintf(fp, first, " %s: ", field);
>> - bool first_bit = true;
>> -
>> - do {
>> - if (value & bits[i].bit) {
>> - printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
>> - first_bit = false;
>> - }
>> - } while (bits[++i].name != NULL);
>> -
>> - return printed;
>> -}
>> -
>> -static int sample_type__fprintf(FILE *fp, bool *first, u64 value)
>> -{
>> -#define bit_name(n) { PERF_SAMPLE_##n, #n }
>> - struct bit_names bits[] = {
>> - bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
>> - bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
>> - bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
>> - bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
>> - bit_name(IDENTIFIER), bit_name(REGS_INTR),
>> - { .name = NULL, }
>> - };
>> -#undef bit_name
>> - return bits__fprintf(fp, "sample_type", value, bits, first);
>> -}
>> -
>> -static int read_format__fprintf(FILE *fp, bool *first, u64 value)
>> -{
>> -#define bit_name(n) { PERF_FORMAT_##n, #n }
>> - struct bit_names bits[] = {
>> - bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
>> - bit_name(ID), bit_name(GROUP),
>> - { .name = NULL, }
>> - };
>> -#undef bit_name
>> - return bits__fprintf(fp, "read_format", value, bits, first);
>> -}
>> -
>> int perf_evsel__fprintf(struct perf_evsel *evsel,
>> struct perf_attr_details *details, FILE *fp)
>> {
>> @@ -2080,51 +1984,24 @@ int perf_evsel__fprintf(struct perf_evse
>>
>> printed += fprintf(fp, "%s", perf_evsel__name(evsel));
>>
>> - if (details->verbose || details->freq) {
>> - printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
>> - (u64)evsel->attr.sample_freq);
>> - }
>>
>> if (details->verbose) {
>> - if_print(type);
>> - if_print(config);
>> - if_print(config1);
>> - if_print(config2);
>> - if_print(size);
>> - printed += sample_type__fprintf(fp, &first, evsel->attr.sample_type);
>> - if (evsel->attr.read_format)
>> - printed += read_format__fprintf(fp, &first, evsel->attr.read_format);
>> - if_print(disabled);
>> - if_print(inherit);
>> - if_print(pinned);
>> - if_print(exclusive);
>> - if_print(exclude_user);
>> - if_print(exclude_kernel);
>> - if_print(exclude_hv);
>> - if_print(exclude_idle);
>> - if_print(mmap);
>> - if_print(comm);
>> - if_print(freq);
>> - if_print(inherit_stat);
>> - if_print(enable_on_exec);
>> - if_print(task);
>> - if_print(watermark);
>> - if_print(precise_ip);
>> - if_print(mmap_data);
>> - if_print(sample_id_all);
>> - if_print(exclude_host);
>> - if_print(exclude_guest);
>> - if_print(mmap2);
>> - if_print(comm_exec);
>> - if_print(use_clockid);
>> - if_print(__reserved_1);
>> - if_print(wakeup_events);
>> - if_print(bp_type);
>> - if_print(branch_sample_type);
>> - if_print(sample_regs_user);
>> - if_print(sample_stack_user);
>> - if_print(clockid);
>> - if_print(sample_regs_intr);
>> +
>> +#define PRINT_ATTR(_n, _f, _p) \
>> +do { \
>> + if (evsel->attr._f) { \
>> + printed += comma_fprintf(fp, &first, " %s: ", _n); \
>> + printed += _p(fp, evsel->attr._f); \
>> + } \
>> +} while (0)
>> +
>> +#include "util/print_attr.h"
>> +
>> +#undef PRINT_ATTR
>> +
>> + } else if (details->freq) {
>> + printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
>> + (u64)evsel->attr.sample_freq);
>> }
>> out:
>> fputc('\n', fp);
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -23,6 +23,7 @@
>> #include "strbuf.h"
>> #include "build-id.h"
>> #include "data.h"
>> +#include "print_helper.h"
>>
>> static u32 header_argc;
>> static const char **header_argv;
>> @@ -1069,26 +1070,6 @@ static void print_event_desc(struct perf
>> for (evsel = events; evsel->attr.size; evsel++) {
>> fprintf(fp, "# event : name = %s, ", evsel->name);
>>
>> - fprintf(fp, "type = %d, config = 0x%"PRIx64
>> - ", config1 = 0x%"PRIx64", config2 = 0x%"PRIx64,
>> - evsel->attr.type,
>> - (u64)evsel->attr.config,
>> - (u64)evsel->attr.config1,
>> - (u64)evsel->attr.config2);
>> -
>> - fprintf(fp, ", excl_usr = %d, excl_kern = %d",
>> - evsel->attr.exclude_user,
>> - evsel->attr.exclude_kernel);
>> -
>> - fprintf(fp, ", excl_host = %d, excl_guest = %d",
>> - evsel->attr.exclude_host,
>> - evsel->attr.exclude_guest);
>> -
>> - fprintf(fp, ", precise_ip = %d", evsel->attr.precise_ip);
>> -
>> - fprintf(fp, ", attr_mmap2 = %d", evsel->attr.mmap2);
>> - fprintf(fp, ", attr_mmap = %d", evsel->attr.mmap);
>> - fprintf(fp, ", attr_mmap_data = %d", evsel->attr.mmap_data);
>> if (evsel->ids) {
>> fprintf(fp, ", id = {");
>> for (j = 0, id = evsel->id; j < evsel->ids; j++, id++) {
>> @@ -1098,9 +1079,18 @@ static void print_event_desc(struct perf
>> }
>> fprintf(fp, " }");
>> }
>> - if (evsel->attr.use_clockid)
>> - fprintf(fp, ", clockid = %d", evsel->attr.clockid);
>>
>> +#define PRINT_ATTR(_n, _f, _p) \
>> +do { \
>> + if (evsel->attr._f) { \
>> + fprintf(fp, ", %s =", _n); \
>> + _p(fp, evsel->attr._f); \
>> + } \
>> +} while (0)
>> +
>> +#include "util/print_attr.h"
>> +
>> +#undef PRINT_ATTR
>>
>> fputc('\n', fp);
>> }
>> --- /dev/null
>> +++ b/tools/perf/util/print_attr.h
>> @@ -0,0 +1,69 @@
>> +
>> +#ifndef PRINT_ATTR
>> +/*
>> + * #define PRINT_ATTR(name, field, print) \
>> + * do { \
>> + * fprintf(fp, " %s = ", name); \
>> + * print(fp, attr->field); \
>> + * } while (0)
>> + */
>> +#error "General Error and Major Fault yell at you!"
>> +#endif
>> +
>> +#define p_hex(fp, val) fprintf(fp, "%"PRIx64, (uint64_t)(val))
>> +#define p_unsigned(fp, val) fprintf(fp, "%"PRIu64, (uint64_t)(val))
>> +#define p_signed(fp, val) fprintf(fp, "%"PRId64, (int64_t)(val))
>> +#define p_sample_type(fp, val) sample_type__fprintf(fp, val)
>> +#define p_read_format(fp, val) read_format__fprintf(fp, val)
>> +
>> +#define PRINT_ATTRf(field, print) PRINT_ATTR(#field, field, print)
>> +
>> +PRINT_ATTRf(type, p_unsigned);
>> +PRINT_ATTRf(size, p_unsigned);
>> +PRINT_ATTRf(config, p_hex);
>> +PRINT_ATTR("{ sample_period, sample_freq }", sample_period, p_unsigned);
>> +PRINT_ATTRf(sample_type, p_sample_type);
>> +PRINT_ATTRf(read_format, p_read_format);
>> +
>> +PRINT_ATTRf(disabled, p_unsigned);
>> +PRINT_ATTRf(inherit, p_unsigned);
>> +PRINT_ATTRf(pinned, p_unsigned);
>> +PRINT_ATTRf(exclusive, p_unsigned);
>> +PRINT_ATTRf(exclude_user, p_unsigned);
>> +PRINT_ATTRf(exclude_kernel, p_unsigned);
>> +PRINT_ATTRf(exclude_hv, p_unsigned);
>> +PRINT_ATTRf(exclude_idle, p_unsigned);
>> +PRINT_ATTRf(mmap, p_unsigned);
>> +PRINT_ATTRf(comm, p_unsigned);
>> +PRINT_ATTRf(freq, p_unsigned);
>> +PRINT_ATTRf(inherit_stat, p_unsigned);
>> +PRINT_ATTRf(enable_on_exec, p_unsigned);
>> +PRINT_ATTRf(task, p_unsigned);
>> +PRINT_ATTRf(watermark, p_unsigned);
>> +PRINT_ATTRf(precise_ip, p_unsigned);
>> +PRINT_ATTRf(mmap_data, p_unsigned);
>> +PRINT_ATTRf(sample_id_all, p_unsigned);
>> +PRINT_ATTRf(exclude_host, p_unsigned);
>> +PRINT_ATTRf(exclude_guest, p_unsigned);
>> +PRINT_ATTRf(exclude_callchain_kernel, p_unsigned);
>> +PRINT_ATTRf(exclude_callchain_user, p_unsigned);
>> +PRINT_ATTRf(mmap2, p_unsigned);
>> +PRINT_ATTRf(comm_exec, p_unsigned);
>> +PRINT_ATTRf(use_clockid, p_unsigned);
>> +
>> +PRINT_ATTR("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
>> +PRINT_ATTRf(bp_type, p_unsigned);
>> +PRINT_ATTR("{ bp_addr, config1 }", bp_addr, p_hex);
>> +PRINT_ATTR("{ bp_len, config2 }", bp_len, p_hex);
>> +PRINT_ATTRf(sample_regs_user, p_hex);
>> +PRINT_ATTRf(sample_stack_user, p_unsigned);
>> +PRINT_ATTRf(clockid, p_signed);
>> +PRINT_ATTRf(sample_regs_intr, p_hex);
>> +
>> +#undef PRINT_ATTRf
>> +
>> +#undef p_hex
>> +#undef p_unsigned
>> +#undef p_signed
>> +#undef p_sample_type
>> +#undef p_read_format
>> --- /dev/null
>> +++ b/tools/perf/util/print_helper.c
>> @@ -0,0 +1,52 @@
>> +
>> +#include <linux/perf_event.h>
>> +#include "util.h"
>> +#include "print_helper.h"
>> +
>> +struct bit_names {
>> + int bit;
>> + const char *name;
>> +};
>> +
>> +static int bits__fprintf(FILE *fp, u64 value, struct bit_names *bits)
>> +{
>> + int i = 0, printed = 0;
>> + bool first_bit = true;
>> +
>> + do {
>> + if (value & bits[i].bit) {
>> + printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
>> + first_bit = false;
>> + }
>> + } while (bits[++i].name != NULL);
>> +
>> + return printed;
>> +}
>> +
>> +int sample_type__fprintf(FILE *fp, u64 value)
>> +{
>> +#define bit_name(n) { PERF_SAMPLE_##n, #n }
>> + struct bit_names bits[] = {
>> + bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
>> + bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
>> + bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
>> + bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
>> + bit_name(IDENTIFIER), bit_name(REGS_INTR),
>> + { .name = NULL, }
>> + };
>> +#undef bit_name
>> + return bits__fprintf(fp, value, bits);
>> +}
>> +
>> +int read_format__fprintf(FILE *fp, u64 value)
>> +{
>> +#define bit_name(n) { PERF_FORMAT_##n, #n }
>> + struct bit_names bits[] = {
>> + bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
>> + bit_name(ID), bit_name(GROUP),
>> + { .name = NULL, }
>> + };
>> +#undef bit_name
>> + return bits__fprintf(fp, value, bits);
>> +}
>> +
>> --- /dev/null
>> +++ b/tools/perf/util/print_helper.h
>> @@ -0,0 +1,7 @@
>> +#ifndef PRINT_HELPER_H
>> +#define PRINT_HELPER_H
>> +
>> +extern int sample_type__fprintf(FILE *fp, u64 value);
>> +extern int read_format__fprintf(FILE *fp, u64 value);
>> +
>> +#endif
>
>

2015-04-02 09:20:30

by Jiri Olsa

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing

On Wed, Apr 01, 2015 at 06:26:38PM +0200, Peter Zijlstra wrote:

SNIP

> - ret += PRINT_ATTR_U32(wakeup_events);
> - ret += PRINT_ATTR_U32(wakeup_watermark);
> - ret += PRINT_ATTR_X32(bp_type);
> - ret += PRINT_ATTR_X64(bp_addr);
> - ret += PRINT_ATTR_X64(config1);
> - ret += PRINT_ATTR_U64(bp_len);
> - ret += PRINT_ATTR_X64(config2);
> - ret += PRINT_ATTR_X64(branch_sample_type);
> - ret += PRINT_ATTR_X64(sample_regs_user);
> - ret += PRINT_ATTR_U32(sample_stack_user);
> - ret += PRINT_ATTR_U32(clockid);
> - ret += PRINT_ATTR_X64(sample_regs_intr);
> +#define PRINT_ATTR(_n, _f, _p) \
> +do { \
> + if (attr->_f) { \
> + ret += fprintf(fp, " %-32s ", _n); \
> + ret += _p(fp, attr->_f); \
> + ret += fprintf(fp, "\n"); \

missing indent ;-) other it seems ok

Acked-by: Jiri Olsa <[email protected]>

jirka

2015-04-02 11:59:53

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing

On Thu, Apr 02, 2015 at 12:01:53PM +0300, Adrian Hunter wrote:
> But personally I think the "include" approach is too ugly. I would just
> add a function instead. Something like:

You've not stared at the kernel tracepoint code long enough ;-)

Something like so then?

---
Subject: perf, tools: Merge all perf_event_attr print functions
From: Peter Zijlstra <[email protected]>
Date: Thu Apr 2 11:49:23 CEST 2015

Currently there's 3 (that I found) different and incomplete
implementations of printing perf_event_attr.

This is quite silly. Merge the lot.

While this patch does not retain the exact form all printing that I
found is debug output and thus it should not be critical.

Also, I cannot find a single print_event_desc() caller.

Pre:

$ perf record -vv -e cycles -- sleep 1
------------------------------------------------------------
perf_event_attr:
type 0
size 104
config 0
sample_period 4000
sample_freq 4000
sample_type 0x107
read_format 0
disabled 1 inherit 1
pinned 0 exclusive 0
exclude_user 0 exclude_kernel 0
exclude_hv 0 exclude_idle 0
mmap 1 comm 1
mmap2 1 comm_exec 1
freq 1 inherit_stat 0
enable_on_exec 1 task 1
watermark 0 precise_ip 0
mmap_data 0 sample_id_all 1
exclude_host 0 exclude_guest 1
excl.callchain_kern 0 excl.callchain_user 0
wakeup_events 0
wakeup_watermark 0
bp_type 0
bp_addr 0
config1 0
bp_len 0
config2 0
branch_sample_type 0
sample_regs_user 0
sample_stack_user 0
sample_regs_intr 0
------------------------------------------------------------

$ perf evlist -vv
cycles: sample_freq=4000, size: 104, sample_type: IP|TID|TIME|PERIOD,
disabled: 1, inherit: 1, mmap: 1, mmap2: 1, comm: 1, comm_exec: 1,
freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest:
1

Post:

$ ./perf record -vv -e cycles -- sleep 1
------------------------------------------------------------
perf_event_attr:
size 112
{ sample_period, sample_freq } 4000
sample_type IP|TID|TIME|PERIOD
disabled 1
inherit 1
mmap 1
comm 1
freq 1
enable_on_exec 1
task 1
sample_id_all 1
exclude_guest 1
mmap2 1
comm_exec 1
------------------------------------------------------------

$ ./perf evlist -vv
cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type:
IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq:
1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1,
mmap2: 1, comm_exec: 1

Cc: [email protected]
Cc: [email protected]
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
tools/perf/util/evsel.c | 284 ++++++++++++++++++++---------------------------
tools/perf/util/evsel.h | 6
tools/perf/util/header.c | 29 +---
3 files changed, 139 insertions(+), 180 deletions(-)

--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1011,70 +1011,126 @@ static int get_group_fd(struct perf_evse
return fd;
}

-#define __PRINT_ATTR(fmt, cast, field) \
- fprintf(fp, " %-19s "fmt"\n", #field, cast attr->field)
+struct bit_names {
+ int bit;
+ const char *name;
+};
+
+static void __p_bits(char *buf, size_t size, u64 value, struct bit_names *bits)
+{
+ bool first_bit = true;
+ int i = 0;
+
+ do {
+ if (value & bits[i].bit) {
+ buf += scnprintf(buf, size, "%s%s", first_bit ? "" : "|", bits[i].name);
+ first_bit = false;
+ }
+ } while (bits[++i].name != NULL);
+}
+
+static void __p_sample_type(char *buf, size_t size, u64 value)
+{
+#define bit_name(n) { PERF_SAMPLE_##n, #n }
+ struct bit_names bits[] = {
+ bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
+ bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
+ bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
+ bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
+ bit_name(IDENTIFIER), bit_name(REGS_INTR),
+ { .name = NULL, }
+ };
+#undef bit_name
+ __p_bits(buf, size, value, bits);
+}

-#define PRINT_ATTR_U32(field) __PRINT_ATTR("%u" , , field)
-#define PRINT_ATTR_X32(field) __PRINT_ATTR("%#x", , field)
-#define PRINT_ATTR_U64(field) __PRINT_ATTR("%" PRIu64, (uint64_t), field)
-#define PRINT_ATTR_X64(field) __PRINT_ATTR("%#"PRIx64, (uint64_t), field)
-
-#define PRINT_ATTR2N(name1, field1, name2, field2) \
- fprintf(fp, " %-19s %u %-19s %u\n", \
- name1, attr->field1, name2, attr->field2)
-
-#define PRINT_ATTR2(field1, field2) \
- PRINT_ATTR2N(#field1, field1, #field2, field2)
-
-static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
-{
- size_t ret = 0;
-
- ret += fprintf(fp, "%.60s\n", graph_dotted_line);
- ret += fprintf(fp, "perf_event_attr:\n");
-
- ret += PRINT_ATTR_U32(type);
- ret += PRINT_ATTR_U32(size);
- ret += PRINT_ATTR_X64(config);
- ret += PRINT_ATTR_U64(sample_period);
- ret += PRINT_ATTR_U64(sample_freq);
- ret += PRINT_ATTR_X64(sample_type);
- ret += PRINT_ATTR_X64(read_format);
-
- ret += PRINT_ATTR2(disabled, inherit);
- ret += PRINT_ATTR2(pinned, exclusive);
- ret += PRINT_ATTR2(exclude_user, exclude_kernel);
- ret += PRINT_ATTR2(exclude_hv, exclude_idle);
- ret += PRINT_ATTR2(mmap, comm);
- ret += PRINT_ATTR2(freq, inherit_stat);
- ret += PRINT_ATTR2(enable_on_exec, task);
- ret += PRINT_ATTR2(watermark, precise_ip);
- ret += PRINT_ATTR2(mmap_data, sample_id_all);
- ret += PRINT_ATTR2(exclude_host, exclude_guest);
- ret += PRINT_ATTR2N("excl.callchain_kern", exclude_callchain_kernel,
- "excl.callchain_user", exclude_callchain_user);
- ret += PRINT_ATTR2(mmap2, comm_exec);
- ret += __PRINT_ATTR("%u",,use_clockid);
-
-
- ret += PRINT_ATTR_U32(wakeup_events);
- ret += PRINT_ATTR_U32(wakeup_watermark);
- ret += PRINT_ATTR_X32(bp_type);
- ret += PRINT_ATTR_X64(bp_addr);
- ret += PRINT_ATTR_X64(config1);
- ret += PRINT_ATTR_U64(bp_len);
- ret += PRINT_ATTR_X64(config2);
- ret += PRINT_ATTR_X64(branch_sample_type);
- ret += PRINT_ATTR_X64(sample_regs_user);
- ret += PRINT_ATTR_U32(sample_stack_user);
- ret += PRINT_ATTR_U32(clockid);
- ret += PRINT_ATTR_X64(sample_regs_intr);
+static void __p_read_format(char *buf, size_t size, u64 value)
+{
+#define bit_name(n) { PERF_FORMAT_##n, #n }
+ struct bit_names bits[] = {
+ bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
+ bit_name(ID), bit_name(GROUP),
+ { .name = NULL, }
+ };
+#undef bit_name
+ __p_bits(buf, size, value, bits);
+}
+
+#define BUF_SIZE 1024
+
+#define p_hex(val) snprintf(buf, BUF_SIZE, "%"PRIx64, (uint64_t)(val))
+#define p_unsigned(val) snprintf(buf, BUF_SIZE, "%"PRIu64, (uint64_t)(val))
+#define p_signed(val) snprintf(buf, BUF_SIZE, "%"PRId64, (int64_t)(val))
+#define p_sample_type(val) __p_sample_type(buf, BUF_SIZE, val)
+#define p_read_format(val) __p_read_format(buf, BUF_SIZE, val)
+
+#define PRINT_ATTRn(_n, _f, _p) \
+do { \
+ if (attr->_f) { \
+ _p(attr->_f); \
+ ret += attr__fprintf(fp, _n, buf, priv);\
+ } \
+} while (0)

- ret += fprintf(fp, "%.60s\n", graph_dotted_line);
+#define PRINT_ATTRf(_f, _p) PRINT_ATTRn(#_f, _f, _p)
+
+int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
+ attr__fprintf_f attr__fprintf, void *priv)
+{
+ char buf[BUF_SIZE];
+ int ret = 0;
+
+ PRINT_ATTRf(type, p_unsigned);
+ PRINT_ATTRf(size, p_unsigned);
+ PRINT_ATTRf(config, p_hex);
+ PRINT_ATTRn("{ sample_period, sample_freq }", sample_period, p_unsigned);
+ PRINT_ATTRf(sample_type, p_sample_type);
+ PRINT_ATTRf(read_format, p_read_format);
+
+ PRINT_ATTRf(disabled, p_unsigned);
+ PRINT_ATTRf(inherit, p_unsigned);
+ PRINT_ATTRf(pinned, p_unsigned);
+ PRINT_ATTRf(exclusive, p_unsigned);
+ PRINT_ATTRf(exclude_user, p_unsigned);
+ PRINT_ATTRf(exclude_kernel, p_unsigned);
+ PRINT_ATTRf(exclude_hv, p_unsigned);
+ PRINT_ATTRf(exclude_idle, p_unsigned);
+ PRINT_ATTRf(mmap, p_unsigned);
+ PRINT_ATTRf(comm, p_unsigned);
+ PRINT_ATTRf(freq, p_unsigned);
+ PRINT_ATTRf(inherit_stat, p_unsigned);
+ PRINT_ATTRf(enable_on_exec, p_unsigned);
+ PRINT_ATTRf(task, p_unsigned);
+ PRINT_ATTRf(watermark, p_unsigned);
+ PRINT_ATTRf(precise_ip, p_unsigned);
+ PRINT_ATTRf(mmap_data, p_unsigned);
+ PRINT_ATTRf(sample_id_all, p_unsigned);
+ PRINT_ATTRf(exclude_host, p_unsigned);
+ PRINT_ATTRf(exclude_guest, p_unsigned);
+ PRINT_ATTRf(exclude_callchain_kernel, p_unsigned);
+ PRINT_ATTRf(exclude_callchain_user, p_unsigned);
+ PRINT_ATTRf(mmap2, p_unsigned);
+ PRINT_ATTRf(comm_exec, p_unsigned);
+ PRINT_ATTRf(use_clockid, p_unsigned);
+
+ PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
+ PRINT_ATTRf(bp_type, p_unsigned);
+ PRINT_ATTRn("{ bp_addr, config1 }", bp_addr, p_hex);
+ PRINT_ATTRn("{ bp_len, config2 }", bp_len, p_hex);
+ PRINT_ATTRf(sample_regs_user, p_hex);
+ PRINT_ATTRf(sample_stack_user, p_unsigned);
+ PRINT_ATTRf(clockid, p_signed);
+ PRINT_ATTRf(sample_regs_intr, p_hex);

return ret;
}

+static int __open_attr__fprintf(FILE *fp, const char *name, const char *val,
+ void *priv __attribute__((unused)))
+{
+ return fprintf(fp, " %-32s %s\n", name, val);
+}
+
static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
struct thread_map *threads)
{
@@ -1114,8 +1170,12 @@ static int __perf_evsel__open(struct per
if (perf_missing_features.sample_id_all)
evsel->attr.sample_id_all = 0;

- if (verbose >= 2)
- perf_event_attr__fprintf(&evsel->attr, stderr);
+ if (verbose >= 2) {
+ fprintf(stderr, "%.60s\n", graph_dotted_line);
+ fprintf(stderr, "perf_event_attr:\n");
+ perf_event_attr__fprintf(stderr, &evsel->attr, __open_attr__fprintf, NULL);
+ fprintf(stderr, "%.60s\n", graph_dotted_line);
+ }

for (cpu = 0; cpu < cpus->nr; cpu++) {

@@ -1996,62 +2056,9 @@ static int comma_fprintf(FILE *fp, bool
return ret;
}

-static int __if_fprintf(FILE *fp, bool *first, const char *field, u64 value)
+static int __print_attr__fprintf(FILE *fp, const char *name, const char *val, void *priv)
{
- if (value == 0)
- return 0;
-
- return comma_fprintf(fp, first, " %s: %" PRIu64, field, value);
-}
-
-#define if_print(field) printed += __if_fprintf(fp, &first, #field, evsel->attr.field)
-
-struct bit_names {
- int bit;
- const char *name;
-};
-
-static int bits__fprintf(FILE *fp, const char *field, u64 value,
- struct bit_names *bits, bool *first)
-{
- int i = 0, printed = comma_fprintf(fp, first, " %s: ", field);
- bool first_bit = true;
-
- do {
- if (value & bits[i].bit) {
- printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
- first_bit = false;
- }
- } while (bits[++i].name != NULL);
-
- return printed;
-}
-
-static int sample_type__fprintf(FILE *fp, bool *first, u64 value)
-{
-#define bit_name(n) { PERF_SAMPLE_##n, #n }
- struct bit_names bits[] = {
- bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
- bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
- bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
- bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
- bit_name(IDENTIFIER), bit_name(REGS_INTR),
- { .name = NULL, }
- };
-#undef bit_name
- return bits__fprintf(fp, "sample_type", value, bits, first);
-}
-
-static int read_format__fprintf(FILE *fp, bool *first, u64 value)
-{
-#define bit_name(n) { PERF_FORMAT_##n, #n }
- struct bit_names bits[] = {
- bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
- bit_name(ID), bit_name(GROUP),
- { .name = NULL, }
- };
-#undef bit_name
- return bits__fprintf(fp, "read_format", value, bits, first);
+ return comma_fprintf(fp, (bool *)priv, " %s: %s", name, val);
}

int perf_evsel__fprintf(struct perf_evsel *evsel,
@@ -2080,52 +2087,13 @@ int perf_evsel__fprintf(struct perf_evse

printed += fprintf(fp, "%s", perf_evsel__name(evsel));

- if (details->verbose || details->freq) {
+ if (details->verbose) {
+ printed += perf_event_attr__fprintf(fp, &evsel->attr,
+ __print_attr__fprintf, &first);
+ } else if (details->freq) {
printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
(u64)evsel->attr.sample_freq);
}
-
- if (details->verbose) {
- if_print(type);
- if_print(config);
- if_print(config1);
- if_print(config2);
- if_print(size);
- printed += sample_type__fprintf(fp, &first, evsel->attr.sample_type);
- if (evsel->attr.read_format)
- printed += read_format__fprintf(fp, &first, evsel->attr.read_format);
- if_print(disabled);
- if_print(inherit);
- if_print(pinned);
- if_print(exclusive);
- if_print(exclude_user);
- if_print(exclude_kernel);
- if_print(exclude_hv);
- if_print(exclude_idle);
- if_print(mmap);
- if_print(comm);
- if_print(freq);
- if_print(inherit_stat);
- if_print(enable_on_exec);
- if_print(task);
- if_print(watermark);
- if_print(precise_ip);
- if_print(mmap_data);
- if_print(sample_id_all);
- if_print(exclude_host);
- if_print(exclude_guest);
- if_print(mmap2);
- if_print(comm_exec);
- if_print(use_clockid);
- if_print(__reserved_1);
- if_print(wakeup_events);
- if_print(bp_type);
- if_print(branch_sample_type);
- if_print(sample_regs_user);
- if_print(sample_stack_user);
- if_print(clockid);
- if_print(sample_regs_intr);
- }
out:
fputc('\n', fp);
return ++printed;
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -359,4 +359,10 @@ static inline bool has_branch_callstack(
{
return evsel->attr.branch_sample_type & PERF_SAMPLE_BRANCH_CALL_STACK;
}
+
+typedef int (*attr__fprintf_f)(FILE *, const char *, const char *, void *);
+
+int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
+ attr__fprintf_f attr__fprintf, void *priv);
+
#endif /* __PERF_EVSEL_H */
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1055,6 +1055,12 @@ read_event_desc(struct perf_header *ph,
goto out;
}

+static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,
+ void *priv __attribute__((unused)))
+{
+ return fprintf(fp, ", %s = %s", name, val);
+}
+
static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
{
struct perf_evsel *evsel, *events = read_event_desc(ph, fd);
@@ -1069,26 +1075,6 @@ static void print_event_desc(struct perf
for (evsel = events; evsel->attr.size; evsel++) {
fprintf(fp, "# event : name = %s, ", evsel->name);

- fprintf(fp, "type = %d, config = 0x%"PRIx64
- ", config1 = 0x%"PRIx64", config2 = 0x%"PRIx64,
- evsel->attr.type,
- (u64)evsel->attr.config,
- (u64)evsel->attr.config1,
- (u64)evsel->attr.config2);
-
- fprintf(fp, ", excl_usr = %d, excl_kern = %d",
- evsel->attr.exclude_user,
- evsel->attr.exclude_kernel);
-
- fprintf(fp, ", excl_host = %d, excl_guest = %d",
- evsel->attr.exclude_host,
- evsel->attr.exclude_guest);
-
- fprintf(fp, ", precise_ip = %d", evsel->attr.precise_ip);
-
- fprintf(fp, ", attr_mmap2 = %d", evsel->attr.mmap2);
- fprintf(fp, ", attr_mmap = %d", evsel->attr.mmap);
- fprintf(fp, ", attr_mmap_data = %d", evsel->attr.mmap_data);
if (evsel->ids) {
fprintf(fp, ", id = {");
for (j = 0, id = evsel->id; j < evsel->ids; j++, id++) {
@@ -1098,9 +1084,8 @@ static void print_event_desc(struct perf
}
fprintf(fp, " }");
}
- if (evsel->attr.use_clockid)
- fprintf(fp, ", clockid = %d", evsel->attr.clockid);

+ perf_event_attr__fprintf(fp, &evsel->attr, __desc_attr__fprintf, NULL);

fputc('\n', fp);
}

2015-04-02 12:56:42

by Adrian Hunter

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing

On 02/04/15 14:59, Peter Zijlstra wrote:
> On Thu, Apr 02, 2015 at 12:01:53PM +0300, Adrian Hunter wrote:
>> But personally I think the "include" approach is too ugly. I would just
>> add a function instead. Something like:
>
> You've not stared at the kernel tracepoint code long enough ;-)

I will try some day when I feel too sane :-)

>
> Something like so then?

Looks fine. Didn't test it, but

Acked-by: Adrian Hunter <[email protected]>

>
> ---
> Subject: perf, tools: Merge all perf_event_attr print functions
> From: Peter Zijlstra <[email protected]>
> Date: Thu Apr 2 11:49:23 CEST 2015
>
> Currently there's 3 (that I found) different and incomplete
> implementations of printing perf_event_attr.
>
> This is quite silly. Merge the lot.
>
> While this patch does not retain the exact form all printing that I
> found is debug output and thus it should not be critical.
>
> Also, I cannot find a single print_event_desc() caller.
>
> Pre:
>
> $ perf record -vv -e cycles -- sleep 1
> ------------------------------------------------------------
> perf_event_attr:
> type 0
> size 104
> config 0
> sample_period 4000
> sample_freq 4000
> sample_type 0x107
> read_format 0
> disabled 1 inherit 1
> pinned 0 exclusive 0
> exclude_user 0 exclude_kernel 0
> exclude_hv 0 exclude_idle 0
> mmap 1 comm 1
> mmap2 1 comm_exec 1
> freq 1 inherit_stat 0
> enable_on_exec 1 task 1
> watermark 0 precise_ip 0
> mmap_data 0 sample_id_all 1
> exclude_host 0 exclude_guest 1
> excl.callchain_kern 0 excl.callchain_user 0
> wakeup_events 0
> wakeup_watermark 0
> bp_type 0
> bp_addr 0
> config1 0
> bp_len 0
> config2 0
> branch_sample_type 0
> sample_regs_user 0
> sample_stack_user 0
> sample_regs_intr 0
> ------------------------------------------------------------
>
> $ perf evlist -vv
> cycles: sample_freq=4000, size: 104, sample_type: IP|TID|TIME|PERIOD,
> disabled: 1, inherit: 1, mmap: 1, mmap2: 1, comm: 1, comm_exec: 1,
> freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest:
> 1
>
> Post:
>
> $ ./perf record -vv -e cycles -- sleep 1
> ------------------------------------------------------------
> perf_event_attr:
> size 112
> { sample_period, sample_freq } 4000
> sample_type IP|TID|TIME|PERIOD
> disabled 1
> inherit 1
> mmap 1
> comm 1
> freq 1
> enable_on_exec 1
> task 1
> sample_id_all 1
> exclude_guest 1
> mmap2 1
> comm_exec 1
> ------------------------------------------------------------
>
> $ ./perf evlist -vv
> cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type:
> IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq:
> 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1,
> mmap2: 1, comm_exec: 1
>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> tools/perf/util/evsel.c | 284 ++++++++++++++++++++---------------------------
> tools/perf/util/evsel.h | 6
> tools/perf/util/header.c | 29 +---
> 3 files changed, 139 insertions(+), 180 deletions(-)
>
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1011,70 +1011,126 @@ static int get_group_fd(struct perf_evse
> return fd;
> }
>
> -#define __PRINT_ATTR(fmt, cast, field) \
> - fprintf(fp, " %-19s "fmt"\n", #field, cast attr->field)
> +struct bit_names {
> + int bit;
> + const char *name;
> +};
> +
> +static void __p_bits(char *buf, size_t size, u64 value, struct bit_names *bits)
> +{
> + bool first_bit = true;
> + int i = 0;
> +
> + do {
> + if (value & bits[i].bit) {
> + buf += scnprintf(buf, size, "%s%s", first_bit ? "" : "|", bits[i].name);
> + first_bit = false;
> + }
> + } while (bits[++i].name != NULL);
> +}
> +
> +static void __p_sample_type(char *buf, size_t size, u64 value)
> +{
> +#define bit_name(n) { PERF_SAMPLE_##n, #n }
> + struct bit_names bits[] = {
> + bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
> + bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
> + bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
> + bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
> + bit_name(IDENTIFIER), bit_name(REGS_INTR),
> + { .name = NULL, }
> + };
> +#undef bit_name
> + __p_bits(buf, size, value, bits);
> +}
>
> -#define PRINT_ATTR_U32(field) __PRINT_ATTR("%u" , , field)
> -#define PRINT_ATTR_X32(field) __PRINT_ATTR("%#x", , field)
> -#define PRINT_ATTR_U64(field) __PRINT_ATTR("%" PRIu64, (uint64_t), field)
> -#define PRINT_ATTR_X64(field) __PRINT_ATTR("%#"PRIx64, (uint64_t), field)
> -
> -#define PRINT_ATTR2N(name1, field1, name2, field2) \
> - fprintf(fp, " %-19s %u %-19s %u\n", \
> - name1, attr->field1, name2, attr->field2)
> -
> -#define PRINT_ATTR2(field1, field2) \
> - PRINT_ATTR2N(#field1, field1, #field2, field2)
> -
> -static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
> -{
> - size_t ret = 0;
> -
> - ret += fprintf(fp, "%.60s\n", graph_dotted_line);
> - ret += fprintf(fp, "perf_event_attr:\n");
> -
> - ret += PRINT_ATTR_U32(type);
> - ret += PRINT_ATTR_U32(size);
> - ret += PRINT_ATTR_X64(config);
> - ret += PRINT_ATTR_U64(sample_period);
> - ret += PRINT_ATTR_U64(sample_freq);
> - ret += PRINT_ATTR_X64(sample_type);
> - ret += PRINT_ATTR_X64(read_format);
> -
> - ret += PRINT_ATTR2(disabled, inherit);
> - ret += PRINT_ATTR2(pinned, exclusive);
> - ret += PRINT_ATTR2(exclude_user, exclude_kernel);
> - ret += PRINT_ATTR2(exclude_hv, exclude_idle);
> - ret += PRINT_ATTR2(mmap, comm);
> - ret += PRINT_ATTR2(freq, inherit_stat);
> - ret += PRINT_ATTR2(enable_on_exec, task);
> - ret += PRINT_ATTR2(watermark, precise_ip);
> - ret += PRINT_ATTR2(mmap_data, sample_id_all);
> - ret += PRINT_ATTR2(exclude_host, exclude_guest);
> - ret += PRINT_ATTR2N("excl.callchain_kern", exclude_callchain_kernel,
> - "excl.callchain_user", exclude_callchain_user);
> - ret += PRINT_ATTR2(mmap2, comm_exec);
> - ret += __PRINT_ATTR("%u",,use_clockid);
> -
> -
> - ret += PRINT_ATTR_U32(wakeup_events);
> - ret += PRINT_ATTR_U32(wakeup_watermark);
> - ret += PRINT_ATTR_X32(bp_type);
> - ret += PRINT_ATTR_X64(bp_addr);
> - ret += PRINT_ATTR_X64(config1);
> - ret += PRINT_ATTR_U64(bp_len);
> - ret += PRINT_ATTR_X64(config2);
> - ret += PRINT_ATTR_X64(branch_sample_type);
> - ret += PRINT_ATTR_X64(sample_regs_user);
> - ret += PRINT_ATTR_U32(sample_stack_user);
> - ret += PRINT_ATTR_U32(clockid);
> - ret += PRINT_ATTR_X64(sample_regs_intr);
> +static void __p_read_format(char *buf, size_t size, u64 value)
> +{
> +#define bit_name(n) { PERF_FORMAT_##n, #n }
> + struct bit_names bits[] = {
> + bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
> + bit_name(ID), bit_name(GROUP),
> + { .name = NULL, }
> + };
> +#undef bit_name
> + __p_bits(buf, size, value, bits);
> +}
> +
> +#define BUF_SIZE 1024
> +
> +#define p_hex(val) snprintf(buf, BUF_SIZE, "%"PRIx64, (uint64_t)(val))
> +#define p_unsigned(val) snprintf(buf, BUF_SIZE, "%"PRIu64, (uint64_t)(val))
> +#define p_signed(val) snprintf(buf, BUF_SIZE, "%"PRId64, (int64_t)(val))
> +#define p_sample_type(val) __p_sample_type(buf, BUF_SIZE, val)
> +#define p_read_format(val) __p_read_format(buf, BUF_SIZE, val)
> +
> +#define PRINT_ATTRn(_n, _f, _p) \
> +do { \
> + if (attr->_f) { \
> + _p(attr->_f); \
> + ret += attr__fprintf(fp, _n, buf, priv);\
> + } \
> +} while (0)
>
> - ret += fprintf(fp, "%.60s\n", graph_dotted_line);
> +#define PRINT_ATTRf(_f, _p) PRINT_ATTRn(#_f, _f, _p)
> +
> +int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
> + attr__fprintf_f attr__fprintf, void *priv)
> +{
> + char buf[BUF_SIZE];
> + int ret = 0;
> +
> + PRINT_ATTRf(type, p_unsigned);
> + PRINT_ATTRf(size, p_unsigned);
> + PRINT_ATTRf(config, p_hex);
> + PRINT_ATTRn("{ sample_period, sample_freq }", sample_period, p_unsigned);
> + PRINT_ATTRf(sample_type, p_sample_type);
> + PRINT_ATTRf(read_format, p_read_format);
> +
> + PRINT_ATTRf(disabled, p_unsigned);
> + PRINT_ATTRf(inherit, p_unsigned);
> + PRINT_ATTRf(pinned, p_unsigned);
> + PRINT_ATTRf(exclusive, p_unsigned);
> + PRINT_ATTRf(exclude_user, p_unsigned);
> + PRINT_ATTRf(exclude_kernel, p_unsigned);
> + PRINT_ATTRf(exclude_hv, p_unsigned);
> + PRINT_ATTRf(exclude_idle, p_unsigned);
> + PRINT_ATTRf(mmap, p_unsigned);
> + PRINT_ATTRf(comm, p_unsigned);
> + PRINT_ATTRf(freq, p_unsigned);
> + PRINT_ATTRf(inherit_stat, p_unsigned);
> + PRINT_ATTRf(enable_on_exec, p_unsigned);
> + PRINT_ATTRf(task, p_unsigned);
> + PRINT_ATTRf(watermark, p_unsigned);
> + PRINT_ATTRf(precise_ip, p_unsigned);
> + PRINT_ATTRf(mmap_data, p_unsigned);
> + PRINT_ATTRf(sample_id_all, p_unsigned);
> + PRINT_ATTRf(exclude_host, p_unsigned);
> + PRINT_ATTRf(exclude_guest, p_unsigned);
> + PRINT_ATTRf(exclude_callchain_kernel, p_unsigned);
> + PRINT_ATTRf(exclude_callchain_user, p_unsigned);
> + PRINT_ATTRf(mmap2, p_unsigned);
> + PRINT_ATTRf(comm_exec, p_unsigned);
> + PRINT_ATTRf(use_clockid, p_unsigned);
> +
> + PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
> + PRINT_ATTRf(bp_type, p_unsigned);
> + PRINT_ATTRn("{ bp_addr, config1 }", bp_addr, p_hex);
> + PRINT_ATTRn("{ bp_len, config2 }", bp_len, p_hex);
> + PRINT_ATTRf(sample_regs_user, p_hex);
> + PRINT_ATTRf(sample_stack_user, p_unsigned);
> + PRINT_ATTRf(clockid, p_signed);
> + PRINT_ATTRf(sample_regs_intr, p_hex);
>
> return ret;
> }
>
> +static int __open_attr__fprintf(FILE *fp, const char *name, const char *val,
> + void *priv __attribute__((unused)))
> +{
> + return fprintf(fp, " %-32s %s\n", name, val);
> +}
> +
> static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
> struct thread_map *threads)
> {
> @@ -1114,8 +1170,12 @@ static int __perf_evsel__open(struct per
> if (perf_missing_features.sample_id_all)
> evsel->attr.sample_id_all = 0;
>
> - if (verbose >= 2)
> - perf_event_attr__fprintf(&evsel->attr, stderr);
> + if (verbose >= 2) {
> + fprintf(stderr, "%.60s\n", graph_dotted_line);
> + fprintf(stderr, "perf_event_attr:\n");
> + perf_event_attr__fprintf(stderr, &evsel->attr, __open_attr__fprintf, NULL);
> + fprintf(stderr, "%.60s\n", graph_dotted_line);
> + }
>
> for (cpu = 0; cpu < cpus->nr; cpu++) {
>
> @@ -1996,62 +2056,9 @@ static int comma_fprintf(FILE *fp, bool
> return ret;
> }
>
> -static int __if_fprintf(FILE *fp, bool *first, const char *field, u64 value)
> +static int __print_attr__fprintf(FILE *fp, const char *name, const char *val, void *priv)
> {
> - if (value == 0)
> - return 0;
> -
> - return comma_fprintf(fp, first, " %s: %" PRIu64, field, value);
> -}
> -
> -#define if_print(field) printed += __if_fprintf(fp, &first, #field, evsel->attr.field)
> -
> -struct bit_names {
> - int bit;
> - const char *name;
> -};
> -
> -static int bits__fprintf(FILE *fp, const char *field, u64 value,
> - struct bit_names *bits, bool *first)
> -{
> - int i = 0, printed = comma_fprintf(fp, first, " %s: ", field);
> - bool first_bit = true;
> -
> - do {
> - if (value & bits[i].bit) {
> - printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
> - first_bit = false;
> - }
> - } while (bits[++i].name != NULL);
> -
> - return printed;
> -}
> -
> -static int sample_type__fprintf(FILE *fp, bool *first, u64 value)
> -{
> -#define bit_name(n) { PERF_SAMPLE_##n, #n }
> - struct bit_names bits[] = {
> - bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
> - bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
> - bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
> - bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
> - bit_name(IDENTIFIER), bit_name(REGS_INTR),
> - { .name = NULL, }
> - };
> -#undef bit_name
> - return bits__fprintf(fp, "sample_type", value, bits, first);
> -}
> -
> -static int read_format__fprintf(FILE *fp, bool *first, u64 value)
> -{
> -#define bit_name(n) { PERF_FORMAT_##n, #n }
> - struct bit_names bits[] = {
> - bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
> - bit_name(ID), bit_name(GROUP),
> - { .name = NULL, }
> - };
> -#undef bit_name
> - return bits__fprintf(fp, "read_format", value, bits, first);
> + return comma_fprintf(fp, (bool *)priv, " %s: %s", name, val);
> }
>
> int perf_evsel__fprintf(struct perf_evsel *evsel,
> @@ -2080,52 +2087,13 @@ int perf_evsel__fprintf(struct perf_evse
>
> printed += fprintf(fp, "%s", perf_evsel__name(evsel));
>
> - if (details->verbose || details->freq) {
> + if (details->verbose) {
> + printed += perf_event_attr__fprintf(fp, &evsel->attr,
> + __print_attr__fprintf, &first);
> + } else if (details->freq) {
> printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
> (u64)evsel->attr.sample_freq);
> }
> -
> - if (details->verbose) {
> - if_print(type);
> - if_print(config);
> - if_print(config1);
> - if_print(config2);
> - if_print(size);
> - printed += sample_type__fprintf(fp, &first, evsel->attr.sample_type);
> - if (evsel->attr.read_format)
> - printed += read_format__fprintf(fp, &first, evsel->attr.read_format);
> - if_print(disabled);
> - if_print(inherit);
> - if_print(pinned);
> - if_print(exclusive);
> - if_print(exclude_user);
> - if_print(exclude_kernel);
> - if_print(exclude_hv);
> - if_print(exclude_idle);
> - if_print(mmap);
> - if_print(comm);
> - if_print(freq);
> - if_print(inherit_stat);
> - if_print(enable_on_exec);
> - if_print(task);
> - if_print(watermark);
> - if_print(precise_ip);
> - if_print(mmap_data);
> - if_print(sample_id_all);
> - if_print(exclude_host);
> - if_print(exclude_guest);
> - if_print(mmap2);
> - if_print(comm_exec);
> - if_print(use_clockid);
> - if_print(__reserved_1);
> - if_print(wakeup_events);
> - if_print(bp_type);
> - if_print(branch_sample_type);
> - if_print(sample_regs_user);
> - if_print(sample_stack_user);
> - if_print(clockid);
> - if_print(sample_regs_intr);
> - }
> out:
> fputc('\n', fp);
> return ++printed;
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -359,4 +359,10 @@ static inline bool has_branch_callstack(
> {
> return evsel->attr.branch_sample_type & PERF_SAMPLE_BRANCH_CALL_STACK;
> }
> +
> +typedef int (*attr__fprintf_f)(FILE *, const char *, const char *, void *);
> +
> +int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
> + attr__fprintf_f attr__fprintf, void *priv);
> +
> #endif /* __PERF_EVSEL_H */
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1055,6 +1055,12 @@ read_event_desc(struct perf_header *ph,
> goto out;
> }
>
> +static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,
> + void *priv __attribute__((unused)))
> +{
> + return fprintf(fp, ", %s = %s", name, val);
> +}
> +
> static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
> {
> struct perf_evsel *evsel, *events = read_event_desc(ph, fd);
> @@ -1069,26 +1075,6 @@ static void print_event_desc(struct perf
> for (evsel = events; evsel->attr.size; evsel++) {
> fprintf(fp, "# event : name = %s, ", evsel->name);
>
> - fprintf(fp, "type = %d, config = 0x%"PRIx64
> - ", config1 = 0x%"PRIx64", config2 = 0x%"PRIx64,
> - evsel->attr.type,
> - (u64)evsel->attr.config,
> - (u64)evsel->attr.config1,
> - (u64)evsel->attr.config2);
> -
> - fprintf(fp, ", excl_usr = %d, excl_kern = %d",
> - evsel->attr.exclude_user,
> - evsel->attr.exclude_kernel);
> -
> - fprintf(fp, ", excl_host = %d, excl_guest = %d",
> - evsel->attr.exclude_host,
> - evsel->attr.exclude_guest);
> -
> - fprintf(fp, ", precise_ip = %d", evsel->attr.precise_ip);
> -
> - fprintf(fp, ", attr_mmap2 = %d", evsel->attr.mmap2);
> - fprintf(fp, ", attr_mmap = %d", evsel->attr.mmap);
> - fprintf(fp, ", attr_mmap_data = %d", evsel->attr.mmap_data);
> if (evsel->ids) {
> fprintf(fp, ", id = {");
> for (j = 0, id = evsel->id; j < evsel->ids; j++, id++) {
> @@ -1098,9 +1084,8 @@ static void print_event_desc(struct perf
> }
> fprintf(fp, " }");
> }
> - if (evsel->attr.use_clockid)
> - fprintf(fp, ", clockid = %d", evsel->attr.clockid);
>
> + perf_event_attr__fprintf(fp, &evsel->attr, __desc_attr__fprintf, NULL);
>
> fputc('\n', fp);
> }
>
>

2015-04-02 22:29:00

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing

Em Thu, Apr 02, 2015 at 10:12:50AM +0200, Ingo Molnar escreveu:
> * Peter Zijlstra <[email protected]> wrote:
> > tools/perf/util/print_helper.h | 7 +
> > 6 files changed, 170 insertions(+), 174 deletions(-)
>
> Acked-by: Ingo Molnar <[email protected]>

I'll get this in tomorrow, just finishing a pull req now,

- Arnaldo

2015-04-03 16:12:00

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing

Em Thu, Apr 02, 2015 at 01:59:34PM +0200, Peter Zijlstra escreveu:
> On Thu, Apr 02, 2015 at 12:01:53PM +0300, Adrian Hunter wrote:
> > But personally I think the "include" approach is too ugly. I would just
> > add a function instead. Something like:
>
> You've not stared at the kernel tracepoint code long enough ;-)
>
> Something like so then?

Ok, this one seems to have gotten acks from Ingo, Jiri and Adrian, but
doesn't apply because it needs that clock_id one first, searching the
situation of that part...

- Arnaldo

> ---
> Subject: perf, tools: Merge all perf_event_attr print functions
> From: Peter Zijlstra <[email protected]>
> Date: Thu Apr 2 11:49:23 CEST 2015
>
> Currently there's 3 (that I found) different and incomplete
> implementations of printing perf_event_attr.
>
> This is quite silly. Merge the lot.
>
> While this patch does not retain the exact form all printing that I
> found is debug output and thus it should not be critical.
>
> Also, I cannot find a single print_event_desc() caller.
>
> Pre:
>
> $ perf record -vv -e cycles -- sleep 1
> ------------------------------------------------------------
> perf_event_attr:
> type 0
> size 104
> config 0
> sample_period 4000
> sample_freq 4000
> sample_type 0x107
> read_format 0
> disabled 1 inherit 1
> pinned 0 exclusive 0
> exclude_user 0 exclude_kernel 0
> exclude_hv 0 exclude_idle 0
> mmap 1 comm 1
> mmap2 1 comm_exec 1
> freq 1 inherit_stat 0
> enable_on_exec 1 task 1
> watermark 0 precise_ip 0
> mmap_data 0 sample_id_all 1
> exclude_host 0 exclude_guest 1
> excl.callchain_kern 0 excl.callchain_user 0
> wakeup_events 0
> wakeup_watermark 0
> bp_type 0
> bp_addr 0
> config1 0
> bp_len 0
> config2 0
> branch_sample_type 0
> sample_regs_user 0
> sample_stack_user 0
> sample_regs_intr 0
> ------------------------------------------------------------
>
> $ perf evlist -vv
> cycles: sample_freq=4000, size: 104, sample_type: IP|TID|TIME|PERIOD,
> disabled: 1, inherit: 1, mmap: 1, mmap2: 1, comm: 1, comm_exec: 1,
> freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest:
> 1
>
> Post:
>
> $ ./perf record -vv -e cycles -- sleep 1
> ------------------------------------------------------------
> perf_event_attr:
> size 112
> { sample_period, sample_freq } 4000
> sample_type IP|TID|TIME|PERIOD
> disabled 1
> inherit 1
> mmap 1
> comm 1
> freq 1
> enable_on_exec 1
> task 1
> sample_id_all 1
> exclude_guest 1
> mmap2 1
> comm_exec 1
> ------------------------------------------------------------
>
> $ ./perf evlist -vv
> cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type:
> IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq:
> 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1,
> mmap2: 1, comm_exec: 1
>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> tools/perf/util/evsel.c | 284 ++++++++++++++++++++---------------------------
> tools/perf/util/evsel.h | 6
> tools/perf/util/header.c | 29 +---
> 3 files changed, 139 insertions(+), 180 deletions(-)
>
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1011,70 +1011,126 @@ static int get_group_fd(struct perf_evse
> return fd;
> }
>
> -#define __PRINT_ATTR(fmt, cast, field) \
> - fprintf(fp, " %-19s "fmt"\n", #field, cast attr->field)
> +struct bit_names {
> + int bit;
> + const char *name;
> +};
> +
> +static void __p_bits(char *buf, size_t size, u64 value, struct bit_names *bits)
> +{
> + bool first_bit = true;
> + int i = 0;
> +
> + do {
> + if (value & bits[i].bit) {
> + buf += scnprintf(buf, size, "%s%s", first_bit ? "" : "|", bits[i].name);
> + first_bit = false;
> + }
> + } while (bits[++i].name != NULL);
> +}
> +
> +static void __p_sample_type(char *buf, size_t size, u64 value)
> +{
> +#define bit_name(n) { PERF_SAMPLE_##n, #n }
> + struct bit_names bits[] = {
> + bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
> + bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
> + bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
> + bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
> + bit_name(IDENTIFIER), bit_name(REGS_INTR),
> + { .name = NULL, }
> + };
> +#undef bit_name
> + __p_bits(buf, size, value, bits);
> +}
>
> -#define PRINT_ATTR_U32(field) __PRINT_ATTR("%u" , , field)
> -#define PRINT_ATTR_X32(field) __PRINT_ATTR("%#x", , field)
> -#define PRINT_ATTR_U64(field) __PRINT_ATTR("%" PRIu64, (uint64_t), field)
> -#define PRINT_ATTR_X64(field) __PRINT_ATTR("%#"PRIx64, (uint64_t), field)
> -
> -#define PRINT_ATTR2N(name1, field1, name2, field2) \
> - fprintf(fp, " %-19s %u %-19s %u\n", \
> - name1, attr->field1, name2, attr->field2)
> -
> -#define PRINT_ATTR2(field1, field2) \
> - PRINT_ATTR2N(#field1, field1, #field2, field2)
> -
> -static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
> -{
> - size_t ret = 0;
> -
> - ret += fprintf(fp, "%.60s\n", graph_dotted_line);
> - ret += fprintf(fp, "perf_event_attr:\n");
> -
> - ret += PRINT_ATTR_U32(type);
> - ret += PRINT_ATTR_U32(size);
> - ret += PRINT_ATTR_X64(config);
> - ret += PRINT_ATTR_U64(sample_period);
> - ret += PRINT_ATTR_U64(sample_freq);
> - ret += PRINT_ATTR_X64(sample_type);
> - ret += PRINT_ATTR_X64(read_format);
> -
> - ret += PRINT_ATTR2(disabled, inherit);
> - ret += PRINT_ATTR2(pinned, exclusive);
> - ret += PRINT_ATTR2(exclude_user, exclude_kernel);
> - ret += PRINT_ATTR2(exclude_hv, exclude_idle);
> - ret += PRINT_ATTR2(mmap, comm);
> - ret += PRINT_ATTR2(freq, inherit_stat);
> - ret += PRINT_ATTR2(enable_on_exec, task);
> - ret += PRINT_ATTR2(watermark, precise_ip);
> - ret += PRINT_ATTR2(mmap_data, sample_id_all);
> - ret += PRINT_ATTR2(exclude_host, exclude_guest);
> - ret += PRINT_ATTR2N("excl.callchain_kern", exclude_callchain_kernel,
> - "excl.callchain_user", exclude_callchain_user);
> - ret += PRINT_ATTR2(mmap2, comm_exec);
> - ret += __PRINT_ATTR("%u",,use_clockid);
> -
> -
> - ret += PRINT_ATTR_U32(wakeup_events);
> - ret += PRINT_ATTR_U32(wakeup_watermark);
> - ret += PRINT_ATTR_X32(bp_type);
> - ret += PRINT_ATTR_X64(bp_addr);
> - ret += PRINT_ATTR_X64(config1);
> - ret += PRINT_ATTR_U64(bp_len);
> - ret += PRINT_ATTR_X64(config2);
> - ret += PRINT_ATTR_X64(branch_sample_type);
> - ret += PRINT_ATTR_X64(sample_regs_user);
> - ret += PRINT_ATTR_U32(sample_stack_user);
> - ret += PRINT_ATTR_U32(clockid);
> - ret += PRINT_ATTR_X64(sample_regs_intr);
> +static void __p_read_format(char *buf, size_t size, u64 value)
> +{
> +#define bit_name(n) { PERF_FORMAT_##n, #n }
> + struct bit_names bits[] = {
> + bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
> + bit_name(ID), bit_name(GROUP),
> + { .name = NULL, }
> + };
> +#undef bit_name
> + __p_bits(buf, size, value, bits);
> +}
> +
> +#define BUF_SIZE 1024
> +
> +#define p_hex(val) snprintf(buf, BUF_SIZE, "%"PRIx64, (uint64_t)(val))
> +#define p_unsigned(val) snprintf(buf, BUF_SIZE, "%"PRIu64, (uint64_t)(val))
> +#define p_signed(val) snprintf(buf, BUF_SIZE, "%"PRId64, (int64_t)(val))
> +#define p_sample_type(val) __p_sample_type(buf, BUF_SIZE, val)
> +#define p_read_format(val) __p_read_format(buf, BUF_SIZE, val)
> +
> +#define PRINT_ATTRn(_n, _f, _p) \
> +do { \
> + if (attr->_f) { \
> + _p(attr->_f); \
> + ret += attr__fprintf(fp, _n, buf, priv);\
> + } \
> +} while (0)
>
> - ret += fprintf(fp, "%.60s\n", graph_dotted_line);
> +#define PRINT_ATTRf(_f, _p) PRINT_ATTRn(#_f, _f, _p)
> +
> +int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
> + attr__fprintf_f attr__fprintf, void *priv)
> +{
> + char buf[BUF_SIZE];
> + int ret = 0;
> +
> + PRINT_ATTRf(type, p_unsigned);
> + PRINT_ATTRf(size, p_unsigned);
> + PRINT_ATTRf(config, p_hex);
> + PRINT_ATTRn("{ sample_period, sample_freq }", sample_period, p_unsigned);
> + PRINT_ATTRf(sample_type, p_sample_type);
> + PRINT_ATTRf(read_format, p_read_format);
> +
> + PRINT_ATTRf(disabled, p_unsigned);
> + PRINT_ATTRf(inherit, p_unsigned);
> + PRINT_ATTRf(pinned, p_unsigned);
> + PRINT_ATTRf(exclusive, p_unsigned);
> + PRINT_ATTRf(exclude_user, p_unsigned);
> + PRINT_ATTRf(exclude_kernel, p_unsigned);
> + PRINT_ATTRf(exclude_hv, p_unsigned);
> + PRINT_ATTRf(exclude_idle, p_unsigned);
> + PRINT_ATTRf(mmap, p_unsigned);
> + PRINT_ATTRf(comm, p_unsigned);
> + PRINT_ATTRf(freq, p_unsigned);
> + PRINT_ATTRf(inherit_stat, p_unsigned);
> + PRINT_ATTRf(enable_on_exec, p_unsigned);
> + PRINT_ATTRf(task, p_unsigned);
> + PRINT_ATTRf(watermark, p_unsigned);
> + PRINT_ATTRf(precise_ip, p_unsigned);
> + PRINT_ATTRf(mmap_data, p_unsigned);
> + PRINT_ATTRf(sample_id_all, p_unsigned);
> + PRINT_ATTRf(exclude_host, p_unsigned);
> + PRINT_ATTRf(exclude_guest, p_unsigned);
> + PRINT_ATTRf(exclude_callchain_kernel, p_unsigned);
> + PRINT_ATTRf(exclude_callchain_user, p_unsigned);
> + PRINT_ATTRf(mmap2, p_unsigned);
> + PRINT_ATTRf(comm_exec, p_unsigned);
> + PRINT_ATTRf(use_clockid, p_unsigned);
> +
> + PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
> + PRINT_ATTRf(bp_type, p_unsigned);
> + PRINT_ATTRn("{ bp_addr, config1 }", bp_addr, p_hex);
> + PRINT_ATTRn("{ bp_len, config2 }", bp_len, p_hex);
> + PRINT_ATTRf(sample_regs_user, p_hex);
> + PRINT_ATTRf(sample_stack_user, p_unsigned);
> + PRINT_ATTRf(clockid, p_signed);
> + PRINT_ATTRf(sample_regs_intr, p_hex);
>
> return ret;
> }
>
> +static int __open_attr__fprintf(FILE *fp, const char *name, const char *val,
> + void *priv __attribute__((unused)))
> +{
> + return fprintf(fp, " %-32s %s\n", name, val);
> +}
> +
> static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
> struct thread_map *threads)
> {
> @@ -1114,8 +1170,12 @@ static int __perf_evsel__open(struct per
> if (perf_missing_features.sample_id_all)
> evsel->attr.sample_id_all = 0;
>
> - if (verbose >= 2)
> - perf_event_attr__fprintf(&evsel->attr, stderr);
> + if (verbose >= 2) {
> + fprintf(stderr, "%.60s\n", graph_dotted_line);
> + fprintf(stderr, "perf_event_attr:\n");
> + perf_event_attr__fprintf(stderr, &evsel->attr, __open_attr__fprintf, NULL);
> + fprintf(stderr, "%.60s\n", graph_dotted_line);
> + }
>
> for (cpu = 0; cpu < cpus->nr; cpu++) {
>
> @@ -1996,62 +2056,9 @@ static int comma_fprintf(FILE *fp, bool
> return ret;
> }
>
> -static int __if_fprintf(FILE *fp, bool *first, const char *field, u64 value)
> +static int __print_attr__fprintf(FILE *fp, const char *name, const char *val, void *priv)
> {
> - if (value == 0)
> - return 0;
> -
> - return comma_fprintf(fp, first, " %s: %" PRIu64, field, value);
> -}
> -
> -#define if_print(field) printed += __if_fprintf(fp, &first, #field, evsel->attr.field)
> -
> -struct bit_names {
> - int bit;
> - const char *name;
> -};
> -
> -static int bits__fprintf(FILE *fp, const char *field, u64 value,
> - struct bit_names *bits, bool *first)
> -{
> - int i = 0, printed = comma_fprintf(fp, first, " %s: ", field);
> - bool first_bit = true;
> -
> - do {
> - if (value & bits[i].bit) {
> - printed += fprintf(fp, "%s%s", first_bit ? "" : "|", bits[i].name);
> - first_bit = false;
> - }
> - } while (bits[++i].name != NULL);
> -
> - return printed;
> -}
> -
> -static int sample_type__fprintf(FILE *fp, bool *first, u64 value)
> -{
> -#define bit_name(n) { PERF_SAMPLE_##n, #n }
> - struct bit_names bits[] = {
> - bit_name(IP), bit_name(TID), bit_name(TIME), bit_name(ADDR),
> - bit_name(READ), bit_name(CALLCHAIN), bit_name(ID), bit_name(CPU),
> - bit_name(PERIOD), bit_name(STREAM_ID), bit_name(RAW),
> - bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
> - bit_name(IDENTIFIER), bit_name(REGS_INTR),
> - { .name = NULL, }
> - };
> -#undef bit_name
> - return bits__fprintf(fp, "sample_type", value, bits, first);
> -}
> -
> -static int read_format__fprintf(FILE *fp, bool *first, u64 value)
> -{
> -#define bit_name(n) { PERF_FORMAT_##n, #n }
> - struct bit_names bits[] = {
> - bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
> - bit_name(ID), bit_name(GROUP),
> - { .name = NULL, }
> - };
> -#undef bit_name
> - return bits__fprintf(fp, "read_format", value, bits, first);
> + return comma_fprintf(fp, (bool *)priv, " %s: %s", name, val);
> }
>
> int perf_evsel__fprintf(struct perf_evsel *evsel,
> @@ -2080,52 +2087,13 @@ int perf_evsel__fprintf(struct perf_evse
>
> printed += fprintf(fp, "%s", perf_evsel__name(evsel));
>
> - if (details->verbose || details->freq) {
> + if (details->verbose) {
> + printed += perf_event_attr__fprintf(fp, &evsel->attr,
> + __print_attr__fprintf, &first);
> + } else if (details->freq) {
> printed += comma_fprintf(fp, &first, " sample_freq=%" PRIu64,
> (u64)evsel->attr.sample_freq);
> }
> -
> - if (details->verbose) {
> - if_print(type);
> - if_print(config);
> - if_print(config1);
> - if_print(config2);
> - if_print(size);
> - printed += sample_type__fprintf(fp, &first, evsel->attr.sample_type);
> - if (evsel->attr.read_format)
> - printed += read_format__fprintf(fp, &first, evsel->attr.read_format);
> - if_print(disabled);
> - if_print(inherit);
> - if_print(pinned);
> - if_print(exclusive);
> - if_print(exclude_user);
> - if_print(exclude_kernel);
> - if_print(exclude_hv);
> - if_print(exclude_idle);
> - if_print(mmap);
> - if_print(comm);
> - if_print(freq);
> - if_print(inherit_stat);
> - if_print(enable_on_exec);
> - if_print(task);
> - if_print(watermark);
> - if_print(precise_ip);
> - if_print(mmap_data);
> - if_print(sample_id_all);
> - if_print(exclude_host);
> - if_print(exclude_guest);
> - if_print(mmap2);
> - if_print(comm_exec);
> - if_print(use_clockid);
> - if_print(__reserved_1);
> - if_print(wakeup_events);
> - if_print(bp_type);
> - if_print(branch_sample_type);
> - if_print(sample_regs_user);
> - if_print(sample_stack_user);
> - if_print(clockid);
> - if_print(sample_regs_intr);
> - }
> out:
> fputc('\n', fp);
> return ++printed;
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -359,4 +359,10 @@ static inline bool has_branch_callstack(
> {
> return evsel->attr.branch_sample_type & PERF_SAMPLE_BRANCH_CALL_STACK;
> }
> +
> +typedef int (*attr__fprintf_f)(FILE *, const char *, const char *, void *);
> +
> +int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
> + attr__fprintf_f attr__fprintf, void *priv);
> +
> #endif /* __PERF_EVSEL_H */
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1055,6 +1055,12 @@ read_event_desc(struct perf_header *ph,
> goto out;
> }
>
> +static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,
> + void *priv __attribute__((unused)))
> +{
> + return fprintf(fp, ", %s = %s", name, val);
> +}
> +
> static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
> {
> struct perf_evsel *evsel, *events = read_event_desc(ph, fd);
> @@ -1069,26 +1075,6 @@ static void print_event_desc(struct perf
> for (evsel = events; evsel->attr.size; evsel++) {
> fprintf(fp, "# event : name = %s, ", evsel->name);
>
> - fprintf(fp, "type = %d, config = 0x%"PRIx64
> - ", config1 = 0x%"PRIx64", config2 = 0x%"PRIx64,
> - evsel->attr.type,
> - (u64)evsel->attr.config,
> - (u64)evsel->attr.config1,
> - (u64)evsel->attr.config2);
> -
> - fprintf(fp, ", excl_usr = %d, excl_kern = %d",
> - evsel->attr.exclude_user,
> - evsel->attr.exclude_kernel);
> -
> - fprintf(fp, ", excl_host = %d, excl_guest = %d",
> - evsel->attr.exclude_host,
> - evsel->attr.exclude_guest);
> -
> - fprintf(fp, ", precise_ip = %d", evsel->attr.precise_ip);
> -
> - fprintf(fp, ", attr_mmap2 = %d", evsel->attr.mmap2);
> - fprintf(fp, ", attr_mmap = %d", evsel->attr.mmap);
> - fprintf(fp, ", attr_mmap_data = %d", evsel->attr.mmap_data);
> if (evsel->ids) {
> fprintf(fp, ", id = {");
> for (j = 0, id = evsel->id; j < evsel->ids; j++, id++) {
> @@ -1098,9 +1084,8 @@ static void print_event_desc(struct perf
> }
> fprintf(fp, " }");
> }
> - if (evsel->attr.use_clockid)
> - fprintf(fp, ", clockid = %d", evsel->attr.clockid);
>
> + perf_event_attr__fprintf(fp, &evsel->attr, __desc_attr__fprintf, NULL);
>
> fputc('\n', fp);
> }

2015-04-03 16:14:40

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC][PATCH] perf tools: unify perf_event_attr printing

Em Fri, Apr 03, 2015 at 01:11:47PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Apr 02, 2015 at 01:59:34PM +0200, Peter Zijlstra escreveu:
> > On Thu, Apr 02, 2015 at 12:01:53PM +0300, Adrian Hunter wrote:
> > > But personally I think the "include" approach is too ugly. I would just
> > > add a function instead. Something like:
> >
> > You've not stared at the kernel tracepoint code long enough ;-)
> >
> > Something like so then?
>
> Ok, this one seems to have gotten acks from Ingo, Jiri and Adrian, but
> doesn't apply because it needs that clock_id one first, searching the
> situation of that part...

Hey Peter, would you be so kind and resubmit this (at least) two part
series? I.e. the first one adding the clockid and the second, with the
consolidation of the perf_event_attr printing?

- Arnaldo