From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753119AbbC0Whu (ORCPT ); Fri, 27 Mar 2015 18:37:50 -0400 Received: from mail-oi0-f47.google.com ([209.85.218.47]:35844 "EHLO mail-oi0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752621AbbC0Whr (ORCPT ); Fri, 27 Mar 2015 18:37:47 -0400 MIME-Version: 1.0 In-Reply-To: <20150327215942.GL23123@twins.programming.kicks-ass.net> References: <20150327143201.GG21418@twins.programming.kicks-ass.net> <55158F25.9040100@gmail.com> <20150327172059.GK23123@twins.programming.kicks-ass.net> <551594DD.7050705@gmail.com> <20150327201534.GD6291@redhat.com> <20150327215942.GL23123@twins.programming.kicks-ass.net> Date: Fri, 27 Mar 2015 15:37:47 -0700 Message-ID: Subject: Re: [PATCH] perf, record: Add clockid parameter From: Stephane Eranian To: Peter Zijlstra Cc: Arnaldo Carvalho de Melo , David Ahern , Thomas Gleixner , Jiri Olsa , Linus Torvalds , LKML , John Stultz , "H. Peter Anvin" , Andrew Morton , Ingo Molnar Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 27, 2015 at 2:59 PM, Peter Zijlstra wrote: > On Fri, Mar 27, 2015 at 05:15:34PM -0300, Arnaldo Carvalho de Melo wrote: >> I.e. we're back to the sys_perf_event_open() error reporting suckz rockz >> thing, this time with PeterZ trying to find a way to avoid getting back >> to that discussion... /me runz... ;-P > > :-) > > Not entirely, its just that I've not seen this userspace code in years > and I'm pretty clueless. > > How about so then? > > --- > Subject: perf, record: Add clockid parameter > From: Peter Zijlstra > Date: Fri, 27 Mar 2015 15:32:01 +0100 > > 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: acme@redhat.com > Signed-off-by: Peter Zijlstra (Intel) > --- > > tools/perf/Documentation/perf-record.txt | 5 +++ > tools/perf/builtin-record.c | 3 ++ > tools/perf/perf.h | 1 > tools/perf/util/evsel.c | 44 ++++++++++++++++++++++++++++--- > 4 files changed, 49 insertions(+), 4 deletions(-) > > --- a/tools/perf/Documentation/perf-record.txt > +++ b/tools/perf/Documentation/perf-record.txt > @@ -250,6 +250,11 @@ 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(). > + > SEE ALSO > -------- > linkperf:perf-stat[1], linkperf:perf-list[1] > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -739,6 +739,7 @@ static struct record record = { > .uses_mmap = true, > .default_per_cpu = true, > }, > + .clockid = -1, > }, > .tool = { > .sample = process_sample_event, > @@ -842,6 +843,8 @@ 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_INTEGER('k', "clockid", &record.opts.clockid, > + "clockid to use for events"), I think you'd want a symbolic name for the clock to make this easier on the user. > 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,6 +32,7 @@ static struct { > bool exclude_guest; > bool mmap2; > bool cloexec; > + bool clockid; > } perf_missing_features; > > static int perf_evsel__no_extra_init(struct perf_evsel *evsel __maybe_unused) > @@ -761,6 +762,11 @@ void perf_evsel__config(struct perf_evse > attr->disabled = 0; > attr->enable_on_exec = 0; > } > + > + 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 +1042,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 +1049,8 @@ 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 +1062,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 +1093,8 @@ static int __perf_evsel__open(struct per > } > > fallback_missing_features: > + if (perf_missing_features.clockid) > + evsel->attr.use_clockid = 0; > if (perf_missing_features.cloexec) > flags &= ~(unsigned long)PERF_FLAG_FD_CLOEXEC; > if (perf_missing_features.mmap2) > @@ -1122,6 +1132,16 @@ 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) { > + err = -EINVAL; > + goto out_close; > + } > } > } > > @@ -1155,7 +1175,10 @@ 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)) { > + 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 +2086,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 +2097,19 @@ int perf_evsel__fprintf(struct perf_evse > if_print(sample_id_all); > if_print(exclude_host); > if_print(exclude_guest); > + if_print(exclude_callchain_kernel); > + if_print(exclude_callchain_user); > + 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 +2188,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, "%s", "clockid not supported."); > + break; > + > default: > break; > }