[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150718124547.GA3759@krava.brq.redhat.com>
Date: Sat, 18 Jul 2015 14:45:47 +0200
From: Jiri Olsa <jolsa@...hat.com>
To: kan.liang@...el.com
Cc: acme@...nel.org, jolsa@...nel.org, namhyung@...nel.org,
ak@...ux.intel.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC V5 2/4] perf,tool: per-event time support
On Fri, Jul 17, 2015 at 07:30:53AM -0400, kan.liang@...el.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@...va perf]$ ./perf record -e 'cpu/cpu-cycles/,cpu/instructions/' kill
...
[jolsa@...va 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@...va perf]$ ./perf record -e 'cpu/cpu-cycles/,cpu/instructions,time/' kill
...
[jolsa@...va 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists