Hi Jiri,
On Sat, Jul 18, 2015 at 02:45:47PM +0200, Jiri Olsa wrote:
> On Fri, Jul 17, 2015 at 07:30:53AM -0400, [email protected] 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
What about this case?
$ perf record -e 'cpu/cpu-cycles/,cpu/instructions,time=0/' kill
>
>
> ---
> 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;
I think it'd be better if the -T/--timestamp option gives the default
TIME sample_type value but it can be overridden by per-event setting.
I mean:
(per-event 'time' setting is meaningless here)
$ perf record -e 'cpu/cpu-cycles/,cpu/instructions,time/' kill
$ perf evlist -v
cpu/cpu-cycles/: ..., sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, ...
cpu/instructions,time/: ..., sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, ...
(adding -T option, same as the default)
$ perf record -T -e 'cpu/cpu-cycles/,cpu/instructions,time/' kill
$ perf evlist -v
cpu/cpu-cycles/: ..., sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, ...
cpu/instructions,time/: ..., sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, ...
$ perf record -T -e 'cpu/cpu-cycles/,cpu/instructions,time=0/' kill
$ perf evlist -v
cpu/cpu-cycles/: ..., sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, ...
cpu/instructions,time/: ..., sample_type: IP|TID|PERIOD|IDENTIFIER, ...
(adding --no-timestamp option)
$ perf record --no-timestamp -e 'cpu/cpu-cycles/,cpu/instructions/' kill
$ perf evlist -v
cpu/cpu-cycles/: ..., sample_type: IP|TID|PERIOD|IDENTIFIER, ...
cpu/instructions,time/: ..., sample_type: IP|TID|PERIOD|IDENTIFIER, ...
$ perf record --no-timestamp -e 'cpu/cpu-cycles/,cpu/instructions,time=1/'
kill
$ perf evlist -v
cpu/cpu-cycles/: ..., sample_type: IP|TID|PERIOD|IDENTIFIER, ...
cpu/instructions,time/: ..., sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, ...
Thanks,
Namhyung
> +
> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
