There are 4 main changes in this patch set:
- perf metric: Modify resolution and recursion check.
- perf parse-events: Add new "metric-id" term.
- perf metrics: Modify setup and deduplication
- perf metric: Allow modifiers on metrics.
In overview the changes start by trying to simplify the metric code,
then it fixes various bugs and finally it builds a new feature of
allowing metrics like:
$ perf stat -M IPC:u,IPC:k -a sleep 1
Performance counter stats for 'system wide':
93,269,988 inst_retired.any:k # 0.26 IPC:k
352,037,460 cpu_clk_unhalted.thread:k
70,317,865 inst_retired.any:u # 0.76 IPC:u
92,762,220 cpu_clk_unhalted.thread:u
1.003754577 seconds time elapsed
Previous complexity came from using the evsel->name as the identifier
for events in metrics, however, this name isn't stable and has issues
around wildcard expansion. These changes fix this by adding a
dedicated metric_id to evsels, performing deduplication on IDs before
event parsing and not handling all evsels on a single evlist.
The recursion and metric_ref logic is simplified, the first by moving
data from the heap to the stack, the latter by building in an array
rather than a linked list. This logic is integral to metric set up and
simplification makes the effects of the changes easier to follow, in
particular as there are fewer structs being maintained.
Event parsing is modified to allow qualifiers on kernel PMU events,
this is necessary to allow the metric-id to be added, but allows
qualifiers in other cases like specifying callgraph or a name.
There is a certain amount of comment adding and const-ification, this
is with a view to making the code more intention revealing and to aid
following its logic. For example, the pmu event tables should never
change and it'd be a bug if they ever did, it's therefore strange to
access it using non-const pointers.
The kernel list_sort.c/h are added for use sorting metrics in order to
deduplicate/reuse events from a larger group in a smaller one. This
was previously done by inserting in size order, but that only worked
within a metric group.
Some of the commit messages show the TopDownL1 metrics being used on a
SkylakeX machine. These metrics were removed by
c4ad8fabd03f76ed3a2a4c8aef6baf6cd4f24542 ("perf vendor events: Update
metrics for SkyLake Server") and the data was gathered with this patch
reverted.
Ian Rogers (21):
tools lib: Add list_sort.
perf pmu: Add const to pmu_events_map.
perf pmu: Make pmu_sys_event_tables const.
perf pmu: Make pmu_event tables const.
perf metric: Move runtime value to the expr context
perf metric: Add documentation and rename a variable.
perf metric: Add metric new and free
perf metric: Only add a referenced metric once
perf metric: Modify resolution and recursion check.
perf metric: Comment data structures.
perf metric: Document the internal 'struct metric'
perf metric: Simplify metric_refs calculation.
perf parse-events: Add const to evsel name
perf parse-events: Add new "metric-id" term.
perf parse-events: Allow config on kernel PMU events
perf metric: Encode and use metric-id as qualifier
perf expr: Add subset utility.
perf metrics: Modify setup and deduplication
perf metric: Switch fprintf to pr_err.
perf parse-events: Identify broken modifiers.
perf metric: Allow modifiers on metrics.
tools/include/linux/list_sort.h | 14 +
tools/lib/list_sort.c | 252 +++++
tools/perf/arch/powerpc/util/header.c | 2 +-
tools/perf/check-headers.sh | 2 +
tools/perf/pmu-events/jevents.c | 6 +-
tools/perf/pmu-events/pmu-events.h | 8 +-
tools/perf/tests/expand-cgroup.c | 2 +-
tools/perf/tests/expr.c | 29 +-
tools/perf/tests/parse-metric.c | 2 +-
tools/perf/tests/pmu-events.c | 59 +-
tools/perf/util/Build | 5 +
tools/perf/util/evsel.c | 17 +
tools/perf/util/evsel.h | 2 +
tools/perf/util/expr.c | 56 +-
tools/perf/util/expr.h | 16 +-
tools/perf/util/expr.l | 6 +-
tools/perf/util/expr.y | 2 +-
tools/perf/util/metricgroup.c | 1441 ++++++++++++++-----------
tools/perf/util/metricgroup.h | 35 +-
tools/perf/util/parse-events-hybrid.c | 34 +-
tools/perf/util/parse-events-hybrid.h | 6 +-
tools/perf/util/parse-events.c | 166 +--
tools/perf/util/parse-events.h | 11 +-
tools/perf/util/parse-events.l | 18 +-
tools/perf/util/parse-events.y | 27 +-
tools/perf/util/pfm.c | 3 +-
tools/perf/util/pmu.c | 22 +-
tools/perf/util/pmu.h | 10 +-
tools/perf/util/s390-sample-raw.c | 6 +-
tools/perf/util/stat-shadow.c | 27 +-
30 files changed, 1448 insertions(+), 838 deletions(-)
create mode 100644 tools/include/linux/list_sort.h
create mode 100644 tools/lib/list_sort.c
--
2.33.0.882.g93a45727a2-goog
The pmu_events_map is generated at compile time and used for lookup. For
testing purposes we need to swap the map being used. Having the
pmu_events_map be non-const is misleading as it may be an out argument.
Make it const and update uses so they work on const too.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/pmu-events/jevents.c | 2 +-
tools/perf/pmu-events/pmu-events.h | 2 +-
tools/perf/tests/expand-cgroup.c | 2 +-
tools/perf/tests/parse-metric.c | 2 +-
tools/perf/tests/pmu-events.c | 18 +++++++++---------
tools/perf/util/metricgroup.c | 20 ++++++++++----------
tools/perf/util/metricgroup.h | 4 ++--
tools/perf/util/pmu.c | 10 +++++-----
tools/perf/util/pmu.h | 6 +++---
tools/perf/util/s390-sample-raw.c | 4 ++--
10 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 19497e4f8a86..5624a37d6f93 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -798,7 +798,7 @@ static bool is_sys_dir(char *fname)
static void print_mapping_table_prefix(FILE *outfp)
{
- fprintf(outfp, "struct pmu_events_map pmu_events_map[] = {\n");
+ fprintf(outfp, "const struct pmu_events_map pmu_events_map[] = {\n");
}
static void print_mapping_table_suffix(FILE *outfp)
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index 5c2bf7275c1c..42c6db6bedec 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -53,7 +53,7 @@ struct pmu_sys_events {
* Global table mapping each known CPU for the architecture to its
* table of PMU events.
*/
-extern struct pmu_events_map pmu_events_map[];
+extern const struct pmu_events_map pmu_events_map[];
extern struct pmu_sys_events pmu_sys_event_tables[];
#endif
diff --git a/tools/perf/tests/expand-cgroup.c b/tools/perf/tests/expand-cgroup.c
index 0e46aeb843ce..aaad51aba12f 100644
--- a/tools/perf/tests/expand-cgroup.c
+++ b/tools/perf/tests/expand-cgroup.c
@@ -193,7 +193,7 @@ static int expand_metric_events(void)
.metric_name = NULL,
},
};
- struct pmu_events_map ev_map = {
+ const struct pmu_events_map ev_map = {
.cpuid = "test",
.version = "1",
.type = "core",
diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
index 4f6f4904e852..dfc797ecc750 100644
--- a/tools/perf/tests/parse-metric.c
+++ b/tools/perf/tests/parse-metric.c
@@ -79,7 +79,7 @@ static struct pmu_event pme_test[] = {
}
};
-static struct pmu_events_map map = {
+static const struct pmu_events_map map = {
.cpuid = "test",
.version = "1",
.type = "core",
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index d3534960ed25..8a1fdcd072f5 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -242,9 +242,9 @@ static bool is_same(const char *reference, const char *test)
return !strcmp(reference, test);
}
-static struct pmu_events_map *__test_pmu_get_events_map(void)
+static const struct pmu_events_map *__test_pmu_get_events_map(void)
{
- struct pmu_events_map *map;
+ const struct pmu_events_map *map;
for (map = &pmu_events_map[0]; map->cpuid; map++) {
if (!strcmp(map->cpuid, "testcpu"))
@@ -421,7 +421,7 @@ static int compare_alias_to_test_event(struct perf_pmu_alias *alias,
static int test_pmu_event_table(void)
{
struct pmu_event *sys_event_tables = __test_pmu_get_sys_events_table();
- struct pmu_events_map *map = __test_pmu_get_events_map();
+ const struct pmu_events_map *map = __test_pmu_get_events_map();
struct pmu_event *table;
int map_events = 0, expected_events;
@@ -518,7 +518,7 @@ static int __test_core_pmu_event_aliases(char *pmu_name, int *count)
struct perf_pmu *pmu;
LIST_HEAD(aliases);
int res = 0;
- struct pmu_events_map *map = __test_pmu_get_events_map();
+ const struct pmu_events_map *map = __test_pmu_get_events_map();
struct perf_pmu_alias *a, *tmp;
if (!map)
@@ -571,7 +571,7 @@ static int __test_uncore_pmu_event_aliases(struct perf_pmu_test_pmu *test_pmu)
struct perf_pmu *pmu = &test_pmu->pmu;
const char *pmu_name = pmu->name;
struct perf_pmu_alias *a, *tmp, *alias;
- struct pmu_events_map *map;
+ const struct pmu_events_map *map;
LIST_HEAD(aliases);
int res = 0;
@@ -825,7 +825,7 @@ struct metric {
static int resolve_metric_simple(struct expr_parse_ctx *pctx,
struct list_head *compound_list,
- struct pmu_events_map *map,
+ const struct pmu_events_map *map,
const char *metric_name)
{
struct hashmap_entry *cur, *cur_tmp;
@@ -885,8 +885,8 @@ static int resolve_metric_simple(struct expr_parse_ctx *pctx,
static int test_parsing(void)
{
- struct pmu_events_map *cpus_map = pmu_events_map__find();
- struct pmu_events_map *map;
+ const struct pmu_events_map *cpus_map = pmu_events_map__find();
+ const struct pmu_events_map *map;
struct pmu_event *pe;
int i, j, k;
int ret = 0;
@@ -1027,7 +1027,7 @@ static int metric_parse_fake(const char *str)
*/
static int test_parsing_fake(void)
{
- struct pmu_events_map *map;
+ const struct pmu_events_map *map;
struct pmu_event *pe;
unsigned int i, j;
int err = 0;
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 8ba5370f5f64..74ea0a3540ce 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -623,7 +623,7 @@ static int metricgroup__print_sys_event_iter(struct pmu_event *pe, void *data)
void metricgroup__print(bool metrics, bool metricgroups, char *filter,
bool raw, bool details)
{
- struct pmu_events_map *map = pmu_events_map__find();
+ const struct pmu_events_map *map = pmu_events_map__find();
struct pmu_event *pe;
int i;
struct rblist groups;
@@ -910,7 +910,7 @@ static int __add_metric(struct list_head *metric_list,
match_metric(__pe->metric_name, __metric)))
struct pmu_event *metricgroup__find_metric(const char *metric,
- struct pmu_events_map *map)
+ const struct pmu_events_map *map)
{
struct pmu_event *pe;
int i;
@@ -977,7 +977,7 @@ static int add_metric(struct list_head *metric_list,
static int __resolve_metric(struct metric *m,
bool metric_no_group,
struct list_head *metric_list,
- struct pmu_events_map *map,
+ const struct pmu_events_map *map,
struct expr_ids *ids)
{
struct hashmap_entry *cur;
@@ -1025,7 +1025,7 @@ static int __resolve_metric(struct metric *m,
static int resolve_metric(bool metric_no_group,
struct list_head *metric_list,
- struct pmu_events_map *map,
+ const struct pmu_events_map *map,
struct expr_ids *ids)
{
struct metric *m;
@@ -1099,7 +1099,7 @@ static int metricgroup__add_metric_sys_event_iter(struct pmu_event *pe,
static int metricgroup__add_metric(const char *metric, bool metric_no_group,
struct strbuf *events,
struct list_head *metric_list,
- struct pmu_events_map *map)
+ const struct pmu_events_map *map)
{
struct expr_ids ids = { .cnt = 0, };
struct pmu_event *pe;
@@ -1173,7 +1173,7 @@ 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 *metric_list,
- struct pmu_events_map *map)
+ const struct pmu_events_map *map)
{
char *llist, *nlist, *p;
int ret = -EINVAL;
@@ -1230,7 +1230,7 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
bool metric_no_merge,
struct perf_pmu *fake_pmu,
struct rblist *metric_events,
- struct pmu_events_map *map)
+ const struct pmu_events_map *map)
{
struct parse_events_error parse_error;
struct strbuf extra_events;
@@ -1266,14 +1266,14 @@ 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 = pmu_events_map__find();
+ const struct pmu_events_map *map = pmu_events_map__find();
return parse_groups(perf_evlist, str, metric_no_group,
metric_no_merge, NULL, metric_events, map);
}
int metricgroup__parse_groups_test(struct evlist *evlist,
- struct pmu_events_map *map,
+ const struct pmu_events_map *map,
const char *str,
bool metric_no_group,
bool metric_no_merge,
@@ -1285,7 +1285,7 @@ int metricgroup__parse_groups_test(struct evlist *evlist,
bool metricgroup__has_metric(const char *metric)
{
- struct pmu_events_map *map = pmu_events_map__find();
+ const struct pmu_events_map *map = pmu_events_map__find();
struct pmu_event *pe;
int i;
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index cc4a92492a61..c931596557bf 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -44,9 +44,9 @@ int metricgroup__parse_groups(const struct option *opt,
bool metric_no_merge,
struct rblist *metric_events);
struct pmu_event *metricgroup__find_metric(const char *metric,
- struct pmu_events_map *map);
+ const struct pmu_events_map *map);
int metricgroup__parse_groups_test(struct evlist *evlist,
- struct pmu_events_map *map,
+ const struct pmu_events_map *map,
const char *str,
bool metric_no_group,
bool metric_no_merge,
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index bdabd62170d2..4bcdc595ce5e 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -710,9 +710,9 @@ static char *perf_pmu__getcpuid(struct perf_pmu *pmu)
return cpuid;
}
-struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
+const struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
{
- struct pmu_events_map *map;
+ const struct pmu_events_map *map;
char *cpuid = perf_pmu__getcpuid(pmu);
int i;
@@ -737,7 +737,7 @@ struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
return map;
}
-struct pmu_events_map *__weak pmu_events_map__find(void)
+const struct pmu_events_map *__weak pmu_events_map__find(void)
{
return perf_pmu__find_map(NULL);
}
@@ -824,7 +824,7 @@ bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
* as aliases.
*/
void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
- struct pmu_events_map *map)
+ const struct pmu_events_map *map)
{
int i;
const char *name = pmu->name;
@@ -859,7 +859,7 @@ void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
{
- struct pmu_events_map *map;
+ const struct pmu_events_map *map;
map = perf_pmu__find_map(pmu);
if (!map)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 394898b07fd9..dd5cdde6a3d0 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -120,10 +120,10 @@ int perf_pmu__test(void);
struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu);
void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
- struct pmu_events_map *map);
+ const struct pmu_events_map *map);
-struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu);
-struct pmu_events_map *pmu_events_map__find(void);
+const struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu);
+const struct pmu_events_map *pmu_events_map__find(void);
bool pmu_uncore_alias_match(const char *pmu_name, const char *name);
void perf_pmu_free_alias(struct perf_pmu_alias *alias);
diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
index 08ec3c3ae0ee..13f33d1ddb78 100644
--- a/tools/perf/util/s390-sample-raw.c
+++ b/tools/perf/util/s390-sample-raw.c
@@ -135,7 +135,7 @@ static int get_counterset_start(int setnr)
* the name of this counter.
* If no match is found a NULL pointer is returned.
*/
-static const char *get_counter_name(int set, int nr, struct pmu_events_map *map)
+static const char *get_counter_name(int set, int nr, const struct pmu_events_map *map)
{
int rc, event_nr, wanted = get_counterset_start(set) + nr;
@@ -159,7 +159,7 @@ static void s390_cpumcfdg_dump(struct perf_sample *sample)
unsigned char *buf = sample->raw_data;
const char *color = PERF_COLOR_BLUE;
struct cf_ctrset_entry *cep, ce;
- struct pmu_events_map *map;
+ const struct pmu_events_map *map;
u64 *p;
map = pmu_events_map__find();
--
2.33.0.882.g93a45727a2-goog
Don't build a list and then turn to an array, just directly build the
array. The size of the array is known due to the search for a duplicate.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/metricgroup.c | 77 +++++++++++------------------------
1 file changed, 23 insertions(+), 54 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 632867cedbae..b48836d7c080 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -137,10 +137,8 @@ struct metric {
* output.
*/
const char *metric_unit;
- /** The list of metrics referenced by this one. */
- struct list_head metric_refs;
- /** The size of the metric_refs list. */
- int metric_refs_cnt;
+ /** Optional null terminated array of referenced metrics. */
+ struct metric_ref *metric_refs;
/**
* Is there a constraint on the group of events? In which case the
* events won't be grouped.
@@ -202,20 +200,14 @@ static struct metric *metric__new(const struct pmu_event *pe,
m->metric_unit = pe->unit;
m->pctx->runtime = runtime;
m->has_constraint = metric_no_group || metricgroup__has_constraint(pe);
- INIT_LIST_HEAD(&m->metric_refs);
- m->metric_refs_cnt = 0;
+ m->metric_refs = NULL;
return m;
}
static void metric__free(struct metric *m)
{
- struct metric_ref_node *ref, *tmp;
-
- list_for_each_entry_safe(ref, tmp, &m->metric_refs, list) {
- list_del(&ref->list);
- free(ref);
- }
+ free(m->metric_refs);
expr__ctx_free(m->pctx);
free(m);
}
@@ -393,7 +385,6 @@ static int metricgroup__setup_events(struct list_head *groups,
list_for_each_entry (m, groups, nd) {
struct evsel **metric_events;
- struct metric_ref *metric_refs = NULL;
const size_t ids_size = hashmap__size(m->pctx->ids);
metric_events = calloc(sizeof(void *),
@@ -427,36 +418,8 @@ static int metricgroup__setup_events(struct list_head *groups,
break;
}
- /*
- * Collect and store collected nested expressions
- * for metric processing.
- */
- if (m->metric_refs_cnt) {
- struct metric_ref_node *ref;
-
- metric_refs = zalloc(sizeof(struct metric_ref) * (m->metric_refs_cnt + 1));
- if (!metric_refs) {
- ret = -ENOMEM;
- free(metric_events);
- free(expr);
- break;
- }
-
- i = 0;
- list_for_each_entry(ref, &m->metric_refs, list) {
- /*
- * Intentionally passing just const char pointers,
- * originally from 'struct pmu_event' object.
- * We don't need to change them, so there's no
- * need to create our own copy.
- */
- metric_refs[i].metric_name = ref->metric_name;
- metric_refs[i].metric_expr = ref->metric_expr;
- i++;
- }
- }
-
- expr->metric_refs = metric_refs;
+ expr->metric_refs = m->metric_refs;
+ m->metric_refs = NULL;
expr->metric_expr = m->metric_expr;
expr->metric_name = m->metric_name;
expr->metric_unit = m->metric_unit;
@@ -936,7 +899,6 @@ static int __add_metric(struct list_head *metric_list,
const struct visited_metric *visited,
const struct pmu_events_map *map)
{
- struct metric_ref_node *ref;
const struct visited_metric *vm;
int ret;
bool is_root = !root_metric;
@@ -962,19 +924,25 @@ static int __add_metric(struct list_head *metric_list,
return -ENOMEM;
} else {
+ int cnt = 0;
+
/*
* This metric was referenced in a metric higher in the
* tree. Check if the same metric is already resolved in the
* metric_refs list.
*/
- list_for_each_entry(ref, &root_metric->metric_refs, list) {
- if (!strcmp(pe->metric_name, ref->metric_name))
- return 0;
+ if (root_metric->metric_refs) {
+ for (; root_metric->metric_refs[cnt].metric_name; cnt++) {
+ if (!strcmp(pe->metric_name,
+ root_metric->metric_refs[cnt].metric_name))
+ return 0;
+ }
}
- /* Create reference */
- ref = malloc(sizeof(*ref));
- if (!ref)
+ /* Create reference. Need space for the entry and the terminator. */
+ root_metric->metric_refs = realloc(root_metric->metric_refs,
+ (cnt + 2) * sizeof(struct metric_ref));
+ if (!root_metric->metric_refs)
return -ENOMEM;
/*
@@ -983,11 +951,12 @@ static int __add_metric(struct list_head *metric_list,
* need to change them, so there's no need to create
* our own copy.
*/
- ref->metric_name = pe->metric_name;
- ref->metric_expr = pe->metric_expr;
+ root_metric->metric_refs[cnt].metric_name = pe->metric_name;
+ root_metric->metric_refs[cnt].metric_expr = pe->metric_expr;
- list_add(&ref->list, &root_metric->metric_refs);
- root_metric->metric_refs_cnt++;
+ /* Null terminate array. */
+ root_metric->metric_refs[cnt+1].metric_name = NULL;
+ root_metric->metric_refs[cnt+1].metric_expr = NULL;
}
/*
--
2.33.0.882.g93a45727a2-goog
Previously the broken modifier causes a usage message to printed but
nothing else. After:
$ perf stat -e 'cycles:kk' -a sleep 2
event syntax error: 'cycles:kk'
\___ Bad modifier
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
$ perf stat -e '{instructions,cycles}:kk' -a sleep 2
event syntax error: '..ns,cycles}:kk'
\___ Bad modifier
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.y | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 17c8c66f3f51..2d60f3cbe42b 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -183,6 +183,11 @@ group_def ':' PE_MODIFIER_EVENT
err = parse_events__modifier_group(list, $3);
free($3);
if (err) {
+ struct parse_events_state *parse_state = _parse_state;
+ struct parse_events_error *error = parse_state->error;
+
+ parse_events__handle_error(error, @3.first_column,
+ strdup("Bad modifier"), NULL);
free_list_evsel(list);
YYABORT;
}
@@ -240,6 +245,11 @@ event_name PE_MODIFIER_EVENT
err = parse_events__modifier_event(list, $2, false);
free($2);
if (err) {
+ struct parse_events_state *parse_state = _parse_state;
+ struct parse_events_error *error = parse_state->error;
+
+ parse_events__handle_error(error, @2.first_column,
+ strdup("Bad modifier"), NULL);
free_list_evsel(list);
YYABORT;
}
--
2.33.0.882.g93a45727a2-goog
The evsel name is strdup-ed before assignment and so can be const.
A later change will add another similar string. Using const makes it
clearer that these are not out arguments.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events-hybrid.c | 15 +++++++++------
tools/perf/util/parse-events-hybrid.h | 6 ++++--
tools/perf/util/parse-events.c | 15 ++++++++-------
tools/perf/util/parse-events.h | 7 ++++---
tools/perf/util/pmu.c | 2 +-
tools/perf/util/pmu.h | 2 +-
6 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c
index b234d95fb10a..7e44deee1343 100644
--- a/tools/perf/util/parse-events-hybrid.c
+++ b/tools/perf/util/parse-events-hybrid.c
@@ -38,7 +38,7 @@ static void config_hybrid_attr(struct perf_event_attr *attr,
static int create_event_hybrid(__u32 config_type, int *idx,
struct list_head *list,
- struct perf_event_attr *attr, char *name,
+ struct perf_event_attr *attr, const char *name,
struct list_head *config_terms,
struct perf_pmu *pmu)
{
@@ -70,7 +70,7 @@ static int pmu_cmp(struct parse_events_state *parse_state,
static int add_hw_hybrid(struct parse_events_state *parse_state,
struct list_head *list, struct perf_event_attr *attr,
- char *name, struct list_head *config_terms)
+ const char *name, struct list_head *config_terms)
{
struct perf_pmu *pmu;
int ret;
@@ -94,7 +94,8 @@ static int add_hw_hybrid(struct parse_events_state *parse_state,
}
static int create_raw_event_hybrid(int *idx, struct list_head *list,
- struct perf_event_attr *attr, char *name,
+ struct perf_event_attr *attr,
+ const char *name,
struct list_head *config_terms,
struct perf_pmu *pmu)
{
@@ -113,7 +114,7 @@ static int create_raw_event_hybrid(int *idx, struct list_head *list,
static int add_raw_hybrid(struct parse_events_state *parse_state,
struct list_head *list, struct perf_event_attr *attr,
- char *name, struct list_head *config_terms)
+ const char *name, struct list_head *config_terms)
{
struct perf_pmu *pmu;
int ret;
@@ -138,7 +139,8 @@ static int add_raw_hybrid(struct parse_events_state *parse_state,
int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
struct list_head *list,
struct perf_event_attr *attr,
- char *name, struct list_head *config_terms,
+ const char *name,
+ struct list_head *config_terms,
bool *hybrid)
{
*hybrid = false;
@@ -159,7 +161,8 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
}
int parse_events__add_cache_hybrid(struct list_head *list, int *idx,
- struct perf_event_attr *attr, char *name,
+ struct perf_event_attr *attr,
+ const char *name,
struct list_head *config_terms,
bool *hybrid,
struct parse_events_state *parse_state)
diff --git a/tools/perf/util/parse-events-hybrid.h b/tools/perf/util/parse-events-hybrid.h
index f33bd67aa851..25a4a4f73f3a 100644
--- a/tools/perf/util/parse-events-hybrid.h
+++ b/tools/perf/util/parse-events-hybrid.h
@@ -11,11 +11,13 @@
int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
struct list_head *list,
struct perf_event_attr *attr,
- char *name, struct list_head *config_terms,
+ const char *name,
+ struct list_head *config_terms,
bool *hybrid);
int parse_events__add_cache_hybrid(struct list_head *list, int *idx,
- struct perf_event_attr *attr, char *name,
+ struct perf_event_attr *attr,
+ const char *name,
struct list_head *config_terms,
bool *hybrid,
struct parse_events_state *parse_state);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1acac3e13b32..88f181a985b7 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -337,7 +337,7 @@ static int parse_events__is_name_term(struct parse_events_term *term)
return term->type_term == PARSE_EVENTS__TERM_TYPE_NAME;
}
-static char *get_config_name(struct list_head *head_terms)
+static const char *get_config_name(struct list_head *head_terms)
{
struct parse_events_term *term;
@@ -355,7 +355,7 @@ static struct evsel *
__add_event(struct list_head *list, int *idx,
struct perf_event_attr *attr,
bool init_attr,
- char *name, struct perf_pmu *pmu,
+ const char *name, struct perf_pmu *pmu,
struct list_head *config_terms, bool auto_merge_stats,
const char *cpu_list)
{
@@ -394,14 +394,14 @@ __add_event(struct list_head *list, int *idx,
}
struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
- char *name, struct perf_pmu *pmu)
+ const char *name, struct perf_pmu *pmu)
{
return __add_event(NULL, &idx, attr, false, name, pmu, NULL, false,
NULL);
}
static int add_event(struct list_head *list, int *idx,
- struct perf_event_attr *attr, char *name,
+ struct perf_event_attr *attr, const char *name,
struct list_head *config_terms)
{
return __add_event(list, idx, attr, true, name, NULL, config_terms,
@@ -464,7 +464,8 @@ int parse_events_add_cache(struct list_head *list, int *idx,
{
struct perf_event_attr attr;
LIST_HEAD(config_terms);
- char name[MAX_NAME_LEN], *config_name;
+ char name[MAX_NAME_LEN];
+ const char *config_name;
int cache_type = -1, cache_op = -1, cache_result = -1;
char *op_result[2] = { op_result1, op_result2 };
int i, n, ret;
@@ -2027,7 +2028,7 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add)
return 0;
}
-int parse_events_name(struct list_head *list, char *name)
+int parse_events_name(struct list_head *list, const char *name)
{
struct evsel *evsel;
@@ -3344,7 +3345,7 @@ char *parse_events_formats_error_string(char *additional_terms)
struct evsel *parse_events__add_event_hybrid(struct list_head *list, int *idx,
struct perf_event_attr *attr,
- char *name, struct perf_pmu *pmu,
+ const char *name, struct perf_pmu *pmu,
struct list_head *config_terms)
{
return __add_event(list, idx, attr, true, name, pmu,
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index b32ed3064c49..54d24c24d074 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -162,7 +162,7 @@ void parse_events_terms__purge(struct list_head *terms);
void parse_events__clear_array(struct parse_events_array *a);
int parse_events__modifier_event(struct list_head *list, char *str, bool add);
int parse_events__modifier_group(struct list_head *list, char *event_mod);
-int parse_events_name(struct list_head *list, char *name);
+int parse_events_name(struct list_head *list, const char *name);
int parse_events_add_tracepoint(struct list_head *list, int *idx,
const char *sys, const char *event,
struct parse_events_error *error,
@@ -199,7 +199,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
bool use_alias);
struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
- char *name, struct perf_pmu *pmu);
+ const char *name, struct perf_pmu *pmu);
int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
char *str,
@@ -266,7 +266,8 @@ int perf_pmu__test_parse_init(void);
struct evsel *parse_events__add_event_hybrid(struct list_head *list, int *idx,
struct perf_event_attr *attr,
- char *name, struct perf_pmu *pmu,
+ const char *name,
+ struct perf_pmu *pmu,
struct list_head *config_terms);
#endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index cdd6c3f6caf1..9b5039bf909a 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1906,7 +1906,7 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
}
void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
- char *name)
+ const char *name)
{
struct perf_pmu_format *format;
__u64 masks = 0, bits;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index cc9f9e001347..f9743eab07b6 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -134,7 +134,7 @@ int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
int perf_pmu__caps_parse(struct perf_pmu *pmu);
void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
- char *name);
+ const char *name);
bool perf_pmu__has_hybrid(void);
int perf_pmu__match(char *pattern, char *name, char *tok);
--
2.33.0.882.g93a45727a2-goog
On 10/7/2021 9:56 AM, Ian Rogers wrote:
> There are 4 main changes in this patch set:
> - perf metric: Modify resolution and recursion check.
> - perf parse-events: Add new "metric-id" term.
> - perf metrics: Modify setup and deduplication
> - perf metric: Allow modifiers on metrics.
Patches look all good to me from a quick read
Acked-by: Andi Kleen <[email protected]>
On 07/10/2021 17:56, Ian Rogers wrote:
> The pmu_events_map is generated at compile time and used for lookup. For
> testing purposes we need to swap the map being used. Having the
> pmu_events_map be non-const is misleading as it may be an out argument.
> Make it const and update uses so they work on const too.
>
> Signed-off-by: Ian Rogers<[email protected]>
Reviewed-by: John Garry <[email protected]>
On 08/10/2021 12:01, John Garry wrote:
> On 07/10/2021 17:56, Ian Rogers wrote:
>> The pmu_events_map is generated at compile time and used for lookup. For
>> testing purposes we need to swap the map being used. Having the
>> pmu_events_map be non-const is misleading as it may be an out argument.
>> Make it const and update uses so they work on const too.
>>
>> Signed-off-by: Ian Rogers<[email protected]>
>
> Reviewed-by: John Garry <[email protected]>
Got a compile error for this on arm64 when basing these patches on
acme/perf/core (be8ecc57f180415e8a7c1cc5620c5236be2a7e56):
$ make DEBUG=1 O=output
...<snipped>...
arch/arm64/util/pmu.c:6:24: error: conflicting types for
‘pmu_events_map__find’
struct pmu_events_map *pmu_events_map__find(void)
^~~~~~~~~~~~~~~~~~~~
In file included from arch/arm64/util/pmu.c:4:0:
arch/arm64/util/../../../util/pmu.h:126:30: note: previous declaration
of ‘pmu_events_map__find’ was here
const struct pmu_events_map *pmu_events_map__find(void);
^~~~~~~~~~~~~~~~~~~~
arch/arm64/util/pmu.c: In function ‘pmu_events_map__find’:
arch/arm64/util/pmu.c:21:10: error: return discards ‘const’ qualifier
from pointer target type [-Werror=discarded-qualifiers]
return perf_pmu__find_map(pmu);
^~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
/home/andkil01/linux/tools/build/Makefile.build:96: recipe for target
'/home/andkil01/linux/tools/perf/output/arch/arm64/util/pmu.o' failed
make[6]: ***
[/home/andkil01/linux/tools/perf/output/arch/arm64/util/pmu.o] Error 1
/home/andkil01/linux/tools/build/Makefile.build:139: recipe for target
'util' failed
make[5]: *** [util] Error 2
/home/andkil01/linux/tools/build/Makefile.build:139: recipe for target
'arm64' failed
make[4]: *** [arm64] Error 2
/home/andkil01/linux/tools/build/Makefile.build:139: recipe for target
'arch' failed
make[3]: *** [arch] Error 2
make[3]: *** Waiting for unfinished jobs....
Andrew