hi,
changes for using metric result in another metric seem
to change lot of core metric code, so it's better we
have some more tests before we do that.
v2 changes:
- some of the patches got accepted
- add missing free to patch 1 [Ian]
- factor pmu-events test functions and reuse it in the new test [Ian]
- add fake_pmu bool to parse_events interface [Ian]
- simplify metric tests
- use proper cover letter subject ;-)
I actually reworked the 2 patches Ian acked so far,
so I did not add them.
Also available in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/metric_test
thanks,
jirka
---
Jiri Olsa (13):
perf tools: Add fake pmu support
perf tools: Add fake_pmu bool to parse_events interface
perf tests: Factor check_parse_id function
perf tests: Add another metric parsing test
perf tools: Factor out parse_groups function
perf tools: Add fake_pmu to parse_events function
perf tools: Add map to parse_events function
perf tools: Add metricgroup__parse_groups_test function
perf tools: Factor out prepare_metric function
perf tools: Release metric_events rblist
perf tools: Add test_generic_metric function
perf tests: Add parse metric test for ipc metric
perf tests: Add parse metric test for frontend metric
tools/perf/arch/arm/util/cs-etm.c | 2 +-
tools/perf/arch/arm64/util/arm-spe.c | 2 +-
tools/perf/arch/powerpc/util/kvm-stat.c | 2 +-
tools/perf/arch/x86/tests/intel-cqm.c | 2 +-
tools/perf/arch/x86/tests/perf-time-to-tsc.c | 2 +-
tools/perf/arch/x86/util/intel-bts.c | 2 +-
tools/perf/arch/x86/util/intel-pt.c | 6 ++--
tools/perf/builtin-stat.c | 9 +++---
tools/perf/builtin-trace.c | 4 +--
tools/perf/tests/Build | 1 +
tools/perf/tests/backward-ring-buffer.c | 3 +-
tools/perf/tests/builtin-test.c | 4 +++
tools/perf/tests/code-reading.c | 2 +-
tools/perf/tests/event-times.c | 2 +-
tools/perf/tests/evsel-roundtrip-name.c | 4 +--
tools/perf/tests/hists_cumulate.c | 2 +-
tools/perf/tests/hists_filter.c | 4 +--
tools/perf/tests/hists_link.c | 4 +--
tools/perf/tests/hists_output.c | 2 +-
tools/perf/tests/keep-tracking.c | 4 +--
tools/perf/tests/parse-events.c | 2 +-
tools/perf/tests/parse-metric.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/tests/pmu-events.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
tools/perf/tests/switch-tracking.c | 8 ++---
tools/perf/tests/tests.h | 1 +
tools/perf/util/bpf-loader.c | 2 +-
tools/perf/util/metricgroup.c | 74 ++++++++++++++++++++++++++++++++++++----------
tools/perf/util/metricgroup.h | 10 +++++++
tools/perf/util/parse-events.c | 29 +++++++++++-------
tools/perf/util/parse-events.h | 5 ++--
tools/perf/util/parse-events.l | 8 +++--
tools/perf/util/parse-events.y | 41 ++++++++++++++++++++++++--
tools/perf/util/perf_api_probe.c | 2 +-
tools/perf/util/record.c | 2 +-
tools/perf/util/stat-shadow.c | 67 ++++++++++++++++++++++++++++++------------
tools/perf/util/stat.h | 3 ++
36 files changed, 527 insertions(+), 92 deletions(-)
create mode 100644 tools/perf/tests/parse-metric.c
Adding the way to create pmu event without the actual
PMU being in place. This way we can test metrics defined
for any processors.
The interface is to define fake_pmu in struct parse_events_state
data. It will be used only in tests via special interface
function added in following changes.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/parse-events.c | 14 +++++++++---
tools/perf/util/parse-events.h | 3 ++-
tools/perf/util/parse-events.l | 8 +++++--
tools/perf/util/parse-events.y | 41 ++++++++++++++++++++++++++++++++--
4 files changed, 58 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3decbb203846..d521b38fa677 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1429,6 +1429,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
bool auto_merge_stats,
bool use_alias)
{
+ bool is_fake = parse_state->fake_pmu;
struct perf_event_attr attr;
struct perf_pmu_info info;
struct perf_pmu *pmu;
@@ -1450,7 +1451,14 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
fprintf(stderr, "' that may result in non-fatal errors\n");
}
- pmu = perf_pmu__find(name);
+ if (is_fake) {
+ static struct perf_pmu fake_pmu = { };
+
+ pmu = &fake_pmu;
+ } else {
+ pmu = perf_pmu__find(name);
+ }
+
if (!pmu) {
char *err_str;
@@ -1483,7 +1491,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
}
}
- if (perf_pmu__check_alias(pmu, head_config, &info))
+ if (!is_fake && perf_pmu__check_alias(pmu, head_config, &info))
return -EINVAL;
if (verbose > 1) {
@@ -1516,7 +1524,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
if (pmu->default_config && get_config_chgs(pmu, head_config, &config_terms))
return -ENOMEM;
- if (perf_pmu__config(pmu, &attr, head_config, parse_state->error)) {
+ if (!is_fake && perf_pmu__config(pmu, &attr, head_config, parse_state->error)) {
struct evsel_config_term *pos, *tmp;
list_for_each_entry_safe(pos, tmp, &config_terms, list) {
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 1fe23a2f9b36..9d6846bea6ab 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -127,9 +127,10 @@ struct parse_events_state {
int idx;
int nr_groups;
struct parse_events_error *error;
- struct evlist *evlist;
+ struct evlist *evlist;
struct list_head *terms;
int stoken;
+ bool fake_pmu;
};
void parse_events__handle_error(struct parse_events_error *err, int idx,
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 002802e17059..56912c9641f5 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -129,12 +129,16 @@ do { \
yyless(0); \
} while (0)
-static int pmu_str_check(yyscan_t scanner)
+static int pmu_str_check(yyscan_t scanner, struct parse_events_state *parse_state)
{
YYSTYPE *yylval = parse_events_get_lval(scanner);
char *text = parse_events_get_text(scanner);
yylval->str = strdup(text);
+
+ if (parse_state->fake_pmu)
+ return PE_PMU_EVENT_FAKE;
+
switch (perf_pmu__parse_check(text)) {
case PMU_EVENT_SYMBOL_PREFIX:
return PE_PMU_EVENT_PRE;
@@ -376,7 +380,7 @@ r{num_raw_hex} { return raw(yyscanner); }
{modifier_event} { return str(yyscanner, PE_MODIFIER_EVENT); }
{bpf_object} { if (!isbpf(yyscanner)) { USER_REJECT }; return str(yyscanner, PE_BPF_OBJECT); }
{bpf_source} { if (!isbpf(yyscanner)) { USER_REJECT }; return str(yyscanner, PE_BPF_SOURCE); }
-{name} { return pmu_str_check(yyscanner); }
+{name} { return pmu_str_check(yyscanner, _parse_state); }
{name_tag} { return str(yyscanner, PE_NAME); }
"/" { BEGIN(config); return '/'; }
- { return '-'; }
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index c4ca932d092d..30f623692cf1 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -69,7 +69,7 @@ static void inc_group_count(struct list_head *list,
%token PE_NAME_CACHE_TYPE PE_NAME_CACHE_OP_RESULT
%token PE_PREFIX_MEM PE_PREFIX_RAW PE_PREFIX_GROUP
%token PE_ERROR
-%token PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT
+%token PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT PE_PMU_EVENT_FAKE
%token PE_ARRAY_ALL PE_ARRAY_RANGE
%token PE_DRV_CFG_TERM
%type <num> PE_VALUE
@@ -87,7 +87,7 @@ static void inc_group_count(struct list_head *list,
%type <str> PE_MODIFIER_EVENT
%type <str> PE_MODIFIER_BP
%type <str> PE_EVENT_NAME
-%type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT
+%type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT PE_PMU_EVENT_FAKE
%type <str> PE_DRV_CFG_TERM
%destructor { free ($$); } <str>
%type <term> event_term
@@ -356,6 +356,43 @@ PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc
YYABORT;
$$ = list;
}
+|
+PE_PMU_EVENT_FAKE sep_dc
+{
+ struct list_head *list;
+ int err;
+
+ list = alloc_list();
+ if (!list)
+ YYABORT;
+
+ err = parse_events_add_pmu(_parse_state, list, $1, NULL, false, false);
+ free($1);
+ if (err < 0) {
+ free(list);
+ YYABORT;
+ }
+ $$ = list;
+}
+|
+PE_PMU_EVENT_FAKE opt_pmu_config
+{
+ struct list_head *list;
+ int err;
+
+ list = alloc_list();
+ if (!list)
+ YYABORT;
+
+ err = parse_events_add_pmu(_parse_state, list, $1, $2, false, false);
+ free($1);
+ parse_events_terms__delete($2);
+ if (err < 0) {
+ free(list);
+ YYABORT;
+ }
+ $$ = list;
+}
value_sym:
PE_VALUE_SYM_HW
--
2.25.4
Adding fake_pmu bool argument parse_events interface to
parse events and use fake pmu event in case pmu event
is parsed.
This way it's possible to parse events from PMUs which
are not present in the system. It's available only for
testing purposes coming in following changes, so all
the current users set fake_pmu argument as false.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/arch/arm/util/cs-etm.c | 2 +-
tools/perf/arch/powerpc/util/kvm-stat.c | 2 +-
tools/perf/arch/x86/tests/perf-time-to-tsc.c | 2 +-
tools/perf/arch/x86/util/intel-bts.c | 2 +-
tools/perf/arch/x86/util/intel-pt.c | 6 +++---
tools/perf/builtin-stat.c | 8 ++++----
tools/perf/builtin-trace.c | 4 ++--
tools/perf/tests/backward-ring-buffer.c | 3 ++-
tools/perf/tests/code-reading.c | 2 +-
tools/perf/tests/event-times.c | 2 +-
tools/perf/tests/evsel-roundtrip-name.c | 4 ++--
tools/perf/tests/hists_cumulate.c | 2 +-
tools/perf/tests/hists_filter.c | 4 ++--
tools/perf/tests/hists_link.c | 4 ++--
tools/perf/tests/hists_output.c | 2 +-
tools/perf/tests/keep-tracking.c | 4 ++--
tools/perf/tests/parse-events.c | 2 +-
tools/perf/tests/pmu-events.c | 2 +-
tools/perf/tests/switch-tracking.c | 8 ++++----
tools/perf/util/bpf-loader.c | 2 +-
tools/perf/util/metricgroup.c | 2 +-
tools/perf/util/parse-events.c | 15 ++++++++-------
tools/perf/util/parse-events.h | 2 +-
tools/perf/util/perf_api_probe.c | 2 +-
tools/perf/util/record.c | 2 +-
25 files changed, 46 insertions(+), 44 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index cea5e33d61d2..c99a6528f45c 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -415,7 +415,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
if (opts->full_auxtrace) {
struct evsel *tracking_evsel;
- err = parse_events(evlist, "dummy:u", NULL);
+ err = parse_events(evlist, "dummy:u", NULL, false);
if (err)
goto out;
diff --git a/tools/perf/arch/powerpc/util/kvm-stat.c b/tools/perf/arch/powerpc/util/kvm-stat.c
index eed9e5a42935..396f6bff44db 100644
--- a/tools/perf/arch/powerpc/util/kvm-stat.c
+++ b/tools/perf/arch/powerpc/util/kvm-stat.c
@@ -114,7 +114,7 @@ static int is_tracepoint_available(const char *str, struct evlist *evlist)
int ret;
bzero(&err, sizeof(err));
- ret = parse_events(evlist, str, &err);
+ ret = parse_events(evlist, str, &err, false);
if (err.str)
parse_events_print_error(&err, "tracepoint");
return ret;
diff --git a/tools/perf/arch/x86/tests/perf-time-to-tsc.c b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
index 026d32ed078e..3d821e07119e 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -80,7 +80,7 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest __maybe
perf_evlist__set_maps(&evlist->core, cpus, threads);
- CHECK__(parse_events(evlist, "cycles:u", NULL));
+ CHECK__(parse_events(evlist, "cycles:u", NULL, false));
perf_evlist__config(evlist, &opts, NULL);
diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
index 0dc09b5809c1..2ac433fd542b 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -232,7 +232,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr,
struct evsel *tracking_evsel;
int err;
- err = parse_events(evlist, "dummy:u", NULL);
+ err = parse_events(evlist, "dummy:u", NULL, false);
if (err)
return err;
diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index 839ef52c1ac2..160fa9b135b4 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -419,7 +419,7 @@ static int intel_pt_track_switches(struct evlist *evlist)
if (!perf_evlist__can_select_event(evlist, sched_switch))
return -EPERM;
- err = parse_events(evlist, sched_switch, NULL);
+ err = parse_events(evlist, sched_switch, NULL, false);
if (err) {
pr_debug2("%s: failed to parse %s, error %d\n",
__func__, sched_switch, err);
@@ -796,7 +796,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
if (!cpu_wide && perf_can_record_cpu_wide()) {
struct evsel *switch_evsel;
- err = parse_events(evlist, "dummy:u", NULL);
+ err = parse_events(evlist, "dummy:u", NULL, false);
if (err)
return err;
@@ -854,7 +854,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
if (opts->full_auxtrace) {
struct evsel *tracking_evsel;
- err = parse_events(evlist, "dummy:u", NULL);
+ err = parse_events(evlist, "dummy:u", NULL, false);
if (err)
return err;
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index b2b79aa161dd..8a0c470dd92b 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1533,11 +1533,11 @@ static int add_default_attributes(void)
if (pmu_have_event("cpu", "cycles-ct") &&
pmu_have_event("cpu", "el-start"))
err = parse_events(evsel_list, transaction_attrs,
- &errinfo);
+ &errinfo, false);
else
err = parse_events(evsel_list,
transaction_limited_attrs,
- &errinfo);
+ &errinfo, false);
if (err) {
fprintf(stderr, "Cannot set up transaction events\n");
parse_events_print_error(&errinfo, transaction_attrs);
@@ -1566,7 +1566,7 @@ static int add_default_attributes(void)
pmu_have_event("msr", "smi")) {
if (!force_metric_only)
stat_config.metric_only = true;
- err = parse_events(evsel_list, smi_cost_attrs, &errinfo);
+ err = parse_events(evsel_list, smi_cost_attrs, &errinfo, false);
} else {
fprintf(stderr, "To measure SMI cost, it needs "
"msr/aperf/, msr/smi/ and cpu/cycles/ support\n");
@@ -1606,7 +1606,7 @@ static int add_default_attributes(void)
if (topdown_attrs[0] && str) {
if (warn)
arch_topdown_group_warn();
- err = parse_events(evsel_list, str, &errinfo);
+ err = parse_events(evsel_list, str, &errinfo, false);
if (err) {
fprintf(stderr,
"Cannot set up top down events %s: %d\n",
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 4cbb64edc998..47585e785f7e 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -3044,7 +3044,7 @@ static bool evlist__add_vfs_getname(struct evlist *evlist)
int ret;
bzero(&err, sizeof(err));
- ret = parse_events(evlist, "probe:vfs_getname*", &err);
+ ret = parse_events(evlist, "probe:vfs_getname*", &err, false);
if (ret) {
free(err.str);
free(err.help);
@@ -4879,7 +4879,7 @@ int cmd_trace(int argc, const char **argv)
struct parse_events_error parse_err;
bzero(&parse_err, sizeof(parse_err));
- err = parse_events(trace.evlist, trace.perfconfig_events, &parse_err);
+ err = parse_events(trace.evlist, trace.perfconfig_events, &parse_err, false);
if (err) {
parse_events_print_error(&parse_err, trace.perfconfig_events);
goto out;
diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index 15cea518f5ad..d08be4576e61 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -120,7 +120,8 @@ int test__backward_ring_buffer(struct test *test __maybe_unused, int subtest __m
* Set backward bit, ring buffer should be writing from end. Record
* it in aux evlist
*/
- err = parse_events(evlist, "syscalls:sys_enter_prctl/overwrite/", &parse_error);
+ err = parse_events(evlist, "syscalls:sys_enter_prctl/overwrite/",
+ &parse_error, false);
if (err) {
pr_debug("Failed to parse tracepoint event, try use root\n");
ret = TEST_SKIP;
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 6fe221d31f07..a87a0ffe3705 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -645,7 +645,7 @@ static int do_test_code_reading(bool try_kcore)
str = do_determine_event(excl_kernel);
pr_debug("Parsing event '%s'\n", str);
- ret = parse_events(evlist, str, NULL);
+ ret = parse_events(evlist, str, NULL, false);
if (ret < 0) {
pr_debug("parse_events failed\n");
goto out_put;
diff --git a/tools/perf/tests/event-times.c b/tools/perf/tests/event-times.c
index db68894a6f40..15773d384747 100644
--- a/tools/perf/tests/event-times.c
+++ b/tools/perf/tests/event-times.c
@@ -174,7 +174,7 @@ static int test_times(int (attach)(struct evlist *),
goto out_err;
}
- err = parse_events(evlist, "cpu-clock:u", NULL);
+ err = parse_events(evlist, "cpu-clock:u", NULL, false);
if (err) {
pr_debug("failed to parse event cpu-clock:u\n");
goto out_err;
diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
index f7f3e5b4c180..9154e068539e 100644
--- a/tools/perf/tests/evsel-roundtrip-name.c
+++ b/tools/perf/tests/evsel-roundtrip-name.c
@@ -25,7 +25,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
__evsel__hw_cache_type_op_res_name(type, op, i, name, sizeof(name));
- err = parse_events(evlist, name, NULL);
+ err = parse_events(evlist, name, NULL, false);
if (err)
ret = err;
}
@@ -72,7 +72,7 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names)
return -ENOMEM;
for (i = 0; i < nr_names; ++i) {
- err = parse_events(evlist, names[i], NULL);
+ err = parse_events(evlist, names[i], NULL, false);
if (err) {
pr_debug("failed to parse event '%s', err %d\n",
names[i], err);
diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index 3f2e1a581247..1ad9052cbdd5 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -706,7 +706,7 @@ int test__hists_cumulate(struct test *test __maybe_unused, int subtest __maybe_u
TEST_ASSERT_VAL("No memory", evlist);
- err = parse_events(evlist, "cpu-clock", NULL);
+ err = parse_events(evlist, "cpu-clock", NULL, false);
if (err)
goto out;
err = TEST_FAIL;
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 123e07d35b55..49828b40bcad 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -111,10 +111,10 @@ int test__hists_filter(struct test *test __maybe_unused, int subtest __maybe_unu
TEST_ASSERT_VAL("No memory", evlist);
- err = parse_events(evlist, "cpu-clock", NULL);
+ err = parse_events(evlist, "cpu-clock", NULL, false);
if (err)
goto out;
- err = parse_events(evlist, "task-clock", NULL);
+ err = parse_events(evlist, "task-clock", NULL, false);
if (err)
goto out;
err = TEST_FAIL;
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index a024d3f3a412..1760d0defdfe 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -276,10 +276,10 @@ int test__hists_link(struct test *test __maybe_unused, int subtest __maybe_unuse
if (evlist == NULL)
return -ENOMEM;
- err = parse_events(evlist, "cpu-clock", NULL);
+ err = parse_events(evlist, "cpu-clock", NULL, false);
if (err)
goto out;
- err = parse_events(evlist, "task-clock", NULL);
+ err = parse_events(evlist, "task-clock", NULL, false);
if (err)
goto out;
diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index 8973f35df604..82773e977ba3 100644
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -593,7 +593,7 @@ int test__hists_output(struct test *test __maybe_unused, int subtest __maybe_unu
TEST_ASSERT_VAL("No memory", evlist);
- err = parse_events(evlist, "cpu-clock", NULL);
+ err = parse_events(evlist, "cpu-clock", NULL, false);
if (err)
goto out;
err = TEST_FAIL;
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 50a0c9fcde7d..d0334a2e1a8e 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -89,8 +89,8 @@ int test__keep_tracking(struct test *test __maybe_unused, int subtest __maybe_un
perf_evlist__set_maps(&evlist->core, cpus, threads);
- CHECK__(parse_events(evlist, "dummy:u", NULL));
- CHECK__(parse_events(evlist, "cycles:u", NULL));
+ CHECK__(parse_events(evlist, "dummy:u", NULL, false));
+ CHECK__(parse_events(evlist, "cycles:u", NULL, false));
perf_evlist__config(evlist, &opts, NULL);
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 895188b63f96..0f6c6b246677 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1797,7 +1797,7 @@ static int test_event(struct evlist_test *e)
if (evlist == NULL)
return -ENOMEM;
- ret = parse_events(evlist, e->name, &err);
+ ret = parse_events(evlist, e->name, &err, false);
if (ret) {
pr_debug("failed to parse event '%s', err %d, str '%s'\n",
e->name, ret, err.str);
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index ab64b4a4e284..894282be073f 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -402,7 +402,7 @@ static int check_parse_id(const char *id, bool same_cpu, struct pmu_event *pe)
evlist = evlist__new();
memset(&error, 0, sizeof(error));
- ret = parse_events(evlist, id, &error);
+ ret = parse_events(evlist, id, &error, false);
if (ret && same_cpu) {
pr_warning("Parse event failed metric '%s' id '%s' expr '%s'\n",
pe->metric_name, id, pe->metric_expr);
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index db5e1f70053a..2628de39d82d 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -362,7 +362,7 @@ int test__switch_tracking(struct test *test __maybe_unused, int subtest __maybe_
perf_evlist__set_maps(&evlist->core, cpus, threads);
/* First event */
- err = parse_events(evlist, "cpu-clock:u", NULL);
+ err = parse_events(evlist, "cpu-clock:u", NULL, false);
if (err) {
pr_debug("Failed to parse event dummy:u\n");
goto out_err;
@@ -371,7 +371,7 @@ int test__switch_tracking(struct test *test __maybe_unused, int subtest __maybe_
cpu_clocks_evsel = evlist__last(evlist);
/* Second event */
- err = parse_events(evlist, "cycles:u", NULL);
+ err = parse_events(evlist, "cycles:u", NULL, false);
if (err) {
pr_debug("Failed to parse event cycles:u\n");
goto out_err;
@@ -386,7 +386,7 @@ int test__switch_tracking(struct test *test __maybe_unused, int subtest __maybe_
goto out;
}
- err = parse_events(evlist, sched_switch, NULL);
+ err = parse_events(evlist, sched_switch, NULL, false);
if (err) {
pr_debug("Failed to parse event %s\n", sched_switch);
goto out_err;
@@ -416,7 +416,7 @@ int test__switch_tracking(struct test *test __maybe_unused, int subtest __maybe_
evsel__set_sample_bit(cycles_evsel, TIME);
/* Fourth event */
- err = parse_events(evlist, "dummy:u", NULL);
+ err = parse_events(evlist, "dummy:u", NULL, false);
if (err) {
pr_debug("Failed to parse event dummy:u\n");
goto out_err;
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 2feb751516ab..98307b14def1 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -1560,7 +1560,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
if (asprintf(&event_definition, "bpf-output/no-inherit=1,name=%s/", name) < 0)
return ERR_PTR(-ENOMEM);
- err = parse_events(evlist, event_definition, NULL);
+ err = parse_events(evlist, event_definition, NULL, false);
free(event_definition);
if (err) {
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 9e21aa767e41..9a90afb4428c 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -729,7 +729,7 @@ int metricgroup__parse_groups(const struct option *opt,
return ret;
pr_debug("adding %s\n", extra_events.buf);
bzero(&parse_error, sizeof(parse_error));
- ret = parse_events(perf_evlist, extra_events.buf, &parse_error);
+ ret = parse_events(perf_evlist, extra_events.buf, &parse_error, false);
if (ret) {
parse_events_print_error(&parse_error, extra_events.buf);
goto out;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d521b38fa677..ec146a6f057a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2097,14 +2097,15 @@ int parse_events_terms(struct list_head *terms, const char *str)
}
int parse_events(struct evlist *evlist, const char *str,
- struct parse_events_error *err)
+ struct parse_events_error *err, bool fake_pmu)
{
struct parse_events_state parse_state = {
- .list = LIST_HEAD_INIT(parse_state.list),
- .idx = evlist->core.nr_entries,
- .error = err,
- .evlist = evlist,
- .stoken = PE_START_EVENTS,
+ .list = LIST_HEAD_INIT(parse_state.list),
+ .idx = evlist->core.nr_entries,
+ .error = err,
+ .evlist = evlist,
+ .stoken = PE_START_EVENTS,
+ .fake_pmu = fake_pmu,
};
int ret;
@@ -2232,7 +2233,7 @@ int parse_events_option(const struct option *opt, const char *str,
int ret;
bzero(&err, sizeof(err));
- ret = parse_events(evlist, str, &err);
+ ret = parse_events(evlist, str, &err, false);
if (ret) {
parse_events_print_error(&err, str);
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 9d6846bea6ab..1864b784a587 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -34,7 +34,7 @@ const char *event_type(int type);
int parse_events_option(const struct option *opt, const char *str, int unset);
int parse_events_option_new_evlist(const struct option *opt, const char *str, int unset);
int parse_events(struct evlist *evlist, const char *str,
- struct parse_events_error *error);
+ struct parse_events_error *error, bool fake_pmu);
int parse_events_terms(struct list_head *terms, const char *str);
int parse_filter(const struct option *opt, const char *str, int unset);
int exclude_perf(const struct option *opt, const char *arg, int unset);
diff --git a/tools/perf/util/perf_api_probe.c b/tools/perf/util/perf_api_probe.c
index 1337965673d7..7657cc57f93b 100644
--- a/tools/perf/util/perf_api_probe.c
+++ b/tools/perf/util/perf_api_probe.c
@@ -23,7 +23,7 @@ static int perf_do_probe_api(setup_probe_fn_t fn, int cpu, const char *str)
if (!evlist)
return -ENOMEM;
- if (parse_events(evlist, str, NULL))
+ if (parse_events(evlist, str, NULL, false))
goto out_delete;
evsel = evlist__first(evlist);
diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
index a4cc11592f6b..bae21b43ac08 100644
--- a/tools/perf/util/record.c
+++ b/tools/perf/util/record.c
@@ -210,7 +210,7 @@ bool perf_evlist__can_select_event(struct evlist *evlist, const char *str)
if (!temp_evlist)
return false;
- err = parse_events(temp_evlist, str, NULL);
+ err = parse_events(temp_evlist, str, NULL, false);
if (err)
goto out_delete;
--
2.25.4
Separating the generic part of check_parse_id function,
so it can be used in following changes for the new test.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/pmu-events.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 894282be073f..f65380d2066f 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -390,9 +390,8 @@ static bool is_number(const char *str)
return errno == 0 && end_ptr != str;
}
-static int check_parse_id(const char *id, bool same_cpu, struct pmu_event *pe)
+static int check_parse_id(const char *id, struct parse_events_error *error)
{
- struct parse_events_error error;
struct evlist *evlist;
int ret;
@@ -401,8 +400,19 @@ static int check_parse_id(const char *id, bool same_cpu, struct pmu_event *pe)
return 0;
evlist = evlist__new();
- memset(&error, 0, sizeof(error));
- ret = parse_events(evlist, id, &error, false);
+ if (!evlist)
+ return -ENOMEM;
+ memset(error, 0, sizeof(*error));
+ ret = parse_events(evlist, id, error, false);
+ evlist__delete(evlist);
+ return ret;
+}
+
+static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
+{
+ struct parse_events_error error;
+
+ int ret = check_parse_id(id, &error);
if (ret && same_cpu) {
pr_warning("Parse event failed metric '%s' id '%s' expr '%s'\n",
pe->metric_name, id, pe->metric_expr);
@@ -413,7 +423,6 @@ static int check_parse_id(const char *id, bool same_cpu, struct pmu_event *pe)
id, pe->metric_name, pe->metric_expr);
ret = 0;
}
- evlist__delete(evlist);
free(error.str);
free(error.help);
free(error.first_str);
@@ -474,7 +483,7 @@ static int test_parsing(void)
expr__add_id(&ctx, strdup(cur->key), k++);
hashmap__for_each_entry((&ctx.ids), cur, bkt) {
- if (check_parse_id(cur->key, map == cpus_map,
+ if (check_parse_cpu(cur->key, map == cpus_map,
pe))
ret++;
}
--
2.25.4
The test goes through all metrics compiled for arch
within pmu events and try to parse them.
This test is different from 'test_parsing' in that
we go through all the events in the current arch,
not just one defined for current CPU model. Using
'fake_pmu' to parse events which do not have PMUs
defined in the system.
Say there's bad change in ivybridge metrics file, like:
--- a/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
+++ b/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
@@ -8,7 +8,7 @@
- "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / (4 * ((
+ "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / / (4 *
the test fails with (on my kabylake laptop):
$ perf test 'Parsing of PMU event table metrics with fake PMUs' -v
parsing 'idq_uops_not_delivered.core / / (4 * (( ( cpu_clk_unh...
syntax error, line 1
expr__parse failed
test child finished with -1
...
The test also defines its own list of metrics and
tries to parse them. It's handy for developing.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/pmu-events.c | 117 +++++++++++++++++++++++++++++++++-
1 file changed, 114 insertions(+), 3 deletions(-)
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index f65380d2066f..d3343827eb4d 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -390,7 +390,8 @@ static bool is_number(const char *str)
return errno == 0 && end_ptr != str;
}
-static int check_parse_id(const char *id, struct parse_events_error *error)
+static int check_parse_id(const char *id, struct parse_events_error *error,
+ bool fake_pmu)
{
struct evlist *evlist;
int ret;
@@ -403,7 +404,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error)
if (!evlist)
return -ENOMEM;
memset(error, 0, sizeof(*error));
- ret = parse_events(evlist, id, error, false);
+ ret = parse_events(evlist, id, error, fake_pmu);
evlist__delete(evlist);
return ret;
}
@@ -412,7 +413,7 @@ static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
{
struct parse_events_error error;
- int ret = check_parse_id(id, &error);
+ int ret = check_parse_id(id, &error, false);
if (ret && same_cpu) {
pr_warning("Parse event failed metric '%s' id '%s' expr '%s'\n",
pe->metric_name, id, pe->metric_expr);
@@ -430,6 +431,18 @@ static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
return ret;
}
+static int check_parse_fake(const char *id)
+{
+ struct parse_events_error error;
+ int ret = check_parse_id(id, &error, true);
+
+ free(error.str);
+ free(error.help);
+ free(error.first_str);
+ free(error.first_help);
+ return ret;
+}
+
static void expr_failure(const char *msg,
const struct pmu_events_map *map,
const struct pmu_event *pe)
@@ -499,6 +512,100 @@ static int test_parsing(void)
return ret == 0 ? TEST_OK : TEST_SKIP;
}
+struct test_metric {
+ const char *str;
+};
+
+static struct test_metric metrics[] = {
+ { "(unc_p_power_state_occupancy.cores_c0 / unc_p_clockticks) * 100." },
+ { "imx8_ddr0@read\\-cycles@ * 4 * 4", },
+ { "imx8_ddr0@axid\\-read\\,axi_mask\\=0xffff\\,axi_id\\=0x0000@ * 4", },
+ { "(cstate_pkg@c2\\-residency@ / msr@tsc@) * 100", },
+ { "(imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@)", },
+};
+
+static int metric_parse_fake(const char *str)
+{
+ struct expr_parse_ctx ctx;
+ struct hashmap_entry *cur;
+ double result;
+ int ret = -1;
+ size_t bkt;
+ int i;
+
+ pr_debug("parsing '%s'\n", str);
+
+ expr__ctx_init(&ctx);
+ if (expr__find_other(str, NULL, &ctx, 0) < 0) {
+ pr_err("expr__find_other failed\n");
+ return -1;
+ }
+
+ /*
+ * Add all ids with a made up value. The value may
+ * trigger divide by zero when subtracted and so try to
+ * make them unique.
+ */
+ i = 1;
+ hashmap__for_each_entry((&ctx.ids), cur, bkt)
+ expr__add_id(&ctx, strdup(cur->key), i++);
+
+ hashmap__for_each_entry((&ctx.ids), cur, bkt) {
+ if (check_parse_fake(cur->key)) {
+ pr_err("check_parse_fake failed\n");
+ goto out;
+ }
+ }
+
+ if (expr__parse(&result, &ctx, str, 1))
+ pr_err("expr__parse failed\n");
+ else
+ ret = 0;
+
+out:
+ expr__ctx_clear(&ctx);
+ return ret;
+}
+
+/*
+ * Parse all the metrics for current architecture,
+ * or all defined cpus via the 'fake_pmu' bool
+ * in parse_events.
+ */
+static int test_parsing_fake(void)
+{
+ struct pmu_events_map *map;
+ struct pmu_event *pe;
+ unsigned int i, j;
+ int err = 0;
+
+ for (i = 0; i < ARRAY_SIZE(metrics); i++) {
+ err = metric_parse_fake(metrics[i].str);
+ if (err)
+ return err;
+ }
+
+ i = 0;
+ for (;;) {
+ map = &pmu_events_map[i++];
+ if (!map->table)
+ break;
+ j = 0;
+ for (;;) {
+ pe = &map->table[j++];
+ if (!pe->name && !pe->metric_group && !pe->metric_name)
+ break;
+ if (!pe->metric_expr)
+ continue;
+ err = metric_parse_fake(pe->metric_expr);
+ if (err)
+ return err;
+ }
+ }
+
+ return 0;
+}
+
static const struct {
int (*func)(void);
const char *desc;
@@ -515,6 +622,10 @@ static const struct {
.func = test_parsing,
.desc = "Parsing of PMU event table metrics",
},
+ {
+ .func = test_parsing_fake,
+ .desc = "Parsing of PMU event table metrics with fake PMUs",
+ },
};
const char *test__pmu_events_subtest_get_desc(int subtest)
--
2.25.4
Factor out the parse_groups function, it will be used
for new test interface coming in following changes.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/metricgroup.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 9a90afb4428c..47afe5867f9b 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -709,14 +709,12 @@ static void metricgroup__free_egroups(struct list_head *group_list)
}
}
-int metricgroup__parse_groups(const struct option *opt,
- const char *str,
- bool metric_no_group,
- bool metric_no_merge,
- struct rblist *metric_events)
+static int parse_groups(struct evlist *perf_evlist, const char *str,
+ bool metric_no_group,
+ bool metric_no_merge,
+ struct rblist *metric_events)
{
struct parse_events_error parse_error;
- struct evlist *perf_evlist = *(struct evlist **)opt->value;
struct strbuf extra_events;
LIST_HEAD(group_list);
int ret;
@@ -742,6 +740,18 @@ int metricgroup__parse_groups(const struct option *opt,
return ret;
}
+int metricgroup__parse_groups(const struct option *opt,
+ const char *str,
+ bool metric_no_group,
+ bool metric_no_merge,
+ struct rblist *metric_events)
+{
+ struct evlist *perf_evlist = *(struct evlist **)opt->value;
+
+ return parse_groups(perf_evlist, str, metric_no_group,
+ metric_no_merge, metric_events);
+}
+
bool metricgroup__has_metric(const char *metric)
{
struct pmu_events_map *map = perf_pmu__find_map(NULL);
--
2.25.4
For testing purposes we need to pass our own map of events
from parse_groups through metricgroup__add_metric.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/metricgroup.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index f3e761e14e66..72999aacfa37 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -597,17 +597,14 @@ static int __metricgroup__add_metric(struct list_head *group_list,
static int metricgroup__add_metric(const char *metric, bool metric_no_group,
struct strbuf *events,
- struct list_head *group_list)
+ struct list_head *group_list,
+ struct pmu_events_map *map)
{
- struct pmu_events_map *map = perf_pmu__find_map(NULL);
struct pmu_event *pe;
struct egroup *eg;
int i, ret;
bool has_match = false;
- if (!map)
- return 0;
-
for (i = 0; ; i++) {
pe = &map->table[i];
@@ -668,7 +665,8 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
static int metricgroup__add_metric_list(const char *list, bool metric_no_group,
struct strbuf *events,
- struct list_head *group_list)
+ struct list_head *group_list,
+ struct pmu_events_map *map)
{
char *llist, *nlist, *p;
int ret = -EINVAL;
@@ -683,7 +681,7 @@ static int metricgroup__add_metric_list(const char *list, bool metric_no_group,
while ((p = strsep(&llist, ",")) != NULL) {
ret = metricgroup__add_metric(p, metric_no_group, events,
- group_list);
+ group_list, map);
if (ret == -EINVAL) {
fprintf(stderr, "Cannot find metric or group `%s'\n",
p);
@@ -713,7 +711,8 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
bool metric_no_group,
bool metric_no_merge,
bool fake_pmu,
- struct rblist *metric_events)
+ struct rblist *metric_events,
+ struct pmu_events_map *map)
{
struct parse_events_error parse_error;
struct strbuf extra_events;
@@ -723,7 +722,7 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
if (metric_events->nr_entries == 0)
metricgroup__rblist_init(metric_events);
ret = metricgroup__add_metric_list(str, metric_no_group,
- &extra_events, &group_list);
+ &extra_events, &group_list, map);
if (ret)
return ret;
pr_debug("adding %s\n", extra_events.buf);
@@ -748,9 +747,13 @@ int metricgroup__parse_groups(const struct option *opt,
struct rblist *metric_events)
{
struct evlist *perf_evlist = *(struct evlist **)opt->value;
+ struct pmu_events_map *map = perf_pmu__find_map(NULL);
+
+ if (!map)
+ return 0;
return parse_groups(perf_evlist, str, metric_no_group,
- metric_no_merge, false, metric_events);
+ metric_no_merge, false, metric_events, map);
}
bool metricgroup__has_metric(const char *metric)
--
2.25.4
Adding metricgroup__parse_groups_test function. It will
be used as test's interface to metric parsing in following
changes.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/metricgroup.c | 11 +++++++++++
tools/perf/util/metricgroup.h | 9 +++++++++
2 files changed, 20 insertions(+)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 72999aacfa37..fe33fee7f6ad 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -756,6 +756,17 @@ int metricgroup__parse_groups(const struct option *opt,
metric_no_merge, false, metric_events, map);
}
+int metricgroup__parse_groups_test(struct evlist *evlist,
+ struct pmu_events_map *map,
+ const char *str,
+ bool metric_no_group,
+ bool metric_no_merge,
+ struct rblist *metric_events)
+{
+ return parse_groups(evlist, str, metric_no_group,
+ metric_no_merge, true, metric_events, map);
+}
+
bool metricgroup__has_metric(const char *metric)
{
struct pmu_events_map *map = perf_pmu__find_map(NULL);
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 287850bcdeca..426c824e20bf 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -7,8 +7,10 @@
#include <stdbool.h>
struct evsel;
+struct evlist;
struct option;
struct rblist;
+struct pmu_events_map;
struct metric_event {
struct rb_node nd;
@@ -34,6 +36,13 @@ int metricgroup__parse_groups(const struct option *opt,
bool metric_no_merge,
struct rblist *metric_events);
+int metricgroup__parse_groups_test(struct evlist *evlist,
+ struct pmu_events_map *map,
+ const char *str,
+ bool metric_no_group,
+ bool metric_no_merge,
+ struct rblist *metric_events);
+
void metricgroup__print(bool metrics, bool groups, char *filter,
bool raw, bool details);
bool metricgroup__has_metric(const char *metric);
--
2.25.4
Allow to pass fake_pmu in parse_groups function
so it can be used in parse_events call.
It's set true by metricgroup__parse_groups_test
function.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/metricgroup.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 47afe5867f9b..f3e761e14e66 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -712,6 +712,7 @@ static void metricgroup__free_egroups(struct list_head *group_list)
static int parse_groups(struct evlist *perf_evlist, const char *str,
bool metric_no_group,
bool metric_no_merge,
+ bool fake_pmu,
struct rblist *metric_events)
{
struct parse_events_error parse_error;
@@ -727,7 +728,7 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
return ret;
pr_debug("adding %s\n", extra_events.buf);
bzero(&parse_error, sizeof(parse_error));
- ret = parse_events(perf_evlist, extra_events.buf, &parse_error, false);
+ ret = parse_events(perf_evlist, extra_events.buf, &parse_error, fake_pmu);
if (ret) {
parse_events_print_error(&parse_error, extra_events.buf);
goto out;
@@ -749,7 +750,7 @@ int metricgroup__parse_groups(const struct option *opt,
struct evlist *perf_evlist = *(struct evlist **)opt->value;
return parse_groups(perf_evlist, str, metric_no_group,
- metric_no_merge, metric_events);
+ metric_no_merge, false, metric_events);
}
bool metricgroup__has_metric(const char *metric)
--
2.25.4
Adding new metri test for frontend metric. It's stolen
from x86 pmu events.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/parse-metric.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
index 717a73fa7446..1939e567a8b3 100644
--- a/tools/perf/tests/parse-metric.c
+++ b/tools/perf/tests/parse-metric.c
@@ -17,6 +17,11 @@ static struct pmu_event pme_test[] = {
.metric_expr = "inst_retired.any / cpu_clk_unhalted.thread",
.metric_name = "IPC",
},
+{
+ .metric_expr = "idq_uops_not_delivered.core / (4 * (( ( cpu_clk_unhalted.thread / 2 ) * "
+ "( 1 + cpu_clk_unhalted.one_thread_active / cpu_clk_unhalted.ref_xclk ) )))",
+ .metric_name = "Frontend_Bound_SMT",
+},
};
static struct pmu_events_map map = {
@@ -138,8 +143,28 @@ static int test_ipc(void)
return 0;
}
+static int test_frontend(void)
+{
+ double ratio;
+ struct value vals[] = {
+ { .event = "idq_uops_not_delivered.core", .val = 300 },
+ { .event = "cpu_clk_unhalted.thread", .val = 200 },
+ { .event = "cpu_clk_unhalted.one_thread_active", .val = 400 },
+ { .event = "cpu_clk_unhalted.ref_xclk", .val = 600 },
+ { 0 },
+ };
+
+ TEST_ASSERT_VAL("failed to compute metric",
+ compute_metric("Frontend_Bound_SMT", vals, &ratio) == 0);
+
+ TEST_ASSERT_VAL("Frontend_Bound_SMT failed, wrong ratio",
+ ratio == 0.45);
+ return 0;
+}
+
int test__parse_metric(struct test *test __maybe_unused, int subtest __maybe_unused)
{
TEST_ASSERT_VAL("IPC failed", test_ipc() == 0);
+ TEST_ASSERT_VAL("frontend failed", test_frontend() == 0);
return 0;
}
--
2.25.4
We don't release metric_events rblist, add the missing
delete hook and call the release before leaving cmd_stat.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-stat.c | 1 +
tools/perf/util/metricgroup.c | 19 +++++++++++++++++++
tools/perf/util/metricgroup.h | 1 +
3 files changed, 21 insertions(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 8a0c470dd92b..6d45b9783b73 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2252,6 +2252,7 @@ int cmd_stat(int argc, const char **argv)
evlist__delete(evsel_list);
+ metricgroup__rblist_exit(&stat_config.metric_events);
runtime_stat_delete(&stat_config);
return status;
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index fe33fee7f6ad..1b80a7901388 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -76,11 +76,30 @@ static struct rb_node *metric_event_new(struct rblist *rblist __maybe_unused,
return &me->nd;
}
+static void metric_event_delete(struct rblist *rblist __maybe_unused,
+ struct rb_node *rb_node)
+{
+ struct metric_event *me = container_of(rb_node, struct metric_event, nd);
+ struct metric_expr *expr, *tmp;
+
+ list_for_each_entry_safe(expr, tmp, &me->head, nd) {
+ free(expr);
+ }
+
+ free(me);
+}
+
static void metricgroup__rblist_init(struct rblist *metric_events)
{
rblist__init(metric_events);
metric_events->node_cmp = metric_event_cmp;
metric_events->node_new = metric_event_new;
+ metric_events->node_delete = metric_event_delete;
+}
+
+void metricgroup__rblist_exit(struct rblist *metric_events)
+{
+ rblist__exit(metric_events);
}
struct egroup {
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 426c824e20bf..8315bd1a7da4 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -47,4 +47,5 @@ void metricgroup__print(bool metrics, bool groups, char *filter,
bool raw, bool details);
bool metricgroup__has_metric(const char *metric);
int arch_get_runtimeparam(void);
+void metricgroup__rblist_exit(struct rblist *metric_events);
#endif
--
2.25.4
Factoring out prepare_metric functio so it can
be used in test interface coming in following
changes.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/stat-shadow.c | 53 ++++++++++++++++++++++-------------
1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index a7c13a88ecb9..27be7ce2fff4 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -730,25 +730,16 @@ static void print_smi_cost(struct perf_stat_config *config,
out->print_metric(config, out->ctx, NULL, "%4.0f", "SMI#", smi_num);
}
-static void generic_metric(struct perf_stat_config *config,
- const char *metric_expr,
- struct evsel **metric_events,
- char *name,
- const char *metric_name,
- const char *metric_unit,
- int runtime,
- int cpu,
- struct perf_stat_output_ctx *out,
- struct runtime_stat *st)
+static int prepare_metric(struct evsel **metric_events,
+ struct expr_parse_ctx *pctx,
+ int cpu,
+ struct runtime_stat *st)
{
- print_metric_t print_metric = out->print_metric;
- struct expr_parse_ctx pctx;
- double ratio, scale;
- int i;
- void *ctxp = out->ctx;
+ double scale;
char *n, *pn;
+ int i;
- expr__ctx_init(&pctx);
+ expr__ctx_init(pctx);
for (i = 0; metric_events[i]; i++) {
struct saved_value *v;
struct stats *stats;
@@ -771,7 +762,7 @@ static void generic_metric(struct perf_stat_config *config,
n = strdup(metric_events[i]->name);
if (!n)
- return;
+ return -ENOMEM;
/*
* This display code with --no-merge adds [cpu] postfixes.
* These are not supported by the parser. Remove everything
@@ -782,11 +773,35 @@ static void generic_metric(struct perf_stat_config *config,
*pn = 0;
if (metric_total)
- expr__add_id(&pctx, n, metric_total);
+ expr__add_id(pctx, n, metric_total);
else
- expr__add_id(&pctx, n, avg_stats(stats)*scale);
+ expr__add_id(pctx, n, avg_stats(stats)*scale);
}
+ return i;
+}
+
+static void generic_metric(struct perf_stat_config *config,
+ const char *metric_expr,
+ struct evsel **metric_events,
+ char *name,
+ const char *metric_name,
+ const char *metric_unit,
+ int runtime,
+ int cpu,
+ struct perf_stat_output_ctx *out,
+ struct runtime_stat *st)
+{
+ print_metric_t print_metric = out->print_metric;
+ struct expr_parse_ctx pctx;
+ double ratio, scale;
+ int i;
+ void *ctxp = out->ctx;
+
+ i = prepare_metric(metric_events, &pctx, cpu, st);
+ if (i < 0)
+ return;
+
if (!metric_events[i]) {
if (expr__parse(&ratio, &pctx, metric_expr, runtime) == 0) {
char *unit;
--
2.25.4
Adding test_generic_metric that prepares and runs given metric
over the data from psswd struct runtime_stat object.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/stat-shadow.c | 14 ++++++++++++++
tools/perf/util/stat.h | 3 +++
2 files changed, 17 insertions(+)
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 27be7ce2fff4..8fdef47005e6 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -842,6 +842,20 @@ static void generic_metric(struct perf_stat_config *config,
expr__ctx_clear(&pctx);
}
+double test_generic_metric(struct metric_expr *mexp, int cpu, struct runtime_stat *st)
+{
+ struct expr_parse_ctx pctx;
+ double ratio;
+
+ if (prepare_metric(mexp->metric_events, &pctx, cpu, st) < 0)
+ return 0.;
+
+ if (expr__parse(&ratio, &pctx, mexp->metric_expr, 1))
+ return 0.;
+
+ return ratio;
+}
+
void perf_stat__print_shadow_stats(struct perf_stat_config *config,
struct evsel *evsel,
double avg, int cpu,
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index f75ae679eb28..6911c7249199 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -230,4 +230,7 @@ perf_evlist__print_counters(struct evlist *evlist,
struct target *_target,
struct timespec *ts,
int argc, const char **argv);
+
+struct metric_expr;
+double test_generic_metric(struct metric_expr *mexp, int cpu, struct runtime_stat *st);
#endif
--
2.25.4
Adding new test that process metrics code and checks
the expected results. Starting with easy ipc metric.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/Build | 1 +
tools/perf/tests/builtin-test.c | 4 +
tools/perf/tests/parse-metric.c | 145 ++++++++++++++++++++++++++++++++
tools/perf/tests/tests.h | 1 +
4 files changed, 151 insertions(+)
create mode 100644 tools/perf/tests/parse-metric.c
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index cd00498a5dce..84352fc49a20 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -59,6 +59,7 @@ perf-y += genelf.o
perf-y += api-io.o
perf-y += demangle-java-test.o
perf-y += pfm.o
+perf-y += parse-metric.o
$(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
$(call rule_mkdir)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index da5b6cc23f25..d328caaba45d 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -337,6 +337,10 @@ static struct test generic_tests[] = {
.desc = "Demangle Java",
.func = test__demangle_java,
},
+ {
+ .desc = "Parse and process metrics",
+ .func = test__parse_metric,
+ },
{
.func = NULL,
},
diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
new file mode 100644
index 000000000000..717a73fa7446
--- /dev/null
+++ b/tools/perf/tests/parse-metric.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/compiler.h>
+#include <string.h>
+#include <perf/cpumap.h>
+#include <perf/evlist.h>
+#include "metricgroup.h"
+#include "tests.h"
+#include "pmu-events/pmu-events.h"
+#include "evlist.h"
+#include "rblist.h"
+#include "debug.h"
+#include "expr.h"
+#include "stat.h"
+
+static struct pmu_event pme_test[] = {
+{
+ .metric_expr = "inst_retired.any / cpu_clk_unhalted.thread",
+ .metric_name = "IPC",
+},
+};
+
+static struct pmu_events_map map = {
+ .cpuid = "test",
+ .version = "1",
+ .type = "core",
+ .table = pme_test,
+};
+
+struct value {
+ const char *event;
+ u64 val;
+};
+
+static u64 find_value(const char *name, struct value *values)
+{
+ struct value *v = values;
+
+ while (v->event) {
+ if (!strcmp(name, v->event))
+ return v->val;
+ v++;
+ };
+ return 0;
+}
+
+static void load_runtime_stat(struct runtime_stat *st, struct evlist *evlist,
+ struct value *vals)
+{
+ struct evsel *evsel;
+ u64 count;
+
+ evlist__for_each_entry(evlist, evsel) {
+ count = find_value(evsel->name, vals);
+ perf_stat__update_shadow_stats(evsel, count, 0, st);
+ }
+}
+
+static double compute_single(struct rblist *metric_events, struct evlist *evlist,
+ struct runtime_stat *st)
+{
+ struct evsel *evsel = evlist__first(evlist);
+ struct metric_event *me;
+
+ me = metricgroup__lookup(metric_events, evsel, false);
+ if (me != NULL) {
+ struct metric_expr *mexp;
+
+ mexp = list_first_entry(&me->head, struct metric_expr, nd);
+ return test_generic_metric(mexp, 0, st);
+ }
+ return 0.;
+}
+
+static int compute_metric(const char *name, struct value *vals, double *ratio)
+{
+ struct rblist metric_events = {
+ .nr_entries = 0,
+ };
+ struct perf_cpu_map *cpus;
+ struct runtime_stat st;
+ struct evlist *evlist;
+ int err;
+
+ /*
+ * We need to prepare evlist for stat mode running on CPU 0
+ * because that's where all the stats are going to be created.
+ */
+ evlist = evlist__new();
+ if (!evlist)
+ return -ENOMEM;
+
+ cpus = perf_cpu_map__new("0");
+ if (!cpus)
+ return -ENOMEM;
+
+ perf_evlist__set_maps(&evlist->core, cpus, NULL);
+
+ /* Parse the metric into metric_events list. */
+ err = metricgroup__parse_groups_test(evlist, &map, name,
+ false, false,
+ &metric_events);
+
+ TEST_ASSERT_VAL("failed to parse metric", err == 0);
+
+ if (perf_evlist__alloc_stats(evlist, false))
+ return -1;
+
+ /* Load the runtime stats with given numbers for events. */
+ runtime_stat__init(&st);
+ load_runtime_stat(&st, evlist, vals);
+
+ /* And execute the metric */
+ *ratio = compute_single(&metric_events, evlist, &st);
+
+ /* ... clenup. */
+ metricgroup__rblist_exit(&metric_events);
+ runtime_stat__exit(&st);
+ perf_evlist__free_stats(evlist);
+ perf_cpu_map__put(cpus);
+ evlist__delete(evlist);
+ return 0;
+}
+
+static int test_ipc(void)
+{
+ double ratio;
+ struct value vals[] = {
+ { .event = "inst_retired.any", .val = 300 },
+ { .event = "cpu_clk_unhalted.thread", .val = 200 },
+ { 0 },
+ };
+
+ TEST_ASSERT_VAL("failed to compute metric",
+ compute_metric("IPC", vals, &ratio) == 0);
+
+ TEST_ASSERT_VAL("IPC failed, wrong ratio",
+ ratio == 1.5);
+ return 0;
+}
+
+int test__parse_metric(struct test *test __maybe_unused, int subtest __maybe_unused)
+{
+ TEST_ASSERT_VAL("IPC failed", test_ipc() == 0);
+ return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 76a4e352eaaf..4447a516c689 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -121,6 +121,7 @@ int test__demangle_java(struct test *test, int subtest);
int test__pfm(struct test *test, int subtest);
const char *test__pfm_subtest_get_desc(int subtest);
int test__pfm_subtest_get_nr(void);
+int test__parse_metric(struct test *test, int subtest);
bool test__bp_signal_is_supported(void);
bool test__bp_account_is_supported(void);
--
2.25.4
On Tue, Jun 2, 2020 at 4:51 AM Jiri Olsa <[email protected]> wrote:
>
> The test goes through all metrics compiled for arch
> within pmu events and try to parse them.
>
> This test is different from 'test_parsing' in that
> we go through all the events in the current arch,
> not just one defined for current CPU model. Using
> 'fake_pmu' to parse events which do not have PMUs
> defined in the system.
>
> Say there's bad change in ivybridge metrics file, like:
>
> --- a/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
> +++ b/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
> @@ -8,7 +8,7 @@
> - "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / (4 * ((
> + "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / / (4 *
>
> the test fails with (on my kabylake laptop):
>
> $ perf test 'Parsing of PMU event table metrics with fake PMUs' -v
> parsing 'idq_uops_not_delivered.core / / (4 * (( ( cpu_clk_unh...
> syntax error, line 1
> expr__parse failed
> test child finished with -1
> ...
For this example as the problem is the expression, presumably this was
"passing" with test_parsing due to returning TEST_SKIP? I did this
initially so that we could get the test merged and then the metrics
fixed. I'd prefer if test_parsing were returning TEST_FAIL.
> The test also defines its own list of metrics and
> tries to parse them. It's handy for developing.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> tools/perf/tests/pmu-events.c | 117 +++++++++++++++++++++++++++++++++-
> 1 file changed, 114 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index f65380d2066f..d3343827eb4d 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -390,7 +390,8 @@ static bool is_number(const char *str)
> return errno == 0 && end_ptr != str;
> }
>
> -static int check_parse_id(const char *id, struct parse_events_error *error)
> +static int check_parse_id(const char *id, struct parse_events_error *error,
> + bool fake_pmu)
> {
> struct evlist *evlist;
> int ret;
> @@ -403,7 +404,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error)
> if (!evlist)
> return -ENOMEM;
> memset(error, 0, sizeof(*error));
> - ret = parse_events(evlist, id, error, false);
> + ret = parse_events(evlist, id, error, fake_pmu);
> evlist__delete(evlist);
> return ret;
> }
> @@ -412,7 +413,7 @@ static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
> {
> struct parse_events_error error;
>
> - int ret = check_parse_id(id, &error);
> + int ret = check_parse_id(id, &error, false);
> if (ret && same_cpu) {
> pr_warning("Parse event failed metric '%s' id '%s' expr '%s'\n",
> pe->metric_name, id, pe->metric_expr);
> @@ -430,6 +431,18 @@ static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
> return ret;
> }
>
> +static int check_parse_fake(const char *id)
> +{
> + struct parse_events_error error;
nit: this reads funny as it isn't clear, without looking at
check_parse_id, that error is zero initialized.
Thanks,
Ian
> + int ret = check_parse_id(id, &error, true);
> +
> + free(error.str);
> + free(error.help);
> + free(error.first_str);
> + free(error.first_help);
> + return ret;
> +}
> +
> static void expr_failure(const char *msg,
> const struct pmu_events_map *map,
> const struct pmu_event *pe)
> @@ -499,6 +512,100 @@ static int test_parsing(void)
> return ret == 0 ? TEST_OK : TEST_SKIP;
> }
>
> +struct test_metric {
> + const char *str;
> +};
> +
> +static struct test_metric metrics[] = {
> + { "(unc_p_power_state_occupancy.cores_c0 / unc_p_clockticks) * 100." },
> + { "imx8_ddr0@read\\-cycles@ * 4 * 4", },
> + { "imx8_ddr0@axid\\-read\\,axi_mask\\=0xffff\\,axi_id\\=0x0000@ * 4", },
> + { "(cstate_pkg@c2\\-residency@ / msr@tsc@) * 100", },
> + { "(imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@)", },
> +};
> +
> +static int metric_parse_fake(const char *str)
> +{
> + struct expr_parse_ctx ctx;
> + struct hashmap_entry *cur;
> + double result;
> + int ret = -1;
> + size_t bkt;
> + int i;
> +
> + pr_debug("parsing '%s'\n", str);
> +
> + expr__ctx_init(&ctx);
> + if (expr__find_other(str, NULL, &ctx, 0) < 0) {
> + pr_err("expr__find_other failed\n");
> + return -1;
> + }
> +
> + /*
> + * Add all ids with a made up value. The value may
> + * trigger divide by zero when subtracted and so try to
> + * make them unique.
> + */
> + i = 1;
> + hashmap__for_each_entry((&ctx.ids), cur, bkt)
> + expr__add_id(&ctx, strdup(cur->key), i++);
> +
> + hashmap__for_each_entry((&ctx.ids), cur, bkt) {
> + if (check_parse_fake(cur->key)) {
> + pr_err("check_parse_fake failed\n");
> + goto out;
> + }
> + }
> +
> + if (expr__parse(&result, &ctx, str, 1))
> + pr_err("expr__parse failed\n");
> + else
> + ret = 0;
> +
> +out:
> + expr__ctx_clear(&ctx);
> + return ret;
> +}
> +
> +/*
> + * Parse all the metrics for current architecture,
> + * or all defined cpus via the 'fake_pmu' bool
> + * in parse_events.
> + */
> +static int test_parsing_fake(void)
> +{
> + struct pmu_events_map *map;
> + struct pmu_event *pe;
> + unsigned int i, j;
> + int err = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(metrics); i++) {
> + err = metric_parse_fake(metrics[i].str);
> + if (err)
> + return err;
> + }
> +
> + i = 0;
> + for (;;) {
> + map = &pmu_events_map[i++];
> + if (!map->table)
> + break;
> + j = 0;
> + for (;;) {
> + pe = &map->table[j++];
> + if (!pe->name && !pe->metric_group && !pe->metric_name)
> + break;
> + if (!pe->metric_expr)
> + continue;
> + err = metric_parse_fake(pe->metric_expr);
> + if (err)
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> static const struct {
> int (*func)(void);
> const char *desc;
> @@ -515,6 +622,10 @@ static const struct {
> .func = test_parsing,
> .desc = "Parsing of PMU event table metrics",
> },
> + {
> + .func = test_parsing_fake,
> + .desc = "Parsing of PMU event table metrics with fake PMUs",
> + },
> };
>
> const char *test__pmu_events_subtest_get_desc(int subtest)
> --
> 2.25.4
>
On Tue, Jun 2, 2020 at 4:51 AM Jiri Olsa <[email protected]> wrote:
>
> hi,
> changes for using metric result in another metric seem
> to change lot of core metric code, so it's better we
> have some more tests before we do that.
>
> v2 changes:
> - some of the patches got accepted
> - add missing free to patch 1 [Ian]
> - factor pmu-events test functions and reuse it in the new test [Ian]
> - add fake_pmu bool to parse_events interface [Ian]
> - simplify metric tests
> - use proper cover letter subject ;-)
>
> I actually reworked the 2 patches Ian acked so far,
> so I did not add them.
>
> Also available in here:
> git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> perf/metric_test
>
> thanks,
> jirka
>
>
> ---
> Jiri Olsa (13):
> perf tools: Add fake pmu support
> perf tools: Add fake_pmu bool to parse_events interface
> perf tests: Factor check_parse_id function
> perf tests: Add another metric parsing test
> perf tools: Factor out parse_groups function
> perf tools: Add fake_pmu to parse_events function
> perf tools: Add map to parse_events function
> perf tools: Add metricgroup__parse_groups_test function
> perf tools: Factor out prepare_metric function
> perf tools: Release metric_events rblist
> perf tools: Add test_generic_metric function
> perf tests: Add parse metric test for ipc metric
> perf tests: Add parse metric test for frontend metric
>
> tools/perf/arch/arm/util/cs-etm.c | 2 +-
> tools/perf/arch/arm64/util/arm-spe.c | 2 +-
> tools/perf/arch/powerpc/util/kvm-stat.c | 2 +-
> tools/perf/arch/x86/tests/intel-cqm.c | 2 +-
> tools/perf/arch/x86/tests/perf-time-to-tsc.c | 2 +-
> tools/perf/arch/x86/util/intel-bts.c | 2 +-
> tools/perf/arch/x86/util/intel-pt.c | 6 ++--
> tools/perf/builtin-stat.c | 9 +++---
> tools/perf/builtin-trace.c | 4 +--
> tools/perf/tests/Build | 1 +
> tools/perf/tests/backward-ring-buffer.c | 3 +-
> tools/perf/tests/builtin-test.c | 4 +++
> tools/perf/tests/code-reading.c | 2 +-
> tools/perf/tests/event-times.c | 2 +-
> tools/perf/tests/evsel-roundtrip-name.c | 4 +--
> tools/perf/tests/hists_cumulate.c | 2 +-
> tools/perf/tests/hists_filter.c | 4 +--
> tools/perf/tests/hists_link.c | 4 +--
> tools/perf/tests/hists_output.c | 2 +-
> tools/perf/tests/keep-tracking.c | 4 +--
> tools/perf/tests/parse-events.c | 2 +-
> tools/perf/tests/parse-metric.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/tests/pmu-events.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> tools/perf/tests/switch-tracking.c | 8 ++---
> tools/perf/tests/tests.h | 1 +
> tools/perf/util/bpf-loader.c | 2 +-
> tools/perf/util/metricgroup.c | 74 ++++++++++++++++++++++++++++++++++++----------
> tools/perf/util/metricgroup.h | 10 +++++++
> tools/perf/util/parse-events.c | 29 +++++++++++-------
> tools/perf/util/parse-events.h | 5 ++--
> tools/perf/util/parse-events.l | 8 +++--
> tools/perf/util/parse-events.y | 41 ++++++++++++++++++++++++--
> tools/perf/util/perf_api_probe.c | 2 +-
> tools/perf/util/record.c | 2 +-
> tools/perf/util/stat-shadow.c | 67 ++++++++++++++++++++++++++++++------------
> tools/perf/util/stat.h | 3 ++
> 36 files changed, 527 insertions(+), 92 deletions(-)
> create mode 100644 tools/perf/tests/parse-metric.c
>
Acked-by: Ian Rogers <[email protected]>
On Tue, Jun 02, 2020 at 10:58:32AM -0700, Ian Rogers wrote:
> On Tue, Jun 2, 2020 at 4:51 AM Jiri Olsa <[email protected]> wrote:
> >
> > The test goes through all metrics compiled for arch
> > within pmu events and try to parse them.
> >
> > This test is different from 'test_parsing' in that
> > we go through all the events in the current arch,
> > not just one defined for current CPU model. Using
> > 'fake_pmu' to parse events which do not have PMUs
> > defined in the system.
> >
> > Say there's bad change in ivybridge metrics file, like:
> >
> > --- a/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
> > +++ b/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
> > @@ -8,7 +8,7 @@
> > - "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / (4 * ((
> > + "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / / (4 *
> >
> > the test fails with (on my kabylake laptop):
> >
> > $ perf test 'Parsing of PMU event table metrics with fake PMUs' -v
> > parsing 'idq_uops_not_delivered.core / / (4 * (( ( cpu_clk_unh...
> > syntax error, line 1
> > expr__parse failed
> > test child finished with -1
> > ...
>
> For this example as the problem is the expression, presumably this was
> "passing" with test_parsing due to returning TEST_SKIP? I did this
> initially so that we could get the test merged and then the metrics
> fixed. I'd prefer if test_parsing were returning TEST_FAIL.
it will fail:
[jolsa@krava perf]$ git diff
diff --git a/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json b/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
index 28e25447d3ef..0cad6b709f96 100644
--- a/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
+++ b/tools/perf/pmu-events/arch/x86/ivybridge/ivb-metrics.json
@@ -1,7 +1,7 @@
[
{
"BriefDescription": "This category represents fraction of slots where the processor's Frontend undersupplies its Backend",
- "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / (4 * cycles)",
+ "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / / (4 * cycles)",
"MetricGroup": "TopdownL1",
"MetricName": "Frontend_Bound",
[jolsa@krava perf]$ ./perf test 'Parsing of PMU event table metrics with fake PMUs'
10: PMU events :
10.4: Parsing of PMU event table metrics with fake PMUs : FAILED!
because the metric is malformed
jirka
On Tue, Jun 02, 2020 at 10:58:32AM -0700, Ian Rogers wrote:
SNIP
> > +static int check_parse_fake(const char *id)
> > +{
> > + struct parse_events_error error;
>
> nit: this reads funny as it isn't clear, without looking at
> check_parse_id, that error is zero initialized.
right, how about something like below?
thanks,
jirka
---
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index d3343827eb4d..c745b6e13cbe 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -403,7 +403,6 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
evlist = evlist__new();
if (!evlist)
return -ENOMEM;
- memset(error, 0, sizeof(*error));
ret = parse_events(evlist, id, error, fake_pmu);
evlist__delete(evlist);
return ret;
@@ -411,7 +410,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
{
- struct parse_events_error error;
+ struct parse_events_error error = { 0 };
int ret = check_parse_id(id, &error, false);
if (ret && same_cpu) {
@@ -433,7 +432,7 @@ static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
static int check_parse_fake(const char *id)
{
- struct parse_events_error error;
+ struct parse_events_error error = { 0 };
int ret = check_parse_id(id, &error, true);
free(error.str);
On Tue, Jun 2, 2020 at 12:08 PM Jiri Olsa <[email protected]> wrote:
>
> On Tue, Jun 02, 2020 at 10:58:32AM -0700, Ian Rogers wrote:
>
> SNIP
>
> > > +static int check_parse_fake(const char *id)
> > > +{
> > > + struct parse_events_error error;
> >
> > nit: this reads funny as it isn't clear, without looking at
> > check_parse_id, that error is zero initialized.
>
> right, how about something like below?
Thanks, looks good!
Ian
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index d3343827eb4d..c745b6e13cbe 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -403,7 +403,6 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
> evlist = evlist__new();
> if (!evlist)
> return -ENOMEM;
> - memset(error, 0, sizeof(*error));
> ret = parse_events(evlist, id, error, fake_pmu);
> evlist__delete(evlist);
> return ret;
> @@ -411,7 +410,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
>
> static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
> {
> - struct parse_events_error error;
> + struct parse_events_error error = { 0 };
>
> int ret = check_parse_id(id, &error, false);
> if (ret && same_cpu) {
> @@ -433,7 +432,7 @@ static int check_parse_cpu(const char *id, bool same_cpu, struct pmu_event *pe)
>
> static int check_parse_fake(const char *id)
> {
> - struct parse_events_error error;
> + struct parse_events_error error = { 0 };
> int ret = check_parse_id(id, &error, true);
>
> free(error.str);
>