From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751645AbbGMN35 (ORCPT ); Mon, 13 Jul 2015 09:29:57 -0400 Received: from mail-pd0-f177.google.com ([209.85.192.177]:34046 "EHLO mail-pd0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751031AbbGMN34 (ORCPT ); Mon, 13 Jul 2015 09:29:56 -0400 Date: Mon, 13 Jul 2015 22:27:42 +0900 From: Namhyung Kim To: kan.liang@intel.com Cc: acme@kernel.org, jolsa@kernel.org, ak@linux.intel.com, linux-kernel@vger.kernel.org, adrian.hunter@intel.com Subject: Re: [PATCH RFC V3 3/5] perf,tool: partial time support Message-ID: <20150713132742.GA9917@danjae.kornet> References: <1436345097-11113-1-git-send-email-kan.liang@intel.com> <1436345097-11113-4-git-send-email-kan.liang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1436345097-11113-4-git-send-email-kan.liang@intel.com> User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, (CC-ing Adrian) On Wed, Jul 08, 2015 at 04:44:55AM -0400, kan.liang@intel.com wrote: > From: Kan Liang > > 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 > --- > 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 >