All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing
@ 2015-04-07  9:09 Peter Zijlstra
  2015-04-07  9:09 ` [PATCH 1/2] perf, record: Add clockid parameter Peter Zijlstra
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Peter Zijlstra @ 2015-04-07  9:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Adrian Hunter, Jiri Olsa, David Ahern, Stephane Eranian,
	Thomas Gleixner, Linus Torvalds, LKML, John Stultz,
	H. Peter Anvin, Andrew Morton, Ingo Molnar, Peter Zijlstra

Hi Arnaldo,

As per your request, here's the latest version of the two patches.

I retained Jiri's and Ingo's ACK on the perf_event_attr printing patch
even though I rewrote it since they acked it; if they feel I wrongly did
so please holler.

Thanks!


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] perf, record: Add clockid parameter
  2015-04-07  9:09 [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing Peter Zijlstra
@ 2015-04-07  9:09 ` Peter Zijlstra
  2015-04-07 15:48   ` Peter Zijlstra
  2015-04-07  9:09 ` [PATCH 2/2] perf, tools: Merge all perf_event_attr print functions Peter Zijlstra
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2015-04-07  9:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Adrian Hunter, Jiri Olsa, David Ahern, Stephane Eranian,
	Thomas Gleixner, Linus Torvalds, LKML, John Stultz,
	H. Peter Anvin, Andrew Morton, Ingo Molnar, Peter Zijlstra

[-- Attachment #1: peter_zijlstra-perfrecord-add_clockid_parameter.patch --]
[-- Type: text/plain, Size: 9034 bytes --]

Teach perf-record about the new perf_event_attr::{use_clockid, clockid}
fields. Add a simple parameter to set the clock (if any) to be used for
the events to be recorded into the data file.

Since we store the entire perf_event_attr in the EVENT_DESC section we
also already store the used clockid in the data file.

Cc: eranian@google.com
Cc: dsahern@gmail.com
Cc: acme@redhat.com
Cc: jolsa@redhat.com
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/perf/Documentation/perf-record.txt |    7 ++
 tools/perf/builtin-record.c              |   78 +++++++++++++++++++++++++++++++
 tools/perf/perf.h                        |    1 
 tools/perf/util/evsel.c                  |   59 +++++++++++++++++++++--
 tools/perf/util/header.c                 |    3 +
 5 files changed, 144 insertions(+), 4 deletions(-)

--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -250,6 +250,13 @@ is off by default.
 --running-time::
 Record running and enabled time for read events (:S)
 
+-k::
+--clockid::
+Sets the clock id to use for the various time fields in the perf_event_type
+records. See clock_gettime(). In particular CLOCK_MONOTONIC and
+CLOCK_MONOTONIC_RAW are supported, some events might also allow
+CLOCK_BOOTTIME, CLOCK_REALTIME and CLOCK_TAI.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -711,6 +711,80 @@ static int perf_record_config(const char
 	return perf_default_config(var, value, cb);
 }
 
+struct clockid_map {
+	const char *name;
+	int clockid;
+};
+
+#define CLOCKID_MAP(n, c)	\
+	{ .name = n, .clockid = (c), }
+
+#define CLOCKID_END	{ .name = NULL, }
+
+/*
+ * Doesn't appear to have made it into userspace so define here if missing.
+ */
+#ifndef CLOCK_TAI
+#define CLOCK_TAI 11
+#endif
+
+static const struct clockid_map clockids[] = {
+	/* available for all events, NMI safe */
+	CLOCKID_MAP("monotonic", CLOCK_MONOTONIC),
+	CLOCKID_MAP("monotonic_raw", CLOCK_MONOTONIC_RAW),
+
+	/* available for some events */
+	CLOCKID_MAP("realtime", CLOCK_REALTIME),
+	CLOCKID_MAP("boottime", CLOCK_BOOTTIME),
+	CLOCKID_MAP("tai", CLOCK_TAI),
+
+	/* available for the lazy */
+	CLOCKID_MAP("mono", CLOCK_MONOTONIC),
+	CLOCKID_MAP("raw", CLOCK_MONOTONIC_RAW),
+	CLOCKID_MAP("real", CLOCK_REALTIME),
+	CLOCKID_MAP("boot", CLOCK_BOOTTIME),
+
+	CLOCKID_END,
+};
+
+static int parse_clockid(const struct option *opt, const char *str, int unset)
+{
+	clockid_t *clk = (clockid_t *)opt->value;
+	const struct clockid_map *cm;
+	const char *ostr = str;
+
+	if (unset) {
+		*clk = -1;
+		return 0;
+	}
+
+	/* no arg passed */
+	if (!str)
+		return 0;
+
+	/* no setting it twice */
+	if (*clk != -1)
+		return -1;
+
+	/* if its a number, we're done */
+	if (sscanf(str, "%d", clk) == 1)
+		return 0;
+
+	/* allow a "CLOCK_" prefix to the name */
+	if (!strncasecmp(str, "CLOCK_", 6))
+		str += 6;
+
+	for (cm = clockids; cm->name; cm++) {
+		if (!strcasecmp(str, cm->name)) {
+			*clk = cm->clockid;
+			return 0;
+		}
+	}
+
+	ui__warning("unknown clockid %s, check man page\n", ostr);
+	return -1;
+}
+
 static const char * const __record_usage[] = {
 	"perf record [<options>] [<command>]",
 	"perf record [<options>] -- <command> [<options>]",
@@ -739,6 +813,7 @@ static struct record record = {
 			.uses_mmap   = true,
 			.default_per_cpu = true,
 		},
+		.clockid             = -1,
 	},
 	.tool = {
 		.sample		= process_sample_event,
@@ -842,6 +917,9 @@ struct option __record_options[] = {
 		    "Sample machine registers on interrupt"),
 	OPT_BOOLEAN(0, "running-time", &record.opts.running_time,
 		    "Record running/enabled time of read (:S) events"),
+	OPT_CALLBACK('k', "clockid", &record.opts.clockid,
+	"clockid", "clockid to use for events, see clock_gettime()",
+	parse_clockid),
 	OPT_END()
 };
 
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -62,6 +62,7 @@ struct record_opts {
 	u64	     user_interval;
 	bool	     sample_transaction;
 	unsigned     initial_delay;
+	clockid_t    clockid;
 };
 
 struct option;
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -32,8 +32,12 @@ static struct {
 	bool exclude_guest;
 	bool mmap2;
 	bool cloexec;
+	bool clockid;
+	bool clockid_wrong;
 } perf_missing_features;
 
+static clockid_t clockid;
+
 static int perf_evsel__no_extra_init(struct perf_evsel *evsel __maybe_unused)
 {
 	return 0;
@@ -761,6 +765,12 @@ void perf_evsel__config(struct perf_evse
 		attr->disabled = 0;
 		attr->enable_on_exec = 0;
 	}
+
+	clockid = opts->clockid;
+	if (opts->clockid >= 0) {
+		attr->use_clockid = 1;
+		attr->clockid = opts->clockid;
+	}
 }
 
 static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
@@ -1036,7 +1046,6 @@ static size_t perf_event_attr__fprintf(s
 	ret += PRINT_ATTR2(exclude_user, exclude_kernel);
 	ret += PRINT_ATTR2(exclude_hv, exclude_idle);
 	ret += PRINT_ATTR2(mmap, comm);
-	ret += PRINT_ATTR2(mmap2, comm_exec);
 	ret += PRINT_ATTR2(freq, inherit_stat);
 	ret += PRINT_ATTR2(enable_on_exec, task);
 	ret += PRINT_ATTR2(watermark, precise_ip);
@@ -1044,6 +1053,9 @@ static size_t perf_event_attr__fprintf(s
 	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);
@@ -1055,6 +1067,7 @@ static size_t perf_event_attr__fprintf(s
 	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);
 
 	ret += fprintf(fp, "%.60s\n", graph_dotted_line);
@@ -1085,6 +1098,12 @@ static int __perf_evsel__open(struct per
 	}
 
 fallback_missing_features:
+	if (perf_missing_features.clockid_wrong)
+		evsel->attr.clockid = CLOCK_MONOTONIC; /* should always work */
+	if (perf_missing_features.clockid) {
+		evsel->attr.use_clockid = 0;
+		evsel->attr.clockid = 0;
+	}
 	if (perf_missing_features.cloexec)
 		flags &= ~(unsigned long)PERF_FLAG_FD_CLOEXEC;
 	if (perf_missing_features.mmap2)
@@ -1122,6 +1141,17 @@ static int __perf_evsel__open(struct per
 				goto try_fallback;
 			}
 			set_rlimit = NO_CHANGE;
+
+			/*
+			 * If we succeeded but had to kill clockid, fail and
+			 * have perf_evsel__open_strerror() print us a nice
+			 * error.
+			 */
+			if (perf_missing_features.clockid ||
+			    perf_missing_features.clockid_wrong) {
+				err = -EINVAL;
+				goto out_close;
+			}
 		}
 	}
 
@@ -1155,7 +1185,17 @@ static int __perf_evsel__open(struct per
 	if (err != -EINVAL || cpu > 0 || thread > 0)
 		goto out_close;
 
-	if (!perf_missing_features.cloexec && (flags & PERF_FLAG_FD_CLOEXEC)) {
+	/*
+	 * Must probe features in the order they were added to the
+	 * perf_event_attr interface.
+	 */
+	if (!perf_missing_features.clockid_wrong && evsel->attr.use_clockid) {
+		perf_missing_features.clockid_wrong = true;
+		goto fallback_missing_features;
+	} else if (!perf_missing_features.clockid && evsel->attr.use_clockid) {
+		perf_missing_features.clockid = true;
+		goto fallback_missing_features;
+	} else if (!perf_missing_features.cloexec && (flags & PERF_FLAG_FD_CLOEXEC)) {
 		perf_missing_features.cloexec = true;
 		goto fallback_missing_features;
 	} else if (!perf_missing_features.mmap2 && evsel->attr.mmap2) {
@@ -2063,9 +2103,7 @@ int perf_evsel__fprintf(struct perf_evse
 		if_print(exclude_hv);
 		if_print(exclude_idle);
 		if_print(mmap);
-		if_print(mmap2);
 		if_print(comm);
-		if_print(comm_exec);
 		if_print(freq);
 		if_print(inherit_stat);
 		if_print(enable_on_exec);
@@ -2076,10 +2114,17 @@ int perf_evsel__fprintf(struct perf_evse
 		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);
@@ -2158,6 +2203,12 @@ int perf_evsel__open_strerror(struct per
 	"The PMU counters are busy/taken by another profiler.\n"
 	"We found oprofile daemon running, please stop it and try again.");
 		break;
+	case EINVAL:
+		if (perf_missing_features.clockid)
+			return scnprintf(msg, size, "clockid feature not supported.");
+		if (perf_missing_features.clockid_wrong)
+			return scnprintf(msg, size, "wrong clockid (%d).", clockid);
+		break;
 	default:
 		break;
 	}
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1098,6 +1098,9 @@ static void print_event_desc(struct perf
 			}
 			fprintf(fp, " }");
 		}
+		if (evsel->attr.use_clockid)
+			fprintf(fp, ", clockid = %d", evsel->attr.clockid);
+
 
 		fputc('\n', fp);
 	}



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] perf, tools: Merge all perf_event_attr print functions
  2015-04-07  9:09 [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing Peter Zijlstra
  2015-04-07  9:09 ` [PATCH 1/2] perf, record: Add clockid parameter Peter Zijlstra
@ 2015-04-07  9:09 ` Peter Zijlstra
  2015-04-07 13:32   ` Arnaldo Carvalho de Melo
  2015-04-08 15:14   ` [tip:perf/core] perf " tip-bot for Peter Zijlstra
  2015-04-07  9:23 ` [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing Ingo Molnar
  2015-04-07 12:08 ` Arnaldo Carvalho de Melo
  3 siblings, 2 replies; 12+ messages in thread
From: Peter Zijlstra @ 2015-04-07  9:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Adrian Hunter, Jiri Olsa, David Ahern, Stephane Eranian,
	Thomas Gleixner, Linus Torvalds, LKML, John Stultz,
	H. Peter Anvin, Andrew Morton, Ingo Molnar, Peter Zijlstra

[-- Attachment #1: peterz-perf-tools-print-attr2.patch --]
[-- Type: text/plain, Size: 15664 bytes --]

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: acme@redhat.com
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 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);
 	}



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing
  2015-04-07  9:09 [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing Peter Zijlstra
  2015-04-07  9:09 ` [PATCH 1/2] perf, record: Add clockid parameter Peter Zijlstra
  2015-04-07  9:09 ` [PATCH 2/2] perf, tools: Merge all perf_event_attr print functions Peter Zijlstra
@ 2015-04-07  9:23 ` Ingo Molnar
  2015-04-07 12:08 ` Arnaldo Carvalho de Melo
  3 siblings, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2015-04-07  9:23 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, Adrian Hunter, Jiri Olsa, David Ahern,
	Stephane Eranian, Thomas Gleixner, Linus Torvalds, LKML,
	John Stultz, H. Peter Anvin, Andrew Morton


* Peter Zijlstra <peterz@infradead.org> wrote:

> Hi Arnaldo,
> 
> As per your request, here's the latest version of the two patches.
> 
> I retained Jiri's and Ingo's ACK on the perf_event_attr printing patch
> even though I rewrote it since they acked it; if they feel I wrongly did
> so please holler.

It's only getting better so my ack stands! :-)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing
  2015-04-07  9:09 [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing Peter Zijlstra
                   ` (2 preceding siblings ...)
  2015-04-07  9:23 ` [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing Ingo Molnar
@ 2015-04-07 12:08 ` Arnaldo Carvalho de Melo
  2015-04-07 12:25   ` Jiri Olsa
  3 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-04-07 12:08 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Adrian Hunter, Jiri Olsa, David Ahern, Stephane Eranian,
	Thomas Gleixner, Linus Torvalds, LKML, John Stultz,
	H. Peter Anvin, Andrew Morton, Ingo Molnar

Em Tue, Apr 07, 2015 at 11:09:52AM +0200, Peter Zijlstra escreveu:
> Hi Arnaldo,
> 
> As per your request, here's the latest version of the two patches.
> 
> I retained Jiri's and Ingo's ACK on the perf_event_attr printing patch
> even though I rewrote it since they acked it; if they feel I wrongly did
> so please holler.

Ingo re-acked it, I'll keep Jiri's but will wait for him to renew it
before pushing it to Ingo,

Thanks,

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing
  2015-04-07 12:08 ` Arnaldo Carvalho de Melo
@ 2015-04-07 12:25   ` Jiri Olsa
  0 siblings, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2015-04-07 12:25 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Adrian Hunter, David Ahern, Stephane Eranian,
	Thomas Gleixner, Linus Torvalds, LKML, John Stultz,
	H. Peter Anvin, Andrew Morton, Ingo Molnar

On Tue, Apr 07, 2015 at 09:08:16AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Apr 07, 2015 at 11:09:52AM +0200, Peter Zijlstra escreveu:
> > Hi Arnaldo,
> > 
> > As per your request, here's the latest version of the two patches.
> > 
> > I retained Jiri's and Ingo's ACK on the perf_event_attr printing patch
> > even though I rewrote it since they acked it; if they feel I wrongly did
> > so please holler.
> 
> Ingo re-acked it, I'll keep Jiri's but will wait for him to renew it
> before pushing it to Ingo,

booth look ok to me.. ack ;-)

jirka

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] perf, tools: Merge all perf_event_attr print functions
  2015-04-07  9:09 ` [PATCH 2/2] perf, tools: Merge all perf_event_attr print functions Peter Zijlstra
@ 2015-04-07 13:32   ` Arnaldo Carvalho de Melo
  2015-04-08 15:14   ` [tip:perf/core] perf " tip-bot for Peter Zijlstra
  1 sibling, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-04-07 13:32 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Adrian Hunter, Jiri Olsa, David Ahern, Stephane Eranian,
	Thomas Gleixner, Linus Torvalds, LKML, John Stultz,
	H. Peter Anvin, Andrew Morton, Ingo Molnar

Em Tue, Apr 07, 2015 at 11:09:54AM +0200, Peter Zijlstra escreveu:
> 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
> ------------------------------------------------------------

If you do the above, i.e. start a line with ----, then the following
lines will go down the drain, fixing by prepending a whitespace :-)

- Arnaldo

> 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: acme@redhat.com
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> Acked-by: Ingo Molnar <mingo@kernel.org>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  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);
>  	}
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] perf, record: Add clockid parameter
  2015-04-07  9:09 ` [PATCH 1/2] perf, record: Add clockid parameter Peter Zijlstra
@ 2015-04-07 15:48   ` Peter Zijlstra
  2015-04-07 17:33     ` Jiri Olsa
  2015-04-08 15:14     ` [tip:perf/core] perf " tip-bot for Peter Zijlstra
  0 siblings, 2 replies; 12+ messages in thread
From: Peter Zijlstra @ 2015-04-07 15:48 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Adrian Hunter, Jiri Olsa, David Ahern, Stephane Eranian,
	Thomas Gleixner, Linus Torvalds, LKML, John Stultz,
	H. Peter Anvin, Andrew Morton, Ingo Molnar

On Tue, Apr 07, 2015 at 11:09:53AM +0200, Peter Zijlstra wrote:
> @@ -739,6 +813,7 @@ static struct record record = {
>  			.uses_mmap   = true,
>  			.default_per_cpu = true,
>  		},
> +		.clockid             = -1,
>  	},

As spotted by dahern this is not working well for all other record_opts
instances, fix that by adding a use_clockid field just like
perf_event_attr has.

perf test does (now) not appear to fail more with this patch than without.

---
Subject: perf, record: Add clockid parameter
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue, 31 Mar 2015 00:19:31 +0200

Teach perf-record about the new perf_event_attr::{use_clockid, clockid}
fields. Add a simple parameter to set the clock (if any) to be used for
the events to be recorded into the data file.

Since we store the entire perf_event_attr in the EVENT_DESC section we
also already store the used clockid in the data file.

Cc: eranian@google.com
Cc: dsahern@gmail.com
Cc: acme@redhat.com
Cc: jolsa@redhat.com
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/perf/Documentation/perf-record.txt |    7 ++
 tools/perf/builtin-record.c              |   80 +++++++++++++++++++++++++++++++
 tools/perf/perf.h                        |    2 
 tools/perf/util/evsel.c                  |   59 +++++++++++++++++++++-
 tools/perf/util/header.c                 |    3 +
 5 files changed, 147 insertions(+), 4 deletions(-)

--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -250,6 +250,13 @@ is off by default.
 --running-time::
 Record running and enabled time for read events (:S)
 
+-k::
+--clockid::
+Sets the clock id to use for the various time fields in the perf_event_type
+records. See clock_gettime(). In particular CLOCK_MONOTONIC and
+CLOCK_MONOTONIC_RAW are supported, some events might also allow
+CLOCK_BOOTTIME, CLOCK_REALTIME and CLOCK_TAI.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -711,6 +711,83 @@ static int perf_record_config(const char
 	return perf_default_config(var, value, cb);
 }
 
+struct clockid_map {
+	const char *name;
+	int clockid;
+};
+
+#define CLOCKID_MAP(n, c)	\
+	{ .name = n, .clockid = (c), }
+
+#define CLOCKID_END	{ .name = NULL, }
+
+/*
+ * Doesn't appear to have made it into userspace so define here if missing.
+ */
+#ifndef CLOCK_TAI
+#define CLOCK_TAI 11
+#endif
+
+static const struct clockid_map clockids[] = {
+	/* available for all events, NMI safe */
+	CLOCKID_MAP("monotonic", CLOCK_MONOTONIC),
+	CLOCKID_MAP("monotonic_raw", CLOCK_MONOTONIC_RAW),
+
+	/* available for some events */
+	CLOCKID_MAP("realtime", CLOCK_REALTIME),
+	CLOCKID_MAP("boottime", CLOCK_BOOTTIME),
+	CLOCKID_MAP("tai", CLOCK_TAI),
+
+	/* available for the lazy */
+	CLOCKID_MAP("mono", CLOCK_MONOTONIC),
+	CLOCKID_MAP("raw", CLOCK_MONOTONIC_RAW),
+	CLOCKID_MAP("real", CLOCK_REALTIME),
+	CLOCKID_MAP("boot", CLOCK_BOOTTIME),
+
+	CLOCKID_END,
+};
+
+static int parse_clockid(const struct option *opt, const char *str, int unset)
+{
+	struct record_opts *opts = (struct record_opts *)opt->value;
+	const struct clockid_map *cm;
+	const char *ostr = str;
+
+	if (unset) {
+		opts->use_clockid = 0;
+		return 0;
+	}
+
+	/* no arg passed */
+	if (!str)
+		return 0;
+
+	/* no setting it twice */
+	if (opts->use_clockid)
+		return -1;
+
+	opts->use_clockid = true;
+
+	/* if its a number, we're done */
+	if (sscanf(str, "%d", &opts->clockid) == 1)
+		return 0;
+
+	/* allow a "CLOCK_" prefix to the name */
+	if (!strncasecmp(str, "CLOCK_", 6))
+		str += 6;
+
+	for (cm = clockids; cm->name; cm++) {
+		if (!strcasecmp(str, cm->name)) {
+			opts->clockid = cm->clockid;
+			return 0;
+		}
+	}
+
+	opts->use_clockid = false;
+	ui__warning("unknown clockid %s, check man page\n", ostr);
+	return -1;
+}
+
 static const char * const __record_usage[] = {
 	"perf record [<options>] [<command>]",
 	"perf record [<options>] -- <command> [<options>]",
@@ -842,6 +919,9 @@ struct option __record_options[] = {
 		    "Sample machine registers on interrupt"),
 	OPT_BOOLEAN(0, "running-time", &record.opts.running_time,
 		    "Record running/enabled time of read (:S) events"),
+	OPT_CALLBACK('k', "clockid", &record.opts,
+	"clockid", "clockid to use for events, see clock_gettime()",
+	parse_clockid),
 	OPT_END()
 };
 
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -62,6 +62,8 @@ struct record_opts {
 	u64	     user_interval;
 	bool	     sample_transaction;
 	unsigned     initial_delay;
+	bool         use_clockid;
+	clockid_t    clockid;
 };
 
 struct option;
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -32,8 +32,12 @@ static struct {
 	bool exclude_guest;
 	bool mmap2;
 	bool cloexec;
+	bool clockid;
+	bool clockid_wrong;
 } perf_missing_features;
 
+static clockid_t clockid;
+
 static int perf_evsel__no_extra_init(struct perf_evsel *evsel __maybe_unused)
 {
 	return 0;
@@ -761,6 +765,12 @@ void perf_evsel__config(struct perf_evse
 		attr->disabled = 0;
 		attr->enable_on_exec = 0;
 	}
+
+	clockid = opts->clockid;
+	if (opts->use_clockid) {
+		attr->use_clockid = 1;
+		attr->clockid = opts->clockid;
+	}
 }
 
 static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
@@ -1036,7 +1046,6 @@ static size_t perf_event_attr__fprintf(s
 	ret += PRINT_ATTR2(exclude_user, exclude_kernel);
 	ret += PRINT_ATTR2(exclude_hv, exclude_idle);
 	ret += PRINT_ATTR2(mmap, comm);
-	ret += PRINT_ATTR2(mmap2, comm_exec);
 	ret += PRINT_ATTR2(freq, inherit_stat);
 	ret += PRINT_ATTR2(enable_on_exec, task);
 	ret += PRINT_ATTR2(watermark, precise_ip);
@@ -1044,6 +1053,9 @@ static size_t perf_event_attr__fprintf(s
 	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);
@@ -1055,6 +1067,7 @@ static size_t perf_event_attr__fprintf(s
 	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);
 
 	ret += fprintf(fp, "%.60s\n", graph_dotted_line);
@@ -1085,6 +1098,12 @@ static int __perf_evsel__open(struct per
 	}
 
 fallback_missing_features:
+	if (perf_missing_features.clockid_wrong)
+		evsel->attr.clockid = CLOCK_MONOTONIC; /* should always work */
+	if (perf_missing_features.clockid) {
+		evsel->attr.use_clockid = 0;
+		evsel->attr.clockid = 0;
+	}
 	if (perf_missing_features.cloexec)
 		flags &= ~(unsigned long)PERF_FLAG_FD_CLOEXEC;
 	if (perf_missing_features.mmap2)
@@ -1122,6 +1141,17 @@ static int __perf_evsel__open(struct per
 				goto try_fallback;
 			}
 			set_rlimit = NO_CHANGE;
+
+			/*
+			 * If we succeeded but had to kill clockid, fail and
+			 * have perf_evsel__open_strerror() print us a nice
+			 * error.
+			 */
+			if (perf_missing_features.clockid ||
+			    perf_missing_features.clockid_wrong) {
+				err = -EINVAL;
+				goto out_close;
+			}
 		}
 	}
 
@@ -1155,7 +1185,17 @@ static int __perf_evsel__open(struct per
 	if (err != -EINVAL || cpu > 0 || thread > 0)
 		goto out_close;
 
-	if (!perf_missing_features.cloexec && (flags & PERF_FLAG_FD_CLOEXEC)) {
+	/*
+	 * Must probe features in the order they were added to the
+	 * perf_event_attr interface.
+	 */
+	if (!perf_missing_features.clockid_wrong && evsel->attr.use_clockid) {
+		perf_missing_features.clockid_wrong = true;
+		goto fallback_missing_features;
+	} else if (!perf_missing_features.clockid && evsel->attr.use_clockid) {
+		perf_missing_features.clockid = true;
+		goto fallback_missing_features;
+	} else if (!perf_missing_features.cloexec && (flags & PERF_FLAG_FD_CLOEXEC)) {
 		perf_missing_features.cloexec = true;
 		goto fallback_missing_features;
 	} else if (!perf_missing_features.mmap2 && evsel->attr.mmap2) {
@@ -2063,9 +2103,7 @@ int perf_evsel__fprintf(struct perf_evse
 		if_print(exclude_hv);
 		if_print(exclude_idle);
 		if_print(mmap);
-		if_print(mmap2);
 		if_print(comm);
-		if_print(comm_exec);
 		if_print(freq);
 		if_print(inherit_stat);
 		if_print(enable_on_exec);
@@ -2076,10 +2114,17 @@ int perf_evsel__fprintf(struct perf_evse
 		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);
@@ -2158,6 +2203,12 @@ int perf_evsel__open_strerror(struct per
 	"The PMU counters are busy/taken by another profiler.\n"
 	"We found oprofile daemon running, please stop it and try again.");
 		break;
+	case EINVAL:
+		if (perf_missing_features.clockid)
+			return scnprintf(msg, size, "clockid feature not supported.");
+		if (perf_missing_features.clockid_wrong)
+			return scnprintf(msg, size, "wrong clockid (%d).", clockid);
+		break;
 	default:
 		break;
 	}
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1098,6 +1098,9 @@ static void print_event_desc(struct perf
 			}
 			fprintf(fp, " }");
 		}
+		if (evsel->attr.use_clockid)
+			fprintf(fp, ", clockid = %d", evsel->attr.clockid);
+
 
 		fputc('\n', fp);
 	}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] perf, record: Add clockid parameter
  2015-04-07 15:48   ` Peter Zijlstra
@ 2015-04-07 17:33     ` Jiri Olsa
  2015-04-08  9:47       ` Peter Zijlstra
  2015-04-08 15:14     ` [tip:perf/core] perf " tip-bot for Peter Zijlstra
  1 sibling, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2015-04-07 17:33 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, Adrian Hunter, David Ahern,
	Stephane Eranian, Thomas Gleixner, Linus Torvalds, LKML,
	John Stultz, H. Peter Anvin, Andrew Morton, Ingo Molnar

On Tue, Apr 07, 2015 at 05:48:51PM +0200, Peter Zijlstra wrote:
> On Tue, Apr 07, 2015 at 11:09:53AM +0200, Peter Zijlstra wrote:
> > @@ -739,6 +813,7 @@ static struct record record = {
> >  			.uses_mmap   = true,
> >  			.default_per_cpu = true,
> >  		},
> > +		.clockid             = -1,
> >  	},
> 
> As spotted by dahern this is not working well for all other record_opts
> instances, fix that by adding a use_clockid field just like
> perf_event_attr has.

hum, havent realized that.. I saw Arnaldo already included
this one in pull req., anyway FWIW it looks ok ;-)

jirka

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] perf, record: Add clockid parameter
  2015-04-07 17:33     ` Jiri Olsa
@ 2015-04-08  9:47       ` Peter Zijlstra
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2015-04-08  9:47 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Adrian Hunter, David Ahern,
	Stephane Eranian, Thomas Gleixner, Linus Torvalds, LKML,
	John Stultz, H. Peter Anvin, Andrew Morton, Ingo Molnar

On Tue, Apr 07, 2015 at 07:33:37PM +0200, Jiri Olsa wrote:
> On Tue, Apr 07, 2015 at 05:48:51PM +0200, Peter Zijlstra wrote:
> > On Tue, Apr 07, 2015 at 11:09:53AM +0200, Peter Zijlstra wrote:
> > > @@ -739,6 +813,7 @@ static struct record record = {
> > >  			.uses_mmap   = true,
> > >  			.default_per_cpu = true,
> > >  		},
> > > +		.clockid             = -1,
> > >  	},
> > 
> > As spotted by dahern this is not working well for all other record_opts
> > instances, fix that by adding a use_clockid field just like
> > perf_event_attr has.
> 
> hum, havent realized that.. I saw Arnaldo already included
> this one in pull req., anyway FWIW it looks ok ;-)

Yeah, I was slightly surprised to find out record_opts were used outside
of builtin-record.c, maybe a rename might be in order.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf record: Add clockid parameter
  2015-04-07 15:48   ` Peter Zijlstra
  2015-04-07 17:33     ` Jiri Olsa
@ 2015-04-08 15:14     ` tip-bot for Peter Zijlstra
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for Peter Zijlstra @ 2015-04-08 15:14 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, acme, tglx, mingo, torvalds, hpa, peterz, dsahern,
	jolsa, adrian.hunter, akpm, eranian, yunlong.song, john.stultz

Commit-ID:  814c8c38e13c7050259c72f89bb01f3fc903f642
Gitweb:     http://git.kernel.org/tip/814c8c38e13c7050259c72f89bb01f3fc903f642
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Tue, 31 Mar 2015 00:19:31 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 8 Apr 2015 10:04:55 -0300

perf record: Add clockid parameter

Teach perf-record about the new perf_event_attr::{use_clockid, clockid}
fields. Add a simple parameter to set the clock (if any) to be used for
the events to be recorded into the data file.

Since we store the entire perf_event_attr in the EVENT_DESC section we
also already store the used clockid in the data file.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: David Ahern <dsahern@gmail.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yunlong Song <yunlong.song@huawei.com>
Link: http://lkml.kernel.org/r/20150407154851.GR23123@twins.programming.kicks-ass.net
[ Conditionally define CLOCK_BOOTTIME, at least rhel6 doesn't have it - dsahern
  Ditto for CLOCK_MONOTONIC_RAW, sles11sp2 doesn't have it - yunlong.song ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt |  7 +++
 tools/perf/builtin-record.c              | 87 ++++++++++++++++++++++++++++++++
 tools/perf/perf.h                        |  2 +
 tools/perf/util/evsel.c                  | 59 ++++++++++++++++++++--
 tools/perf/util/header.c                 |  3 ++
 5 files changed, 154 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 355c4f5..4847a79 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -250,6 +250,13 @@ is off by default.
 --running-time::
 Record running and enabled time for read events (:S)
 
+-k::
+--clockid::
+Sets the clock id to use for the various time fields in the perf_event_type
+records. See clock_gettime(). In particular CLOCK_MONOTONIC and
+CLOCK_MONOTONIC_RAW are supported, some events might also allow
+CLOCK_BOOTTIME, CLOCK_REALTIME and CLOCK_TAI.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 18aad239..ac61048 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -711,6 +711,90 @@ static int perf_record_config(const char *var, const char *value, void *cb)
 	return perf_default_config(var, value, cb);
 }
 
+struct clockid_map {
+	const char *name;
+	int clockid;
+};
+
+#define CLOCKID_MAP(n, c)	\
+	{ .name = n, .clockid = (c), }
+
+#define CLOCKID_END	{ .name = NULL, }
+
+
+/*
+ * Add the missing ones, we need to build on many distros...
+ */
+#ifndef CLOCK_MONOTONIC_RAW
+#define CLOCK_MONOTONIC_RAW 4
+#endif
+#ifndef CLOCK_BOOTTIME
+#define CLOCK_BOOTTIME 7
+#endif
+#ifndef CLOCK_TAI
+#define CLOCK_TAI 11
+#endif
+
+static const struct clockid_map clockids[] = {
+	/* available for all events, NMI safe */
+	CLOCKID_MAP("monotonic", CLOCK_MONOTONIC),
+	CLOCKID_MAP("monotonic_raw", CLOCK_MONOTONIC_RAW),
+
+	/* available for some events */
+	CLOCKID_MAP("realtime", CLOCK_REALTIME),
+	CLOCKID_MAP("boottime", CLOCK_BOOTTIME),
+	CLOCKID_MAP("tai", CLOCK_TAI),
+
+	/* available for the lazy */
+	CLOCKID_MAP("mono", CLOCK_MONOTONIC),
+	CLOCKID_MAP("raw", CLOCK_MONOTONIC_RAW),
+	CLOCKID_MAP("real", CLOCK_REALTIME),
+	CLOCKID_MAP("boot", CLOCK_BOOTTIME),
+
+	CLOCKID_END,
+};
+
+static int parse_clockid(const struct option *opt, const char *str, int unset)
+{
+	struct record_opts *opts = (struct record_opts *)opt->value;
+	const struct clockid_map *cm;
+	const char *ostr = str;
+
+	if (unset) {
+		opts->use_clockid = 0;
+		return 0;
+	}
+
+	/* no arg passed */
+	if (!str)
+		return 0;
+
+	/* no setting it twice */
+	if (opts->use_clockid)
+		return -1;
+
+	opts->use_clockid = true;
+
+	/* if its a number, we're done */
+	if (sscanf(str, "%d", &opts->clockid) == 1)
+		return 0;
+
+	/* allow a "CLOCK_" prefix to the name */
+	if (!strncasecmp(str, "CLOCK_", 6))
+		str += 6;
+
+	for (cm = clockids; cm->name; cm++) {
+		if (!strcasecmp(str, cm->name)) {
+			opts->clockid = cm->clockid;
+			return 0;
+		}
+	}
+
+	opts->use_clockid = false;
+	ui__warning("unknown clockid %s, check man page\n", ostr);
+	return -1;
+}
+
 static const char * const __record_usage[] = {
 	"perf record [<options>] [<command>]",
 	"perf record [<options>] -- <command> [<options>]",
@@ -842,6 +926,9 @@ struct option __record_options[] = {
 		    "Sample machine registers on interrupt"),
 	OPT_BOOLEAN(0, "running-time", &record.opts.running_time,
 		    "Record running/enabled time of read (:S) events"),
+	OPT_CALLBACK('k', "clockid", &record.opts,
+	"clockid", "clockid to use for events, see clock_gettime()",
+	parse_clockid),
 	OPT_END()
 };
 
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index c38a085..e14bb63 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -62,6 +62,8 @@ struct record_opts {
 	u64	     user_interval;
 	bool	     sample_transaction;
 	unsigned     initial_delay;
+	bool         use_clockid;
+	clockid_t    clockid;
 };
 
 struct option;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 358e595..d190f99 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -32,8 +32,12 @@ static struct {
 	bool exclude_guest;
 	bool mmap2;
 	bool cloexec;
+	bool clockid;
+	bool clockid_wrong;
 } perf_missing_features;
 
+static clockid_t clockid;
+
 static int perf_evsel__no_extra_init(struct perf_evsel *evsel __maybe_unused)
 {
 	return 0;
@@ -761,6 +765,12 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 		attr->disabled = 0;
 		attr->enable_on_exec = 0;
 	}
+
+	clockid = opts->clockid;
+	if (opts->use_clockid) {
+		attr->use_clockid = 1;
+		attr->clockid = opts->clockid;
+	}
 }
 
 static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
@@ -1036,7 +1046,6 @@ static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
 	ret += PRINT_ATTR2(exclude_user, exclude_kernel);
 	ret += PRINT_ATTR2(exclude_hv, exclude_idle);
 	ret += PRINT_ATTR2(mmap, comm);
-	ret += PRINT_ATTR2(mmap2, comm_exec);
 	ret += PRINT_ATTR2(freq, inherit_stat);
 	ret += PRINT_ATTR2(enable_on_exec, task);
 	ret += PRINT_ATTR2(watermark, precise_ip);
@@ -1044,6 +1053,9 @@ static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
 	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);
@@ -1055,6 +1067,7 @@ static size_t perf_event_attr__fprintf(struct perf_event_attr *attr, FILE *fp)
 	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);
 
 	ret += fprintf(fp, "%.60s\n", graph_dotted_line);
@@ -1085,6 +1098,12 @@ static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 	}
 
 fallback_missing_features:
+	if (perf_missing_features.clockid_wrong)
+		evsel->attr.clockid = CLOCK_MONOTONIC; /* should always work */
+	if (perf_missing_features.clockid) {
+		evsel->attr.use_clockid = 0;
+		evsel->attr.clockid = 0;
+	}
 	if (perf_missing_features.cloexec)
 		flags &= ~(unsigned long)PERF_FLAG_FD_CLOEXEC;
 	if (perf_missing_features.mmap2)
@@ -1122,6 +1141,17 @@ retry_open:
 				goto try_fallback;
 			}
 			set_rlimit = NO_CHANGE;
+
+			/*
+			 * If we succeeded but had to kill clockid, fail and
+			 * have perf_evsel__open_strerror() print us a nice
+			 * error.
+			 */
+			if (perf_missing_features.clockid ||
+			    perf_missing_features.clockid_wrong) {
+				err = -EINVAL;
+				goto out_close;
+			}
 		}
 	}
 
@@ -1155,7 +1185,17 @@ try_fallback:
 	if (err != -EINVAL || cpu > 0 || thread > 0)
 		goto out_close;
 
-	if (!perf_missing_features.cloexec && (flags & PERF_FLAG_FD_CLOEXEC)) {
+	/*
+	 * Must probe features in the order they were added to the
+	 * perf_event_attr interface.
+	 */
+	if (!perf_missing_features.clockid_wrong && evsel->attr.use_clockid) {
+		perf_missing_features.clockid_wrong = true;
+		goto fallback_missing_features;
+	} else if (!perf_missing_features.clockid && evsel->attr.use_clockid) {
+		perf_missing_features.clockid = true;
+		goto fallback_missing_features;
+	} else if (!perf_missing_features.cloexec && (flags & PERF_FLAG_FD_CLOEXEC)) {
 		perf_missing_features.cloexec = true;
 		goto fallback_missing_features;
 	} else if (!perf_missing_features.mmap2 && evsel->attr.mmap2) {
@@ -2063,9 +2103,7 @@ int perf_evsel__fprintf(struct perf_evsel *evsel,
 		if_print(exclude_hv);
 		if_print(exclude_idle);
 		if_print(mmap);
-		if_print(mmap2);
 		if_print(comm);
-		if_print(comm_exec);
 		if_print(freq);
 		if_print(inherit_stat);
 		if_print(enable_on_exec);
@@ -2076,10 +2114,17 @@ int perf_evsel__fprintf(struct perf_evsel *evsel,
 		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);
@@ -2158,6 +2203,12 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
 	"The PMU counters are busy/taken by another profiler.\n"
 	"We found oprofile daemon running, please stop it and try again.");
 		break;
+	case EINVAL:
+		if (perf_missing_features.clockid)
+			return scnprintf(msg, size, "clockid feature not supported.");
+		if (perf_missing_features.clockid_wrong)
+			return scnprintf(msg, size, "wrong clockid (%d).", clockid);
+		break;
 	default:
 		break;
 	}
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index fb43215..de5f466 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1098,6 +1098,9 @@ static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
 			}
 			fprintf(fp, " }");
 		}
+		if (evsel->attr.use_clockid)
+			fprintf(fp, ", clockid = %d", evsel->attr.clockid);
+
 
 		fputc('\n', fp);
 	}

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf tools: Merge all perf_event_attr print functions
  2015-04-07  9:09 ` [PATCH 2/2] perf, tools: Merge all perf_event_attr print functions Peter Zijlstra
  2015-04-07 13:32   ` Arnaldo Carvalho de Melo
@ 2015-04-08 15:14   ` tip-bot for Peter Zijlstra
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for Peter Zijlstra @ 2015-04-08 15:14 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, torvalds, peterz, linux-kernel, dsahern, acme, eranian,
	mingo, jolsa, john.stultz, adrian.hunter, akpm, hpa

Commit-ID:  2c5e8c52c6354f77c4019357be8231bcc34456f8
Gitweb:     http://git.kernel.org/tip/2c5e8c52c6354f77c4019357be8231bcc34456f8
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Tue, 7 Apr 2015 11:09:54 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 8 Apr 2015 10:06:28 -0300

perf tools: Merge all perf_event_attr print functions

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

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150407091150.644238729@infradead.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c  | 288 +++++++++++++++++++++--------------------------
 tools/perf/util/evsel.h  |   6 +
 tools/perf/util/header.c |  29 ++---
 3 files changed, 141 insertions(+), 182 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d190f99..33e3fd8 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1011,70 +1011,126 @@ static int get_group_fd(struct perf_evsel *evsel, int cpu, int thread)
 	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;
-
-	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);
-
-	ret += fprintf(fp, "%.60s\n", graph_dotted_line);
+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);
+}
+
+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)
+
+#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 @@ retry_sample_id:
 	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 *first, const char *fmt, ...)
 	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)
+static int __print_attr__fprintf(FILE *fp, const char *name, const char *val, void *priv)
 {
-	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_evsel *evsel,
 
 	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;
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index c5a43d6..e486151 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -360,4 +360,10 @@ static inline bool has_branch_callstack(struct perf_evsel *evsel)
 {
 	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 */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index de5f466..fff3b2a 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1055,6 +1055,12 @@ error:
 	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_header *ph, int fd, FILE *fp)
 	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_header *ph, int fd, FILE *fp)
 			}
 			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);
 	}

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-04-08 15:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-07  9:09 [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing Peter Zijlstra
2015-04-07  9:09 ` [PATCH 1/2] perf, record: Add clockid parameter Peter Zijlstra
2015-04-07 15:48   ` Peter Zijlstra
2015-04-07 17:33     ` Jiri Olsa
2015-04-08  9:47       ` Peter Zijlstra
2015-04-08 15:14     ` [tip:perf/core] perf " tip-bot for Peter Zijlstra
2015-04-07  9:09 ` [PATCH 2/2] perf, tools: Merge all perf_event_attr print functions Peter Zijlstra
2015-04-07 13:32   ` Arnaldo Carvalho de Melo
2015-04-08 15:14   ` [tip:perf/core] perf " tip-bot for Peter Zijlstra
2015-04-07  9:23 ` [PATCH 0/2] perf tools: clock_id support and perf_event_attr printing Ingo Molnar
2015-04-07 12:08 ` Arnaldo Carvalho de Melo
2015-04-07 12:25   ` Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.