Re: [PATCH] perf, record: Add clockid parameter

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Mar 30, 2015 at 01:41:12PM -0600, David Ahern wrote:
> On 3/30/15 1:33 PM, Peter Zijlstra wrote:
> >On Mon, Mar 30, 2015 at 11:24:12AM -0600, David Ahern wrote:
> >>This works but the result is not always intuitive as to why it failed.
> >>
> >>On a kernel that does not support the clock id you get:
> >>     $ perf sched record -k mono -- sleep 1
> >>     Error:
> >>     clockid not supported.
> >>
> >>And on a kernel that supports clockid but not for NMI:
> >>
> >>     $ perf record -k realtime -a -- sleep 1
> >>     Error:
> >>     clockid not supported.
> >>
> >>     --> H/W counters so realtime is not allowed
> >>
> >>Same message though different root causes.
> >
> >Heh, ok I can fudge that :-)
> >
> 
> What about having the kernel return 'not supported' error for the latter --
> H/W counters with unsafe clock?

That's bound to only work for a little while until something else starts
returning -EOPNOTSUPP too.


Fwiw, having 3 incomplete ways to print perf_event_attr() is
disgusting.

---
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 dat file.

Cc: jolsa@redhat.com
Cc: dsahern@gmail.com
Cc: adrian.hunter@intel.com
Cc: pawel.moll@arm.com
Cc: acme@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);
 	}