From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752567AbbGNGyn (ORCPT ); Tue, 14 Jul 2015 02:54:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32826 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752466AbbGNGyl (ORCPT ); Tue, 14 Jul 2015 02:54:41 -0400 Date: Tue, 14 Jul 2015 08:54:38 +0200 From: Jiri Olsa To: kan.liang@intel.com Cc: acme@kernel.org, jolsa@kernel.org, namhyung@kernel.org, ak@linux.intel.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC V3 3/5] perf,tool: partial time support Message-ID: <20150714065438.GE22977@krava.local> 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=us-ascii Content-Disposition: inline In-Reply-To: <1436345097-11113-4-git-send-email-kan.liang@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. [PATCH RFC V3 3/5] perf,tool: partial time support for a minute I was wondering why's the patch not full and just partial ;-) 'per event' might sound better SNIP > 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); > + } so we have TIME enabled by default, like: [jolsa@krava perf]$ ./perf record ls ... [jolsa@krava perf]$ ./perf evlist -v cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD, 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 this patch disables it by default and there might be people out there who actualy care. I wonder you should instead disable it completely via --no-timestamp option and enable it via 'time' term for specific events, like: $ perf record --no-timestamp -e 'cpu/...time/,cpu/.../'ls or disable timestamps globaly if 'time' term is detected in any event definition..? jirka