...
...
19
to a more consistent cpu_cycles and avoided the problem. With these
19
to a more consistent cpu_cycles and avoided the problem. With these
20
changes the problematic event will now be skipped, a large warning
20
changes the problematic event will now be skipped, a large warning
21
produced, and perf record will continue for the other PMU events. This
21
produced, and perf record will continue for the other PMU events. This
22
solution was proposed by Arnaldo.
22
solution was proposed by Arnaldo.
23
23
24
Two minor changes have been added to help with the error message and
24
v6: Rebase of v5 (dropping already merged patches):
25
to work around issues occurring with "perf stat metrics (shadow stat)
25
https://lore.kernel.org/lkml/20250109222109.567031-1-irogers@google.com/
26
test".
26
that unusually had an RFC posted for it:
27
https://lore.kernel.org/lkml/Z7Z5kv75BMML2A1q@google.com/
28
Note, this patch conflicts/contradicts:
29
https://lore.kernel.org/lkml/20250312211623.2495798-1-irogers@google.com/
30
that I posted so that we could either consistently prioritize
31
sysfs/json (these patches) or legacy events (the other
32
patches). That lack of event printing and encoding inconsistency
33
is most prominent in the encoding of events like "instructions"
34
which on hybrid are reported as "cpu_core/instructions/" but
35
"instructions" before these patches gets a legacy encoding while
36
"cpu_core/instructions/" gets a sysfs/json encoding. These patches
37
make "instructions" always get a sysfs/json encoding while the
38
alternate patches make it always get a legacy encoding.
27
39
28
The patches have only been tested on my x86 non-hybrid laptop.
40
v5: Follow Namhyung's suggestion and ignore the case where command
41
line dummy events fail to open alongside other events that all
42
fail to open. Note, the Tested-by tags are left on the series as
43
v4 and v5 were changing an error case that doesn't occur in
44
testing but was manually tested by myself.
45
46
v4: Rework the no events opening change from v3 to make it handle
47
multiple dummy events. Sadly an evlist isn't empty if it just
48
contains dummy events as the dummy event may be used with "perf
49
record -e dummy .." as a way to determine whether permission
50
issues exist. Other software events like cpu-clock would suffice
51
for this, but the using dummy genie has left the bottle.
52
53
Another problem is that we appear to have an excessive number of
54
dummy events added, for example, we can likely avoid a dummy event
55
and add sideband data to the original event. For auxtrace more
56
dummy events may be opened too. Anyway, this has led to the
57
approach taken in patch 3 where the number of dummy parsed events
58
is computed. If the number of removed/failing-to-open non-dummy
59
events matches the number of non-dummy events then we want to
60
fail, but only if there are no parsed dummy events or if there was
61
one then it must have opened. The math here is hard to read, but
62
passes my manual testing.
29
63
30
v3: Make no events opening for perf record a failure as suggested by
64
v3: Make no events opening for perf record a failure as suggested by
31
James Clark and Aditya Bodkhe <Aditya.Bodkhe1@ibm.com>. Also,
65
James Clark and Aditya Bodkhe <Aditya.Bodkhe1@ibm.com>. Also,
32
rebase.
66
rebase.
67
33
v2: Rebase and add tested-by tags from James Clark, Leo Yan and Atish
68
v2: Rebase and add tested-by tags from James Clark, Leo Yan and Atish
34
Patra who have tested on RISC-V and ARM CPUs, including the
69
Patra who have tested on RISC-V and ARM CPUs, including the
35
problem case from before.
70
problem case from before.
36
71
37
Ian Rogers (4):
72
Ian Rogers (2):
38
perf evsel: Add pmu_name helper
39
perf stat: Fix find_stat for mixed legacy/non-legacy events
40
perf record: Skip don't fail for events that don't open
73
perf record: Skip don't fail for events that don't open
41
perf parse-events: Reapply "Prefer sysfs/JSON hardware events over
74
perf parse-events: Reapply "Prefer sysfs/JSON hardware events over
42
legacy"
75
legacy"
43
76
44
tools/perf/builtin-record.c | 34 ++++++++++++---
77
tools/perf/builtin-record.c | 47 ++++++++++++++++++---
45
tools/perf/util/evsel.c | 10 +++++
46
tools/perf/util/evsel.h | 1 +
47
tools/perf/util/parse-events.c | 26 +++++++++---
78
tools/perf/util/parse-events.c | 26 +++++++++---
48
tools/perf/util/parse-events.l | 76 +++++++++++++++++-----------------
79
tools/perf/util/parse-events.l | 76 +++++++++++++++++-----------------
49
tools/perf/util/parse-events.y | 60 ++++++++++++++++++---------
80
tools/perf/util/parse-events.y | 60 ++++++++++++++++++---------
50
tools/perf/util/pmus.c | 20 +++++++--
81
4 files changed, 139 insertions(+), 70 deletions(-)
51
tools/perf/util/stat-shadow.c | 3 +-
52
8 files changed, 156 insertions(+), 74 deletions(-)
53
82
54
--
83
--
55
2.47.1.613.gc27f4b7a9f-goog
84
2.49.0.395.g12beb8f557-goog
diff view generated by jsdifflib
Deleted patch
1
Add helper to get the name of the evsel's PMU. This handles the case
2
where there's no sysfs PMU via parse_events event_type helper.
3
1
4
Signed-off-by: Ian Rogers <irogers@google.com>
5
Tested-by: James Clark <james.clark@linaro.org>
6
Tested-by: Leo Yan <leo.yan@arm.com>
7
Tested-by: Atish Patra <atishp@rivosinc.com>
8
---
9
tools/perf/util/evsel.c | 10 ++++++++++
10
tools/perf/util/evsel.h | 1 +
11
2 files changed, 11 insertions(+)
12
13
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
14
index XXXXXXX..XXXXXXX 100644
15
--- a/tools/perf/util/evsel.c
16
+++ b/tools/perf/util/evsel.c
17
@@ -XXX,XX +XXX,XX @@ int evsel__object_config(size_t object_size, int (*init)(struct evsel *evsel),
18
    return 0;
19
}
20
21
+const char *evsel__pmu_name(const struct evsel *evsel)
22
+{
23
+    struct perf_pmu *pmu = evsel__find_pmu(evsel);
24
+
25
+    if (pmu)
26
+        return pmu->name;
27
+
28
+    return event_type(evsel->core.attr.type);
29
+}
30
+
31
#define FD(e, x, y) (*(int *)xyarray__entry(e->core.fd, x, y))
32
33
int __evsel__sample_size(u64 sample_type)
34
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
35
index XXXXXXX..XXXXXXX 100644
36
--- a/tools/perf/util/evsel.h
37
+++ b/tools/perf/util/evsel.h
38
@@ -XXX,XX +XXX,XX @@ int evsel__object_config(size_t object_size,
39
             void (*fini)(struct evsel *evsel));
40
41
struct perf_pmu *evsel__find_pmu(const struct evsel *evsel);
42
+const char *evsel__pmu_name(const struct evsel *evsel);
43
bool evsel__is_aux_event(const struct evsel *evsel);
44
45
struct evsel *evsel__new_idx(struct perf_event_attr *attr, int idx);
46
--
47
2.47.1.613.gc27f4b7a9f-goog
diff view generated by jsdifflib
Deleted patch
1
Legacy events typically don't have a PMU when added leading to
2
mismatched legacy/non-legacy cases in find_stat. Use evsel__find_pmu
3
to make sure the evsel PMU is looked up. Update the evsel__find_pmu
4
code to look for the PMU using the extended config type or, for legacy
5
hardware/hw_cache events on non-hybrid systems, just use the core PMU.
6
1
7
Before:
8
```
9
$ perf stat -e cycles,cpu/instructions/ -a sleep 1
10
Performance counter stats for 'system wide':
11
12
215,309,764 cycles
13
44,326,491 cpu/instructions/
14
15
1.002555314 seconds time elapsed
16
```
17
After:
18
```
19
$ perf stat -e cycles,cpu/instructions/ -a sleep 1
20
21
Performance counter stats for 'system wide':
22
23
990,676,332 cycles
24
1,235,762,487 cpu/instructions/ # 1.25 insn per cycle
25
26
1.002667198 seconds time elapsed
27
```
28
29
Fixes: 3612ca8e2935 ("perf stat: Fix the hard-coded metrics
30
calculation on the hybrid")
31
Signed-off-by: Ian Rogers <irogers@google.com>
32
Tested-by: James Clark <james.clark@linaro.org>
33
Tested-by: Leo Yan <leo.yan@arm.com>
34
Tested-by: Atish Patra <atishp@rivosinc.com>
35
---
36
tools/perf/util/pmus.c | 20 +++++++++++++++++---
37
tools/perf/util/stat-shadow.c | 3 ++-
38
2 files changed, 19 insertions(+), 4 deletions(-)
39
40
diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
41
index XXXXXXX..XXXXXXX 100644
42
--- a/tools/perf/util/pmus.c
43
+++ b/tools/perf/util/pmus.c
44
@@ -XXX,XX +XXX,XX @@ char *perf_pmus__default_pmu_name(void)
45
struct perf_pmu *evsel__find_pmu(const struct evsel *evsel)
46
{
47
    struct perf_pmu *pmu = evsel->pmu;
48
+    bool legacy_core_type;
49
50
-    if (!pmu) {
51
-        pmu = perf_pmus__find_by_type(evsel->core.attr.type);
52
-        ((struct evsel *)evsel)->pmu = pmu;
53
+    if (pmu)
54
+        return pmu;
55
+
56
+    pmu = perf_pmus__find_by_type(evsel->core.attr.type);
57
+    legacy_core_type =
58
+        evsel->core.attr.type == PERF_TYPE_HARDWARE ||
59
+        evsel->core.attr.type == PERF_TYPE_HW_CACHE;
60
+    if (!pmu && legacy_core_type) {
61
+        if (perf_pmus__supports_extended_type()) {
62
+            u32 type = evsel->core.attr.config >> PERF_PMU_TYPE_SHIFT;
63
+
64
+            pmu = perf_pmus__find_by_type(type);
65
+        } else {
66
+            pmu = perf_pmus__find_core_pmu();
67
+        }
68
    }
69
+    ((struct evsel *)evsel)->pmu = pmu;
70
    return pmu;
71
}
72
73
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
74
index XXXXXXX..XXXXXXX 100644
75
--- a/tools/perf/util/stat-shadow.c
76
+++ b/tools/perf/util/stat-shadow.c
77
@@ -XXX,XX +XXX,XX @@ static double find_stat(const struct evsel *evsel, int aggr_idx, enum stat_type
78
{
79
    struct evsel *cur;
80
    int evsel_ctx = evsel_context(evsel);
81
+    struct perf_pmu *evsel_pmu = evsel__find_pmu(evsel);
82
83
    evlist__for_each_entry(evsel->evlist, cur) {
84
        struct perf_stat_aggr *aggr;
85
@@ -XXX,XX +XXX,XX @@ static double find_stat(const struct evsel *evsel, int aggr_idx, enum stat_type
86
         * Except the SW CLOCK events,
87
         * ignore if not the PMU we're looking for.
88
         */
89
-        if ((type != STAT_NSECS) && (evsel->pmu != cur->pmu))
90
+        if ((type != STAT_NSECS) && (evsel_pmu != evsel__find_pmu(cur)))
91
            continue;
92
93
        aggr = &cur->stats->aggr[aggr_idx];
94
--
95
2.47.1.613.gc27f4b7a9f-goog
diff view generated by jsdifflib
...
...
60
The LLC-prefetch-read event is not supported.
60
The LLC-prefetch-read event is not supported.
61
Error:
61
Error:
62
Failure to open any events for recording
62
Failure to open any events for recording
63
```
63
```
64
64
65
This is done by detecting if a dummy event was added by perf and
65
As an evlist may have dummy events that open when all command line
66
seeing if the evlist is empty without it. This allows the dummy event
66
events fail we ignore dummy events when detecting if at least some
67
still to be recorded:
67
events open. This still permits the dummy event on its own to be used
68
as a permission check:
68
```
69
```
69
$ perf record -e dummy true
70
$ perf record -e dummy true
70
[ perf record: Woken up 1 times to write data ]
71
[ perf record: Woken up 1 times to write data ]
71
[ perf record: Captured and wrote 0.046 MB perf.data ]
72
[ perf record: Captured and wrote 0.046 MB perf.data ]
72
```
73
```
73
but fail when inserted:
74
but allows failure when a dummy event is implicilty inserted or when
75
there are insufficient permissions to open it:
74
```
76
```
75
$ perf record -e LLC-prefetch-read -a true
77
$ perf record -e LLC-prefetch-read -a true
76
Error:
78
Error:
77
Failure to open event 'LLC-prefetch-read' on PMU 'cpu' which will be removed.
79
Failure to open event 'LLC-prefetch-read' on PMU 'cpu' which will be removed.
78
The LLC-prefetch-read event is not supported.
80
The LLC-prefetch-read event is not supported.
...
...
106
Signed-off-by: Ian Rogers <irogers@google.com>
108
Signed-off-by: Ian Rogers <irogers@google.com>
107
Tested-by: James Clark <james.clark@linaro.org>
109
Tested-by: James Clark <james.clark@linaro.org>
108
Tested-by: Leo Yan <leo.yan@arm.com>
110
Tested-by: Leo Yan <leo.yan@arm.com>
109
Tested-by: Atish Patra <atishp@rivosinc.com>
111
Tested-by: Atish Patra <atishp@rivosinc.com>
110
---
112
---
111
tools/perf/builtin-record.c | 34 ++++++++++++++++++++++++++++------
113
tools/perf/builtin-record.c | 47 ++++++++++++++++++++++++++++++++-----
112
1 file changed, 28 insertions(+), 6 deletions(-)
114
1 file changed, 41 insertions(+), 6 deletions(-)
113
115
114
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
116
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
115
index XXXXXXX..XXXXXXX 100644
117
index XXXXXXX..XXXXXXX 100644
116
--- a/tools/perf/builtin-record.c
118
--- a/tools/perf/builtin-record.c
117
+++ b/tools/perf/builtin-record.c
119
+++ b/tools/perf/builtin-record.c
118
@@ -XXX,XX +XXX,XX @@ struct record {
119
    bool            timestamp_filename;
120
    bool            timestamp_boundary;
121
    bool            off_cpu;
122
+    bool            dummy_event_added;
123
    const char        *filter_action;
124
    struct switch_output    switch_output;
125
    unsigned long long    samples;
126
@@ -XXX,XX +XXX,XX @@ static int record__config_tracking_events(struct record *rec)
120
@@ -XXX,XX +XXX,XX @@ static int record__config_tracking_events(struct record *rec)
127
     */
121
     */
128
    if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
122
    if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
129
     perf_pmus__num_core_pmus() > 1) {
123
     perf_pmus__num_core_pmus() > 1) {
130
-
124
-
131
+        int evlist_entries_before = evlist->core.nr_entries;
132
        /*
125
        /*
133
         * User space tasks can migrate between CPUs, so when tracing
126
         * User space tasks can migrate between CPUs, so when tracing
134
         * selected CPUs, sideband for all CPUs is still needed.
127
         * selected CPUs, sideband for all CPUs is still needed.
135
@@ -XXX,XX +XXX,XX @@ static int record__config_tracking_events(struct record *rec)
136
        if (!evsel)
137
            return -ENOMEM;
138
139
+        rec->dummy_event_added = evlist->core.nr_entries > evlist_entries_before;
140
+
141
        /*
142
         * Enable the tracking event when the process is forked for
143
         * initial_delay, immediately for system wide.
144
@@ -XXX,XX +XXX,XX @@ static int record__open(struct record *rec)
128
@@ -XXX,XX +XXX,XX @@ static int record__open(struct record *rec)
145
    struct perf_session *session = rec->session;
129
    struct perf_session *session = rec->session;
146
    struct record_opts *opts = &rec->opts;
130
    struct record_opts *opts = &rec->opts;
147
    int rc = 0;
131
    int rc = 0;
148
+    bool skipped = false;
132
+    bool skipped = false;
...
...
169
    }
153
    }
170
154
171
+    if (skipped) {
155
+    if (skipped) {
172
+        struct evsel *tmp;
156
+        struct evsel *tmp;
173
+        int idx = 0;
157
+        int idx = 0;
158
+        bool evlist_empty = true;
174
+
159
+
160
+        /* Remove evsels that failed to open and update indices. */
175
+        evlist__for_each_entry_safe(evlist, tmp, pos) {
161
+        evlist__for_each_entry_safe(evlist, tmp, pos) {
176
+            if (pos->skippable)
162
+            if (pos->skippable) {
177
+                evlist__remove(evlist, pos);
163
+                evlist__remove(evlist, pos);
164
+                continue;
165
+            }
166
+
167
+            /*
168
+             * Note, dummy events may be command line parsed or
169
+             * added by the tool. We care about supporting `perf
170
+             * record -e dummy` which may be used as a permission
171
+             * check. Dummy events that are added to the command
172
+             * line and opened along with other events that fail,
173
+             * will still fail as if the dummy events were tool
174
+             * added events for the sake of code simplicity.
175
+             */
176
+            if (!evsel__is_dummy_event(pos))
177
+                evlist_empty = false;
178
+        }
178
+        }
179
+        evlist__for_each_entry(evlist, pos) {
179
+        evlist__for_each_entry(evlist, pos) {
180
+            pos->core.idx = idx++;
180
+            pos->core.idx = idx++;
181
+        }
181
+        }
182
+        if (idx == 0 || (idx == 1 && rec->dummy_event_added)) {
182
+        /* If list is empty then fail. */
183
+        if (evlist_empty) {
183
+            ui__error("Failure to open any events for recording.\n");
184
+            ui__error("Failure to open any events for recording.\n");
184
+            rc = -1;
185
+            rc = -1;
185
+            goto out;
186
+            goto out;
186
+        }
187
+        }
187
+    }
188
+    }
188
    if (symbol_conf.kptr_restrict && !evlist__exclude_kernel(evlist)) {
189
    if (symbol_conf.kptr_restrict && !evlist__exclude_kernel(evlist)) {
189
        pr_warning(
190
        pr_warning(
190
"WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,\n"
191
"WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,\n"
191
--
192
--
192
2.47.1.613.gc27f4b7a9f-goog
193
2.49.0.395.g12beb8f557-goog
diff view generated by jsdifflib
...
...
359
+                /*head_config=*/NULL, /*wildcard=*/false);
359
+                /*head_config=*/NULL, /*wildcard=*/false);
360
    if (err)
360
    if (err)
361
        PE_ABORT(err);
361
        PE_ABORT(err);
362
    $$ = list;
362
    $$ = list;
363
--
363
--
364
2.47.1.613.gc27f4b7a9f-goog
364
2.49.0.395.g12beb8f557-goog
diff view generated by jsdifflib