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/
