Hi, (CC-ing Adrian)
On Wed, Jul 08, 2015 at 04:44:55AM -0400, kan.li...@intel.com wrote: > From: Kan Liang <kan.li...@intel.com> > > When multiple events are sampled it may not be needed to collect fine > grained time stamps on all events. The sample sites are usually nearby. > It's enough to have time stamps on the regular reference events. > This patchkit adds the ability to turn off time stamps per event. This > in term can reduce sampling overhead and the size of the perf.data. So this patch makes the PERF_SAMPLE_TIME bit set or not independently, right? But AFAIK we sometimes just use first evsel for checking sample_type value, especially for evlist->id_pos. I'm not sure it'll work for all cases of mixed time/notime events.. Thanks, Namhyung > > Signed-off-by: Kan Liang <kan.li...@intel.com> > --- > tools/perf/Documentation/perf-record.txt | 4 +++- > tools/perf/util/evsel.c | 32 > ++++++++++++++++++++++++++++++-- > tools/perf/util/parse-events.c | 7 +++++++ > tools/perf/util/parse-events.h | 1 + > tools/perf/util/parse-events.l | 1 + > 5 files changed, 42 insertions(+), 3 deletions(-) > > 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..1d58330 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -619,10 +619,37 @@ 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) { > + 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) { > + attr->sample_type &= ~PERF_SAMPLE_TIME; > + evsel->sample_size -= sizeof(u64); > + } > + > perf_evsel__set_sample_bit(evsel, IP); > perf_evsel__set_sample_bit(evsel, TID); > > @@ -705,14 +732,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/parse-events.c b/tools/perf/util/parse-events.c > index a71eeb2..9510047 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 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); } > -- > 1.8.3.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/