| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <37D7C6CF3E00A74B8858931C1DB2F0770188F5E9@SHSMSX103.ccr.corp.intel.com>
Date: Mon, 20 Jul 2015 15:04:20 +0000
From: "Liang, Kan" <kan.liang@...el.com>
To: Jiri Olsa <jolsa@...hat.com>, Namhyung Kim <namhyung@...nel.org>
CC: "acme@...nel.org" <acme@...nel.org>,
"jolsa@...nel.org" <jolsa@...nel.org>,
"ak@...ux.intel.com" <ak@...ux.intel.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH RFC V5 2/4] perf,tool: per-event time support
> > > [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
> >
> > What about this case?
> >
> > $ perf record -e 'cpu/cpu-cycles/,cpu/instructions,time=0/' kill
>
> right.. hum, we need somehow separate/pospone the users term
> application to the final perf_event_attr.. spent some time on it today, but
> could not find any nice solution so far.. will try tomorrow ;-)
>
> SNIP
>
> > > + * 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'm not sure about it. Because, for period value, the user set specific value
by option can override the per-event setting.
I think we should make them consistent.
$ perf record -e 'cpu/instructions,period=20000/' -c 1000 sleep 1
$ perf evlist -v
cpu/instructions,period=20000/: type: 4, size: 112, config: 0xc0,
{ sample_period, sample_freq }: 1000
How about adding a per-evsel user_time_set which indicate if the time
item is set?
$ perf record -e 'cpu/cpu-cycles/,cpu/instructions,time=0/'
$ perf evlist -v
cpu/cpu-cycles/:... sample_type: IP|TID|TIME|PERIOD|IDENTIFIER
cpu/instructions,time=0/:... sample_type: IP|TID|PERIOD|IDENTIFIER
$ perf record -e 'cpu/cpu-cycles/,cpu/instructions,time/'
$ perf evlist -v
cpu/cpu-cycles/:... sample_type: IP|TID|TIME|PERIOD|IDENTIFIER
cpu/instructions,time=0/:... sample_type: IP|TID|TIME|PERIOD|IDENTIFIER
$ perf record -T -e 'cpu/cpu-cycles/,cpu/instructions,time=0/' sleep 1
$ perf evlist -v
cpu/cpu-cycles/:... sample_type: IP|TID|TIME|PERIOD|IDENTIFIER
cpu/instructions,time=0/:... sample_type: IP|TID|TIME|PERIOD|IDENTIFIER
$perf record --no-timestamp -e 'cpu/cpu-cycles/,cpu/instructions,time=0/' sleep 1
$ perf evlist -v
cpu/cpu-cycles/:... sample_type: IP|TID|PERIOD|IDENTIFIER
cpu/instructions,time=0/:... sample_type: IP|TID|PERIOD|IDENTIFIER
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 5b47b2c..df47907 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 83c0803..cfa09f1 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -619,10 +619,35 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
struct perf_event_attr *attr = &evsel->attr;
int track = evsel->tracking;
bool per_cpu = opts->target.default_per_cpu && !opts->target.per_thread;
+ bool sample_time = opts->sample_time;
attr->sample_id_all = perf_missing_features.sample_id_all ? 0 : 1;
attr->inherit = !opts->no_inherit;
+ /*
+ * If user doesn't explicitly set time option,
+ * let event attribute decide.
+ */
+
+ if (!opts->sample_time_set && evsel->user_time_set) {
+ if (attr->sample_type & PERF_SAMPLE_TIME)
+ sample_time = true;
+ else
+ sample_time = false;
+ }
+
+ /*
+ * Event parsing doesn't check the availability
+ * Clear the bit which event parsing may be set.
+ * Let following code check and reset if available
+ *
+ * Also, the sample size may be caculated mistakenly,
+ * because event parsing may set the PERF_SAMPLE_TIME.
+ * Remove the size which add in perf_evsel__init
+ */
+ if (attr->sample_type & PERF_SAMPLE_TIME)
+ perf_evsel__reset_sample_bit(evsel, TIME);
+
perf_evsel__set_sample_bit(evsel, IP);
perf_evsel__set_sample_bit(evsel, TID);
@@ -705,14 +730,15 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
/*
* When the user explicitely disabled time don't force it here.
*/
- if (opts->sample_time &&
+ if (sample_time &&
(!perf_missing_features.sample_id_all &&
(!opts->no_inherit || target__has_cpu(&opts->target) || per_cpu ||
opts->sample_time_set)))
perf_evsel__set_sample_bit(evsel, TIME);
if (opts->raw_samples && !evsel->no_aux_samples) {
- perf_evsel__set_sample_bit(evsel, TIME);
+ if (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/evsel.h b/tools/perf/util/evsel.h
index fe9f327..b654b90 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -79,6 +79,7 @@ struct perf_evsel {
bool system_wide;
bool tracking;
bool per_pkg;
+ bool user_time_set;
/* parse modifier helper */
int exclude_GH;
int nr_members;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a71eeb2..6f2e10e 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -561,7 +561,8 @@ static int check_type_val(struct parse_events_term *term,
static int config_term(struct perf_event_attr *attr,
struct parse_events_term *term,
- struct parse_events_error *err)
+ struct parse_events_error *err,
+ bool *time_set)
{
#define CHECK_TYPE_VAL(type) \
do { \
@@ -598,6 +599,15 @@ do { \
* attr->branch_sample_type = term->val.num;
*/
break;
+ case PARSE_EVENTS__TERM_TYPE_TIME:
+ if (time_set)
+ *time_set = true;
+ 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;
@@ -611,12 +621,13 @@ do { \
static int config_attr(struct perf_event_attr *attr,
struct list_head *head,
- struct parse_events_error *err)
+ struct parse_events_error *err,
+ bool *time_set)
{
struct parse_events_term *term;
list_for_each_entry(term, head, list)
- if (config_term(attr, term, err))
+ if (config_term(attr, term, err, time_set))
return -EINVAL;
return 0;
@@ -634,7 +645,7 @@ int parse_events_add_numeric(struct parse_events_evlist *data,
attr.config = config;
if (head_config &&
- config_attr(&attr, head_config, data->error))
+ config_attr(&attr, head_config, data->error, NULL))
return -EINVAL;
return add_event(list, &data->idx, &attr, NULL);
@@ -664,6 +675,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
struct perf_pmu_info info;
struct perf_pmu *pmu;
struct perf_evsel *evsel;
+ bool time_set = false;
pmu = perf_pmu__find(name);
if (!pmu)
@@ -689,7 +701,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
* Configure hardcoded terms first, no need to check
* return value when called with fail == 0 ;)
*/
- if (config_attr(&attr, head_config, data->error))
+ if (config_attr(&attr, head_config, data->error, &time_set))
return -EINVAL;
if (perf_pmu__config(pmu, &attr, head_config, data->error))
@@ -702,6 +714,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
evsel->scale = info.scale;
evsel->per_pkg = info.per_pkg;
evsel->snapshot = info.snapshot;
+ evsel->user_time_set = time_set;
}
return evsel ? 0 : -ENOMEM;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 131f29b..0d8cae3 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 13cef3c..f542750 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 7bcb8c3..b615cdf 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:"))
Thanks,
Kan
--
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