2015-07-16 15:40:43

by Liang, Kan

[permalink] [raw]
Subject: [PATCH RFC V4 0/4] per event callgrap and time support

From: Kan Liang <[email protected]>

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

Changes since V3:
- Replace OPT_CALLBACK_SET by current existing callback mechanism.
- Using perf_evsel__set_sample_bit if possible
- Change the expression "partial" to "per event"
- Using global variable to indicate if 'time' is set per event.
If 'time' is not set, enable it by default for perf record.

Kan Liang (4):
perf,tools: introduce callgraph_set for callgraph option
perf,tool: per-event time support
perf,tool: per-event callgrap support
perf,tests: Add tests to callgrap and time parse

tools/perf/Documentation/perf-record.txt | 8 ++++-
tools/perf/builtin-record.c | 13 ++++++--
tools/perf/builtin-trace.c | 1 +
tools/perf/perf.h | 1 +
tools/perf/tests/parse-events.c | 28 +++++++++++++++++
tools/perf/util/evsel.c | 54 ++++++++++++++++++++++++++++++--
tools/perf/util/parse-events.c | 29 +++++++++++++++++
tools/perf/util/parse-events.h | 5 +++
tools/perf/util/parse-events.l | 3 ++
tools/perf/util/pmu.c | 3 +-
10 files changed, 137 insertions(+), 8 deletions(-)

--
1.8.3.1


2015-07-16 15:41:41

by Liang, Kan

[permalink] [raw]
Subject: [PATCH RFC V4 1/4] perf,tools: introduce callgraph_set for callgraph option

From: Kan Liang <[email protected]>

Introduce callgraph_set to indicate whether the callgraph option was set
by user.

Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/builtin-record.c | 6 ++++--
tools/perf/perf.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 283fe96..1d40be9 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -762,12 +762,13 @@ static void callchain_debug(void)
callchain_param.dump_size);
}

-int record_parse_callchain_opt(const struct option *opt __maybe_unused,
+int record_parse_callchain_opt(const struct option *opt,
const char *arg,
int unset)
{
int ret;

+ *(bool *)opt->set = true;
callchain_param.enabled = !unset;

/* --no-call-graph */
@@ -784,10 +785,11 @@ int record_parse_callchain_opt(const struct option *opt __maybe_unused,
return ret;
}

-int record_callchain_opt(const struct option *opt __maybe_unused,
+int record_callchain_opt(const struct option *opt,
const char *arg __maybe_unused,
int unset __maybe_unused)
{
+ *(bool *)opt->set = true;
callchain_param.enabled = true;

if (callchain_param.record_mode == CALLCHAIN_NONE)
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;
--
1.8.3.1

2015-07-16 15:40:45

by Liang, Kan

[permalink] [raw]
Subject: [PATCH RFC V4 2/4] perf,tool: per-event time support

From: Kan Liang <[email protected]>

This patchkit adds the ability to turn off time stamps per event.
One usable case of partial time is to work with per-event callgraph to
enable "PEBS threshold > 1" (https://lkml.org/lkml/2015/5/10/196), which
can significantly reduce the sampling overhead.
The event samples with time stamps off will not be ordered.

Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/Documentation/perf-record.txt | 4 +++-
tools/perf/builtin-record.c | 7 ++++++-
tools/perf/builtin-trace.c | 1 +
tools/perf/util/evsel.c | 30 ++++++++++++++++++++++++++++--
tools/perf/util/parse-events.c | 11 +++++++++++
tools/perf/util/parse-events.h | 3 +++
tools/perf/util/parse-events.l | 1 +
tools/perf/util/pmu.c | 2 +-
8 files changed, 54 insertions(+), 5 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/builtin-record.c b/tools/perf/builtin-record.c
index 1d40be9..46ebd92 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -953,7 +953,6 @@ const char * const *record_usage = __record_usage;
*/
static struct record record = {
.opts = {
- .sample_time = true,
.mmap_pages = UINT_MAX,
.user_freq = UINT_MAX,
.user_interval = ULLONG_MAX,
@@ -1136,6 +1135,12 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
goto out_symbol_exit;
}

+ /* If no one set time, let time = true as default */
+ if (!rec->opts.sample_time_set && !time_term_detected) {
+ rec->opts.sample_time = true;
+ rec->opts.sample_time_set = true;
+ }
+
if (rec->opts.target.tid && !rec->opts.no_inherit_set)
rec->opts.no_inherit = true;

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 0ebf55b..469d316 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2899,6 +2899,7 @@ int cmd_trace(int argc, const char **argv, const char *prefix __maybe_unused)
if (trace.trace_pgfaults) {
trace.opts.sample_address = true;
trace.opts.sample_time = true;
+ trace.opts.sample_time_set = true;
}

if (trace.evlist->nr_entries > 0)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 83c0803..34f9cfd 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -619,10 +619,35 @@ 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)
+ perf_evsel__reset_sample_bit(evsel, TIME);
+
perf_evsel__set_sample_bit(evsel, IP);
perf_evsel__set_sample_bit(evsel, TID);

@@ -705,14 +730,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..c9981df 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -25,6 +25,9 @@
#ifdef PARSER_DEBUG
extern int parse_events_debug;
#endif
+
+bool time_term_detected = false;
+
int parse_events_parse(void *data, void *scanner);

static struct perf_pmu_event_symbol *perf_pmu_events_list;
@@ -598,6 +601,14 @@ 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;
+ time_term_detected = true;
+ 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..1083478 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -22,6 +22,8 @@ struct tracepoint_path {
struct tracepoint_path *next;
};

+extern bool time_term_detected;
+
extern struct tracepoint_path *tracepoint_id_to_path(u64 config);
extern struct tracepoint_path *tracepoint_name_to_path(const char *name);
extern bool have_tracepoints(struct list_head *evlist);
@@ -62,6 +64,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); }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 7bcb8c3..b615cdf 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -607,7 +607,7 @@ static char *formats_error_string(struct list_head *formats)
{
struct perf_pmu_format *format;
char *err, *str;
- static const char *static_terms = "config,config1,config2,name,period,branch_type\n";
+ static const char *static_terms = "config,config1,config2,name,period,branch_type,time\n";
unsigned i = 0;

if (!asprintf(&str, "valid terms:"))
--
1.8.3.1

2015-07-16 15:40:46

by Liang, Kan

[permalink] [raw]
Subject: [PATCH RFC V4 3/4] perf,tool: per-event callgrap support

From: Kan Liang <[email protected]>

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 stamp
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 <[email protected]>
---
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 ++
tools/perf/util/pmu.c | 3 ++-
6 files changed, 51 insertions(+), 2 deletions(-)

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 34f9cfd..c253ca7 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.
@@ -648,6 +666,10 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
if (attr->sample_type & PERF_SAMPLE_TIME)
perf_evsel__reset_sample_bit(evsel, TIME);

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

@@ -713,7 +735,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 c9981df..1a8ed26 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

@@ -609,6 +610,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 1083478..47136e5 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -65,6 +65,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); }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index b615cdf..586b9fd 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -607,7 +607,8 @@ static char *formats_error_string(struct list_head *formats)
{
struct perf_pmu_format *format;
char *err, *str;
- static const char *static_terms = "config,config1,config2,name,period,branch_type,time\n";
+ static const char *static_terms = "config,config1,config2,name,period,"
+ "branch_type,time,callgraph,stack_size\n";
unsigned i = 0;

if (!asprintf(&str, "valid terms:"))
--
1.8.3.1

2015-07-16 15:41:17

by Liang, Kan

[permalink] [raw]
Subject: [PATCH RFC V4 4/4] perf,tests: Add tests to callgrap and time parse

From: Kan Liang <[email protected]>

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

Signed-off-by: Kan Liang <[email protected]>
---
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

2015-07-17 09:24:08

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH RFC V4 1/4] perf,tools: introduce callgraph_set for callgraph option

On Thu, Jul 16, 2015 at 04:26:07AM -0400, [email protected] wrote:
> From: Kan Liang <[email protected]>
>
> Introduce callgraph_set to indicate whether the callgraph option was set
> by user.
>
> Signed-off-by: Kan Liang <[email protected]>
> ---
> tools/perf/builtin-record.c | 6 ++++--
> tools/perf/perf.h | 1 +
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 283fe96..1d40be9 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -762,12 +762,13 @@ static void callchain_debug(void)
> callchain_param.dump_size);
> }
>
> -int record_parse_callchain_opt(const struct option *opt __maybe_unused,
> +int record_parse_callchain_opt(const struct option *opt,
> const char *arg,
> int unset)
> {
> int ret;
>
> + *(bool *)opt->set = true;
> callchain_param.enabled = !unset;
>
> /* --no-call-graph */
> @@ -784,10 +785,11 @@ int record_parse_callchain_opt(const struct option *opt __maybe_unused,
> return ret;
> }
>
> -int record_callchain_opt(const struct option *opt __maybe_unused,
> +int record_callchain_opt(const struct option *opt,
> const char *arg __maybe_unused,
> int unset __maybe_unused)
> {
> + *(bool *)opt->set = true;

hum, how does this set callgraph_set ?
shouldn't it be 'callgraph_set = true' instead?

jirka

> callchain_param.enabled = true;
>
> if (callchain_param.record_mode == CALLCHAIN_NONE)
> 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;
> --
> 1.8.3.1
>

2015-07-17 14:24:48

by Liang, Kan

[permalink] [raw]
Subject: RE: [PATCH RFC V4 1/4] perf,tools: introduce callgraph_set for callgraph option



> > @@ -784,10 +785,11 @@ int record_parse_callchain_opt(const struct
> option *opt __maybe_unused,
> > return ret;
> > }
> >
> > -int record_callchain_opt(const struct option *opt __maybe_unused,
> > +int record_callchain_opt(const struct option *opt,
> > const char *arg __maybe_unused,
> > int unset __maybe_unused)
> > {
> > + *(bool *)opt->set = true;
>
> hum, how does this set callgraph_set ?
> shouldn't it be 'callgraph_set = true' instead?
>

Right, I mixed the patch with the previous one.
It should be as below.
I will fix it in next version.

Thanks,
Kan
@@ -789,7 +790,9 @@ int record_callchain_opt(const struct option *opt,
const char *arg __maybe_unused,
int unset __maybe_unused)
{
- *(bool *)opt->set = true;
+ struct record_opts *record = (struct record_opts *)opt->value;
+
+ record->callgraph_set = true;
callchain_param.enabled = true;

if (callchain_param.record_mode == CALLCHAIN_NONE)