Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752329AbbGRMpx (ORCPT ); Sat, 18 Jul 2015 08:45:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33133 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751321AbbGRMpu (ORCPT ); Sat, 18 Jul 2015 08:45:50 -0400 Date: Sat, 18 Jul 2015 14:45:47 +0200 From: Jiri Olsa To: kan.liang@intel.com Cc: acme@kernel.org, jolsa@kernel.org, namhyung@kernel.org, ak@linux.intel.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC V5 2/4] perf,tool: per-event time support Message-ID: <20150718124547.GA3759@krava.brq.redhat.com> References: <1437132655-15883-1-git-send-email-kan.liang@intel.com> <1437132655-15883-3-git-send-email-kan.liang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1437132655-15883-3-git-send-email-kan.liang@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7541 Lines: 196 On Fri, Jul 17, 2015 at 07:30:53AM -0400, kan.liang@intel.com wrote: SNIP > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index a71eeb2..c9981df 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -25,6 +25,9 @@ > #ifdef PARSER_DEBUG > extern int parse_events_debug; > #endif > + > +bool time_term_detected = false; > + > int parse_events_parse(void *data, void *scanner); > > static struct perf_pmu_event_symbol *perf_pmu_events_list; > @@ -598,6 +601,14 @@ do { \ > * attr->branch_sample_type = term->val.num; > */ > break; > + case PARSE_EVENTS__TERM_TYPE_TIME: > + CHECK_TYPE_VAL(NUM); > + if (term->val.num > 1) > + return -EINVAL; > + time_term_detected = true; > + if (term->val.num == 1) > + attr->sample_type |= PERF_SAMPLE_TIME; > + break; > case PARSE_EVENTS__TERM_TYPE_NAME: > CHECK_TYPE_VAL(STR); > break; > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h > index 131f29b..1083478 100644 > --- a/tools/perf/util/parse-events.h > +++ b/tools/perf/util/parse-events.h > @@ -22,6 +22,8 @@ struct tracepoint_path { > struct tracepoint_path *next; > }; > > +extern bool time_term_detected; so I wasnt happy about this time_term_detected global variable, and I tried to make it without and ended up with somewhat siplified patch.. not tested very deeply, just the basics: [jolsa@krava perf]$ ./perf record -e 'cpu/cpu-cycles/,cpu/instructions/' kill ... [jolsa@krava perf]$ ./perf evlist -v cpu/cpu-cycles/: type: 4, size: 112, config: 0x3c, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|ID|PERIOD, read_format: ID, 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 cpu/instructions/: type: 4, size: 112, config: 0xc0, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|ID|PERIOD, read_format: ID, disabled: 1, inherit: 1, freq: 1, enable_on_exec: 1, sample_id_all: 1, exclude_guest: 1 [jolsa@krava perf]$ ./perf record -e 'cpu/cpu-cycles/,cpu/instructions,time/' kill ... [jolsa@krava perf]$ ./perf evlist -v cpu/cpu-cycles/: type: 4, size: 112, config: 0x3c, { sample_period, sample_freq }: 4000, sample_type: IP|TID|PERIOD|IDENTIFIER, read_format: ID, 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 cpu/instructions,time/: type: 4, size: 112, config: 0xc0, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, freq: 1, enable_on_exec: 1, sample_id_all: 1, exclude_guest: 1 thoughts? jirka --- diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt index 5b47b2c88223..df479077384d 100644 --- a/tools/perf/Documentation/perf-record.txt +++ b/tools/perf/Documentation/perf-record.txt @@ -49,7 +49,9 @@ OPTIONS These params can be used to set event defaults. Here is a list of the params. - 'period': Set event sampling period - + - 'time': Disable/enable time stamping. Acceptable values are 1 for + enabling time stamping. 0 for disabling time stamping. + The default is 1. Note: If user explicitly sets options which conflict with the params, the value set by the params will be overridden. diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 83c08037e7e2..8e3a17845c37 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -712,7 +712,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts) perf_evsel__set_sample_bit(evsel, TIME); if (opts->raw_samples && !evsel->no_aux_samples) { - perf_evsel__set_sample_bit(evsel, TIME); + if (opts->sample_time) + perf_evsel__set_sample_bit(evsel, TIME); perf_evsel__set_sample_bit(evsel, RAW); perf_evsel__set_sample_bit(evsel, CPU); } diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index a71eeb279ed2..95100478200a 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -598,6 +598,13 @@ do { \ * attr->branch_sample_type = term->val.num; */ break; + case PARSE_EVENTS__TERM_TYPE_TIME: + CHECK_TYPE_VAL(NUM); + if (term->val.num > 1) + return -EINVAL; + if (term->val.num == 1) + attr->sample_type |= PERF_SAMPLE_TIME; + break; case PARSE_EVENTS__TERM_TYPE_NAME: CHECK_TYPE_VAL(STR); break; diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 131f29b2f132..0d8cae31b506 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -62,6 +62,7 @@ enum { PARSE_EVENTS__TERM_TYPE_NAME, PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD, PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE, + PARSE_EVENTS__TERM_TYPE_TIME, }; struct parse_events_term { diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l index 13cef3c65565..f5427505ae77 100644 --- a/tools/perf/util/parse-events.l +++ b/tools/perf/util/parse-events.l @@ -183,6 +183,7 @@ config2 { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG2); } name { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NAME); } period { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD); } branch_type { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE); } +time { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_TIME); } , { return ','; } "/" { BEGIN(INITIAL); return '/'; } {name_minus} { return str(yyscanner, PE_NAME); } diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index 7bcb8c315615..b615cdf211d6 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -607,7 +607,7 @@ static char *formats_error_string(struct list_head *formats) { struct perf_pmu_format *format; char *err, *str; - static const char *static_terms = "config,config1,config2,name,period,branch_type\n"; + static const char *static_terms = "config,config1,config2,name,period,branch_type,time\n"; unsigned i = 0; if (!asprintf(&str, "valid terms:")) diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c index 1f7becbe5e18..6b42c1339fde 100644 --- a/tools/perf/util/record.c +++ b/tools/perf/util/record.c @@ -95,6 +95,18 @@ static bool perf_can_comm_exec(void) return perf_probe_api(perf_probe_comm_exec); } +static bool perf_evlist__has_time(struct perf_evlist *evlist) +{ + struct perf_evsel *evsel; + + evlist__for_each(evlist, evsel) { + if (evsel->attr.sample_type & PERF_SAMPLE_TIME) + return true; + } + + return false; +} + void perf_evlist__config(struct perf_evlist *evlist, struct record_opts *opts) { struct perf_evsel *evsel; @@ -111,6 +123,14 @@ void perf_evlist__config(struct perf_evlist *evlist, struct record_opts *opts) if (evlist->cpus->map[0] < 0) opts->no_inherit = true; + /* + * If time (-T) is not set and we have events with TIME sample_type + * set (tracepoints or events with time term), disable timestamp for + * the rest of the events. + */ + if (!opts->sample_time_set && perf_evlist__has_time(evlist)) + opts->sample_time = false; + use_comm_exec = perf_can_comm_exec(); evlist__for_each(evlist, evsel) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/