[PATCH RFC V3 0/5] partial callgrap and time support

[PATCH RFC V3 0/5] partial callgrap and time support

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

This patchkit adds the ability to turn off callgraphs and time stamps
per event. This in term can reduce sampling overhead and the size of
the perf.data.

Changes since V1:
  - Break up V1 patches into three patches(parse option changes,
    partial time support and partial callgrap support).
  - Use strings 'fp,dwarf,lbr,no' to identify callchains
  - Add test case in parse-events.c

Changes since V2:
  - Rebase on 60cd37eb10

Kan Liang (5):
  perf,tools: introduce OPT_CALLBACK_SET/OPT_CALLBACK_NOOPT_SET
  perf/documentation: Add description for period
  perf,tool: partial time support
  perf,tool: partial callgrap support
  perf,tests: Add tests to callgrap and time parse

 tools/perf/Documentation/perf-record.txt | 14 ++++++++
 tools/perf/builtin-record.c              |  4 +--
 tools/perf/perf.h                        |  1 +
 tools/perf/tests/parse-events.c          | 28 ++++++++++++++++
 tools/perf/util/evsel.c                  | 56 ++++++++++++++++++++++++++++++--
 tools/perf/util/parse-events.c           | 25 ++++++++++++++
 tools/perf/util/parse-events.h           |  3 ++
 tools/perf/util/parse-events.l           |  3 ++
 tools/perf/util/parse-options.c          |  2 ++
 tools/perf/util/parse-options.h          |  4 +++
 10 files changed, 135 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[PATCH RFC V3 1/5] perf,tools: introduce OPT_CALLBACK_SET/OPT_CALLBACK_NOOPT_SET

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

This patch extends the OPT_CALLBACK/OPT_CALLBACK_NOOPT to support 'set'
item of struct option. So the perf knows whether an option was set by
user.
The new macros are used by call-graph options.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/builtin-record.c     | 4 ++--
 tools/perf/perf.h               | 1 +
 tools/perf/util/parse-options.c | 2 ++
 tools/perf/util/parse-options.h | 4 ++++
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 283fe96..a6eb24e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1018,10 +1018,10 @@ struct option __record_options[] = {
 		     record__parse_mmap_pages),
 	OPT_BOOLEAN(0, "group", &record.opts.group,
 		    "put the counters into a counter group"),
-	OPT_CALLBACK_NOOPT('g', NULL, &record.opts,
+	OPT_CALLBACK_NOOPT_SET('g', NULL, &record.opts, &record.opts.callgraph_set,
 			   NULL, "enables call-graph recording" ,
 			   &record_callchain_opt),
-	OPT_CALLBACK(0, "call-graph", &record.opts,
+	OPT_CALLBACK_SET(0, "call-graph", &record.opts, &record.opts.callgraph_set,
 		     "mode[,dump_size]", record_callchain_help,
 		     &record_parse_callchain_opt),
 	OPT_INCR('v', "verbose", &verbose,
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 937b16a..9ba02e0 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -52,6 +52,7 @@ struct record_opts {
 	bool	     sample_weight;
 	bool	     sample_time;
 	bool	     sample_time_set;
+	bool	     callgraph_set;
 	bool	     period;
 	bool	     sample_intr_regs;
 	bool	     running_time;
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 01626be..064385f 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -140,6 +140,8 @@ static int get_value(struct parse_opt_ctx_t *p,
 		return err;
 
 	case OPTION_CALLBACK:
+		if (opt->set)
+			*(bool *)opt->set = true;
 		if (unset)
 			return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
 		if (opt->flags & PARSE_OPT_NOARG)
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index 367d8b8..2bec32e 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -132,8 +132,12 @@ struct option {
 	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), .argh = "time", .help = (h), .callback = parse_opt_approxidate_cb }
 #define OPT_CALLBACK(s, l, v, a, h, f) \
 	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f) }
+#define OPT_CALLBACK_SET(s, l, v, os, a, h, f) \
+	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .set = check_vtype(os, bool *) }
 #define OPT_CALLBACK_NOOPT(s, l, v, a, h, f) \
 	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .flags = PARSE_OPT_NOARG }
+#define OPT_CALLBACK_NOOPT_SET(s, l, v, os, a, h, f) \
+	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .flags = PARSE_OPT_NOARG, .set = check_vtype(os, bool *) }
 #define OPT_CALLBACK_DEFAULT(s, l, v, a, h, f, d) \
 	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .defval = (intptr_t)d, .flags = PARSE_OPT_LASTARG_DEFAULT }
 #define OPT_CALLBACK_DEFAULT_NOOPT(s, l, v, a, h, f, d) \
-- 
1.8.3.1

[PATCH RFC V3 2/5] perf/documentation: Add description for period

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

'period' param is not defined in
/sys/bus/event_sources/devices/<pmu>/format/*

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/Documentation/perf-record.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 9b9d9d0..5b47b2c 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -45,6 +45,14 @@ OPTIONS
           param1 and param2 are defined as formats for the PMU in:
           /sys/bus/event_sources/devices/<pmu>/format/*
 
+	  There are also some params which are not defined in .../<pmu>/format/*.
+	  These params can be used to set event defaults.
+	  Here is a list of the params.
+	  - 'period': Set event sampling period
+
+	  Note: If user explicitly sets options which conflict with the params,
+	  the value set by the params will be overridden.
+
         - a hardware breakpoint event in the form of '\mem:addr[/len][:access]'
           where addr is the address in memory you want to break in.
           Access is the memory access type (read, write, execute) it can
-- 
1.8.3.1

[PATCH RFC V3 3/5] perf,tool: partial time support

[kan.liang]

From: Kan Liang <kan.liang@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.

Signed-off-by: Kan Liang <kan.liang@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

[PATCH RFC V3 4/5] perf,tool: partial callgrap support

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

When multiple events are sampled it may not be needed to collect
callgraphs for all of them. The sample sites are usually nearby, and
it's enough to collect the callgraphs on a reference event (such as
precise cycles or precise instructions).
This patchkit adds the ability to turn off callgraphs and time stamps
per event. This in term can reduce sampling overhead and the size of the
perf.data. Furthermore, it makes collecting back traces and timestamps
possible when PEBS threshold > 1, which significantly reducing the
sampling overhead especially for frequently occurring events
(https://lkml.org/lkml/2015/5/10/196). For example, A slower event with
a larger period collects back traces/timestamps. Other more events run
fast with multi-pebs. The time stamps from the slower events can be used
to order the faster events. Their backtraces can give the user enough
hint to find the right spot.

Here are some examples and test results.

1. Comparing the elapsed time and perf.data size from "kernbench -M -H".

 The test command for FULL callgrap and time support.
   "perf record -e
   '{cpu/cpu-cycles,period=100000/,cpu/instructions,period=20000/p}'
   --call-graph fp --time"

 The test command for PARTIAL callgrap and time support.
   "perf record -e
   '{cpu/cpu-cycles,callgraph=fp,time,period=100000/,
     cpu/instructions,callgraph=no,time=0,period=20000/p}'"

 The elapsed time for FULL is 24.3 Sec, while for PARTIAL is 16.9 Sec.
 The perf.data size for FULL is 22.1 Gb, while for PARTIAL is 12.4 Gb.

2. Comparing the perf.data size and callgraph results.

 The test command for FULL callgrap and time support.
   "perf record -e
   '{cpu/cpu-cycles,period=100000/pp,cpu/instructions,period=20000/p}'
   --call-graph fp -- ./tchain_edit"

 The test command for PARTIAL callgrap and time support.
   "perf record -e
   '{cpu/cpu-cycles,callgraph=fp,time,period=100000/pp,
     cpu/instructions,callgraph=no,time=0,period=20000/p}'
   -- ./tchain_edit"

 The perf.data size for FULL is 43.2 MB, while for PARTIAL is 21.1 MB.
 The callgraph is roughly the same.

 The callgraph from FULL
 # Samples: 87K of event
 'cpu/cpu-cycles,callgraph=fp,time,period=100000/pp'
 # Event count (approx.): 8760000000
 #
 # Children      Self  Command      Shared Object       Symbol
 # ........  ........  ...........  ..................
..........................................
 #
    99.98%     0.00%  tchain_edit  libc-2.15.so        [.]
__libc_start_main
            |
            ---__libc_start_main

    99.97%     0.00%  tchain_edit  tchain_edit         [.] main
            |
            ---main
               __libc_start_main

    99.97%     0.00%  tchain_edit  tchain_edit         [.] f1
            |
            ---f1
               main
               __libc_start_main

    99.85%    87.01%  tchain_edit  tchain_edit         [.] f3
            |
            ---f3
               |
               |--99.74%-- f2
               |          f1
               |          main
               |          __libc_start_main
                --0.26%-- [...]
    99.71%     0.12%  tchain_edit  tchain_edit         [.] f2
            |
            ---f2
               f1
               main
               __libc_start_main

 The callgraph from PARTIAL
 # Samples: 417K of event
 'cpu/instructions,callgraph=no,time=0,period=20000/p'
 # Event count (approx.): 8346980000
 #
 # Children      Self  Command      Shared Object     Symbol
 # ........  ........  ...........  ................
..........................................
 #
    98.82%     0.00%  tchain_edit  libc-2.15.so      [.]
__libc_start_main
            |
            ---__libc_start_main

    98.82%     0.00%  tchain_edit  tchain_edit       [.] main
            |
            ---main
               __libc_start_main

    98.82%     0.00%  tchain_edit  tchain_edit       [.] f1
            |
            ---f1
               main
               __libc_start_main

    98.82%    98.28%  tchain_edit  tchain_edit       [.] f3
            |
            ---f3
               |
               |--0.53%-- f2
               |          f1
               |          main
               |          __libc_start_main
               |
               |--0.01%-- f1
               |          main
               |          __libc_start_main
                --99.46%-- [...]
    97.63%     0.03%  tchain_edit  tchain_edit       [.] f2
            |
            ---f2
               f1
               main
               __libc_start_main

     7.13%     0.03%  tchain_edit  [kernel.vmlinux]  [k] do_nmi
            |
            ---do_nmi
               end_repeat_nmi
               f3
               f2
               f1
               main
               __libc_start_main

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/Documentation/perf-record.txt |  4 ++++
 tools/perf/util/evsel.c                  | 24 +++++++++++++++++++++++-
 tools/perf/util/parse-events.c           | 18 ++++++++++++++++++
 tools/perf/util/parse-events.h           |  2 ++
 tools/perf/util/parse-events.l           |  2 ++
 5 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index df47907..f478dc2 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -52,6 +52,10 @@ OPTIONS
 	  - 'time': Disable/enable time stamping. Acceptable values are 1 for
 		    enabling time stamping. 0 for disabling time stamping.
 		    The default is 1.
+	  - 'callgraph': Disable/enable callgraph. Acceptable str are "fp" for
+			 FP mode, "dwarf" for DWARF mode, "lbr" for LBR mode and
+			 "no" for disable callgraph.
+	  - 'stack_size': user stack size for dwarf mode
 	  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 1d58330..f6b0a2d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -620,6 +620,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	int track = evsel->tracking;
 	bool per_cpu = opts->target.default_per_cpu && !opts->target.per_thread;
 	bool sample_time = opts->sample_time;
+	bool callgraph = callchain_param.enabled;
 
 	attr->sample_id_all = perf_missing_features.sample_id_all ? 0 : 1;
 	attr->inherit	    = !opts->no_inherit;
@@ -636,6 +637,23 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 			sample_time = false;
 	}
 
+	if (!opts->callgraph_set) {
+		if (attr->sample_type & PERF_SAMPLE_CALLCHAIN) {
+			callgraph = true;
+			if (attr->sample_type & PERF_SAMPLE_STACK_USER) {
+				callchain_param.record_mode = CALLCHAIN_DWARF;
+				if (attr->sample_stack_user)
+					callchain_param.dump_size = attr->sample_stack_user;
+				else
+					callchain_param.dump_size = 8192;
+			} else if (attr->sample_type & PERF_SAMPLE_BRANCH_STACK)
+				callchain_param.record_mode = CALLCHAIN_LBR;
+			else
+				callchain_param.record_mode = CALLCHAIN_FP;
+		} else
+			callgraph = false;
+	}
+
 	/*
 	 * Event parsing doesn't check the availability
 	 * Clear the bit which event parsing may be set.
@@ -650,6 +668,10 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 		evsel->sample_size -= sizeof(u64);
 	}
 
+	attr->sample_type &= ~(PERF_SAMPLE_CALLCHAIN |
+			       PERF_SAMPLE_STACK_USER |
+			       PERF_SAMPLE_BRANCH_STACK);
+
 	perf_evsel__set_sample_bit(evsel, IP);
 	perf_evsel__set_sample_bit(evsel, TID);
 
@@ -715,7 +737,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	if (perf_evsel__is_function_event(evsel))
 		evsel->attr.exclude_callchain_user = 1;
 
-	if (callchain_param.enabled && !evsel->no_aux_samples)
+	if (callgraph && !evsel->no_aux_samples)
 		perf_evsel__config_callgraph(evsel, opts);
 
 	if (opts->sample_intr_regs) {
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9510047..14b5e56 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -19,6 +19,7 @@
 #include "thread_map.h"
 #include "cpumap.h"
 #include "asm/bug.h"
+#include "callchain.h"
 
 #define MAX_NAME_LEN 100
 
@@ -605,6 +606,23 @@ do {									   \
 		if (term->val.num == 1)
 			attr->sample_type |= PERF_SAMPLE_TIME;
 		 break;
+	case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
+		CHECK_TYPE_VAL(STR);
+		if (!strcmp(term->val.str, "fp"))
+			attr->sample_type |= PERF_SAMPLE_CALLCHAIN;
+		else if (!strcmp(term->val.str, "dwarf"))
+			attr->sample_type |= PERF_SAMPLE_CALLCHAIN |
+					     PERF_SAMPLE_STACK_USER;
+		else if (!strcmp(term->val.str, "lbr"))
+			attr->sample_type |= PERF_SAMPLE_CALLCHAIN |
+					     PERF_SAMPLE_BRANCH_STACK;
+		else if (strcmp(term->val.str, "no"))
+			return -EINVAL;
+		break;
+	case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
+		CHECK_TYPE_VAL(NUM);
+		attr->sample_stack_user = term->val.num;
+		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 0d8cae3..63fb763 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -63,6 +63,8 @@ enum {
 	PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD,
 	PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE,
 	PARSE_EVENTS__TERM_TYPE_TIME,
+	PARSE_EVENTS__TERM_TYPE_CALLGRAPH,
+	PARSE_EVENTS__TERM_TYPE_STACKSIZE,
 };
 
 struct parse_events_term {
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index f542750..16af73b 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -184,6 +184,8 @@ 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); }
+callgraph		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CALLGRAPH); }
+stack_size		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_TIME); }
 ,			{ return ','; }
 "/"			{ BEGIN(INITIAL); return '/'; }
 {name_minus}		{ return str(yyscanner, PE_NAME); }
-- 
1.8.3.1

[PATCH RFC V3 5/5] perf,tests: Add tests to callgrap and time parse

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

Add tests in tests/parse-events.c to check callgrap and time option

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/tests/parse-events.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index d76963f..d6f9447 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -471,6 +471,29 @@ static int test__checkevent_pmu_name(struct perf_evlist *evlist)
 	return 0;
 }
 
+static int test__checkevent_pmu_partial_time_callgraph(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel = perf_evlist__first(evlist);
+
+	/* cpu/config=1,callgraph=fp,time,period=100000/ */
+	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->nr_entries);
+	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->attr.type);
+	TEST_ASSERT_VAL("wrong config",  1 == evsel->attr.config);
+	TEST_ASSERT_VAL("wrong period",  100000 == evsel->attr.sample_period);
+	TEST_ASSERT_VAL("wrong callgraph",  PERF_SAMPLE_CALLCHAIN & evsel->attr.sample_type);
+	TEST_ASSERT_VAL("wrong time",  PERF_SAMPLE_TIME & evsel->attr.sample_type);
+
+	/* cpu/config=2,callgraph=no,time=0,period=2000/ */
+	evsel = perf_evsel__next(evsel);
+	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->attr.type);
+	TEST_ASSERT_VAL("wrong config",  2 == evsel->attr.config);
+	TEST_ASSERT_VAL("wrong period",  2000 == evsel->attr.sample_period);
+	TEST_ASSERT_VAL("wrong callgraph",  !(PERF_SAMPLE_CALLCHAIN & evsel->attr.sample_type));
+	TEST_ASSERT_VAL("wrong time",  !(PERF_SAMPLE_TIME & evsel->attr.sample_type));
+
+	return 0;
+}
+
 static int test__checkevent_pmu_events(struct perf_evlist *evlist)
 {
 	struct perf_evsel *evsel = perf_evlist__first(evlist);
@@ -1547,6 +1570,11 @@ static struct evlist_test test__events_pmu[] = {
 		.check = test__checkevent_pmu_name,
 		.id    = 1,
 	},
+	{
+		.name  = "cpu/config=1,callgraph=fp,time,period=100000/,cpu/config=2,callgraph=no,time=0,period=2000/",
+		.check = test__checkevent_pmu_partial_time_callgraph,
+		.id    = 2,
+	},
 };
 
 struct terms_test {
-- 
1.8.3.1

Re: [PATCH RFC V3 3/5] perf,tool: partial time support

[Namhyung Kim]

Hi,

(CC-ing Adrian)

On Wed, Jul 08, 2015 at 04:44:55AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@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.liang@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
> 

RE: [PATCH RFC V3 3/5] perf,tool: partial time support

[Liang, Kan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1520 bytes --]



> Hi,
> 
> (CC-ing Adrian)
> 
> On Wed, Jul 08, 2015 at 04:44:55AM -0400, kan.liang@intel.com wrote:
> > From: Kan Liang <kan.liang@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..
>

If the sample_type is different, IDENTIFIER will be set to make sure id is in
a fixed position. So we don't need to worry about evlist->id_pos here.

I think the only unsupported case should be that the kernel doesn't
support IDENTIFIER.  If so, under my test, perf record will print warning
" failed to process type". Perf report will also print warning
" non matching sample_type". So the user can switch back to full time
support.
If you think the warning is not good enough, we can show specific
warning and error out when non IDENTIFIER support is detected.

Thanks,
Kan
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH RFC V3 3/5] perf,tool: partial time support

[Namhyung Kim]

On Mon, Jul 13, 2015 at 07:01:51PM +0000, Liang, Kan wrote:
> > On Wed, Jul 08, 2015 at 04:44:55AM -0400, kan.liang@intel.com wrote:
> > > From: Kan Liang <kan.liang@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..
> >
> 
> If the sample_type is different, IDENTIFIER will be set to make sure id is in
> a fixed position. So we don't need to worry about evlist->id_pos here.
> 
> I think the only unsupported case should be that the kernel doesn't
> support IDENTIFIER.  If so, under my test, perf record will print warning
> " failed to process type". Perf report will also print warning
> " non matching sample_type". So the user can switch back to full time
> support.

Ah, ok then.


> If you think the warning is not good enough, we can show specific
> warning and error out when non IDENTIFIER support is detected.

Well, I think it's good enough. :)

However, perf_evlist__id_hdr_size() seems to be affected also.  Are
you sure it's okay too?

Thanks,
Namhyung

Re: [PATCH RFC V3 3/5] perf,tool: partial time support

[Jiri Olsa]

On Wed, Jul 08, 2015 at 04:44:55AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@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.

[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

Re: [PATCH RFC V3 1/5] perf,tools: introduce OPT_CALLBACK_SET/OPT_CALLBACK_NOOPT_SET

[Jiri Olsa]

On Wed, Jul 08, 2015 at 04:44:53AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> This patch extends the OPT_CALLBACK/OPT_CALLBACK_NOOPT to support 'set'
> item of struct option. So the perf knows whether an option was set by
> user.
> The new macros are used by call-graph options.
> 
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> ---
>  tools/perf/builtin-record.c     | 4 ++--
>  tools/perf/perf.h               | 1 +
>  tools/perf/util/parse-options.c | 2 ++
>  tools/perf/util/parse-options.h | 4 ++++
>  4 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 283fe96..a6eb24e 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1018,10 +1018,10 @@ struct option __record_options[] = {
>  		     record__parse_mmap_pages),
>  	OPT_BOOLEAN(0, "group", &record.opts.group,
>  		    "put the counters into a counter group"),
> -	OPT_CALLBACK_NOOPT('g', NULL, &record.opts,
> +	OPT_CALLBACK_NOOPT_SET('g', NULL, &record.opts, &record.opts.callgraph_set,
>  			   NULL, "enables call-graph recording" ,
>  			   &record_callchain_opt),
> -	OPT_CALLBACK(0, "call-graph", &record.opts,
> +	OPT_CALLBACK_SET(0, "call-graph", &record.opts, &record.opts.callgraph_set,
>  		     "mode[,dump_size]", record_callchain_help,
>  		     &record_parse_callchain_opt),

we already provide &record.opts and callback which can set
record.opts.callgraph_set.. seems redundant do I miss some
other benefit?

jirka

>  	OPT_INCR('v', "verbose", &verbose,
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 937b16a..9ba02e0 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -52,6 +52,7 @@ struct record_opts {
>  	bool	     sample_weight;
>  	bool	     sample_time;
>  	bool	     sample_time_set;
> +	bool	     callgraph_set;
>  	bool	     period;
>  	bool	     sample_intr_regs;
>  	bool	     running_time;
> diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
> index 01626be..064385f 100644
> --- a/tools/perf/util/parse-options.c
> +++ b/tools/perf/util/parse-options.c
> @@ -140,6 +140,8 @@ static int get_value(struct parse_opt_ctx_t *p,
>  		return err;
>  
>  	case OPTION_CALLBACK:
> +		if (opt->set)
> +			*(bool *)opt->set = true;
>  		if (unset)
>  			return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
>  		if (opt->flags & PARSE_OPT_NOARG)
> diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
> index 367d8b8..2bec32e 100644
> --- a/tools/perf/util/parse-options.h
> +++ b/tools/perf/util/parse-options.h
> @@ -132,8 +132,12 @@ struct option {
>  	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), .argh = "time", .help = (h), .callback = parse_opt_approxidate_cb }
>  #define OPT_CALLBACK(s, l, v, a, h, f) \
>  	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f) }
> +#define OPT_CALLBACK_SET(s, l, v, os, a, h, f) \
> +	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .set = check_vtype(os, bool *) }
>  #define OPT_CALLBACK_NOOPT(s, l, v, a, h, f) \
>  	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .flags = PARSE_OPT_NOARG }
> +#define OPT_CALLBACK_NOOPT_SET(s, l, v, os, a, h, f) \
> +	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .flags = PARSE_OPT_NOARG, .set = check_vtype(os, bool *) }
>  #define OPT_CALLBACK_DEFAULT(s, l, v, a, h, f, d) \
>  	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .defval = (intptr_t)d, .flags = PARSE_OPT_LASTARG_DEFAULT }
>  #define OPT_CALLBACK_DEFAULT_NOOPT(s, l, v, a, h, f, d) \
> -- 
> 1.8.3.1
> 

Re: [PATCH RFC V3 3/5] perf,tool: partial time support

[Jiri Olsa]

On Wed, Jul 08, 2015 at 04:44:55AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 

SNIP

> +
> +	/*
> +	 * 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);
> +	}

why not use perf_evsel__reset_sample_bit?

> +
>  	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.
>  	 */

SNIP

> 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;

why not use perf_evsel__set_sample_bit?

> +		 break;
>  	case PARSE_EVENTS__TERM_TYPE_NAME:
>  		CHECK_TYPE_VAL(STR);
>  		break;

SNIP

jirka

Re: [PATCH RFC V3 2/5] perf/documentation: Add description for period

[Jiri Olsa]

On Wed, Jul 08, 2015 at 04:44:54AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> 'period' param is not defined in
> /sys/bus/event_sources/devices/<pmu>/format/*
> 
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> ---
>  tools/perf/Documentation/perf-record.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 9b9d9d0..5b47b2c 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -45,6 +45,14 @@ OPTIONS
>            param1 and param2 are defined as formats for the PMU in:
>            /sys/bus/event_sources/devices/<pmu>/format/*
>  
> +	  There are also some params which are not defined in .../<pmu>/format/*.
> +	  These params can be used to set event defaults.
> +	  Here is a list of the params.
> +	  - 'period': Set event sampling period

Acked-by: Jiri Olsa <jolsa@kernel.org>

while at it, could you please add doc also for 'name'?

thanks,
jirka

> +
> +	  Note: If user explicitly sets options which conflict with the params,
> +	  the value set by the params will be overridden.
> +
>          - a hardware breakpoint event in the form of '\mem:addr[/len][:access]'
>            where addr is the address in memory you want to break in.
>            Access is the memory access type (read, write, execute) it can
> -- 
> 1.8.3.1
> 

Re: [PATCH RFC V3 2/5] perf/documentation: Add description for period

[Arnaldo Carvalho de Melo]

Em Tue, Jul 14, 2015 at 08:55:18AM +0200, Jiri Olsa escreveu:
> On Wed, Jul 08, 2015 at 04:44:54AM -0400, kan.liang@intel.com wrote:
> > From: Kan Liang <kan.liang@intel.com>
> > 
> > 'period' param is not defined in
> > /sys/bus/event_sources/devices/<pmu>/format/*
> > 
> > Signed-off-by: Kan Liang <kan.liang@intel.com>
> > ---
> >  tools/perf/Documentation/perf-record.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> > index 9b9d9d0..5b47b2c 100644
> > --- a/tools/perf/Documentation/perf-record.txt
> > +++ b/tools/perf/Documentation/perf-record.txt
> > @@ -45,6 +45,14 @@ OPTIONS
> >            param1 and param2 are defined as formats for the PMU in:
> >            /sys/bus/event_sources/devices/<pmu>/format/*
> >  
> > +	  There are also some params which are not defined in .../<pmu>/format/*.
> > +	  These params can be used to set event defaults.
> > +	  Here is a list of the params.
> > +	  - 'period': Set event sampling period
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thanks, applied.
 
> while at it, could you please add doc also for 'name'?
> 
> thanks,
> jirka
> 
> > +
> > +	  Note: If user explicitly sets options which conflict with the params,
> > +	  the value set by the params will be overridden.
> > +
> >          - a hardware breakpoint event in the form of '\mem:addr[/len][:access]'
> >            where addr is the address in memory you want to break in.
> >            Access is the memory access type (read, write, execute) it can
> > -- 
> > 1.8.3.1
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH RFC V3 3/5] perf,tool: partial time support

[Liang, Kan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 694 bytes --]



> 
> However, perf_evlist__id_hdr_size() seems to be affected also.  Are you
> sure it's okay too?
> 
Yes.
id_hdr_size is calculated according to the first event. It impacts 
perf tool generated COMM, MMAP and FORK events.
But all of these events have same and position fixed IDENTIFIER, 0.
When parsing the events by perf_evlist__event2evsel, id 0 always
return first event.
So only first event will be involved to parse perf tool generated
COMM, MMAP and FORK events. TIME setting from other events
doesn’t impact.

Thanks,
Kan
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH RFC V3 3/5] perf,tool: partial time support

[Namhyung Kim]

Hi,

On Wed, Jul 15, 2015 at 07:03:38PM +0000, Liang, Kan wrote:
> > However, perf_evlist__id_hdr_size() seems to be affected also.  Are you
> > sure it's okay too?
> > 
> Yes.
> id_hdr_size is calculated according to the first event. It impacts 
> perf tool generated COMM, MMAP and FORK events.
> But all of these events have same and position fixed IDENTIFIER, 0.
> When parsing the events by perf_evlist__event2evsel, id 0 always
> return first event.
> So only first event will be involved to parse perf tool generated
> COMM, MMAP and FORK events. TIME setting from other events
> doesn’t impact.

Good.  Thanks for the explanation..

Thanks,
Namhyung

[tip:perf/core] perf record: Document setting '-e pmu/period=N/' in man page

[tip-bot for Kan Liang]

Commit-ID:  3d5d68aabf4ff47cfb447ad90448ce50531be7f5
Gitweb:     http://git.kernel.org/tip/3d5d68aabf4ff47cfb447ad90448ce50531be7f5
Author:     Kan Liang <kan.liang@intel.com>
AuthorDate: Wed, 8 Jul 2015 04:44:54 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 14 Jul 2015 07:52:00 -0300

perf record: Document setting '-e pmu/period=N/' in man page

The 'period' param is not defined in
/sys/bus/event_sources/devices/<pmu>/format/*, but can be used, document
it.

Signed-off-by: Kan Liang <kan.liang@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1436345097-11113-3-git-send-email-kan.liang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 9b9d9d0..5b47b2c 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -45,6 +45,14 @@ OPTIONS
           param1 and param2 are defined as formats for the PMU in:
           /sys/bus/event_sources/devices/<pmu>/format/*
 
+	  There are also some params which are not defined in .../<pmu>/format/*.
+	  These params can be used to set event defaults.
+	  Here is a list of the params.
+	  - 'period': Set event sampling period
+
+	  Note: If user explicitly sets options which conflict with the params,
+	  the value set by the params will be overridden.
+
         - a hardware breakpoint event in the form of '\mem:addr[/len][:access]'
           where addr is the address in memory you want to break in.
           Access is the memory access type (read, write, execute) it can