2023-12-14 14:47:01

by Liang, Kan

[permalink] [raw]
Subject: [PATCH V4 1/2] perf top: Use evsel's cpus to replace user_requested_cpus

From: Kan Liang <[email protected]>

perf top errors out on a hybrid machine
$perf top

Error:
The cycles:P event is not supported.

The perf top expects that the "cycles" is collected on all CPUs in the
system. But for hybrid there is no single "cycles" event which can cover
all CPUs. Perf has to split it into two cycles events, e.g.,
cpu_core/cycles/ and cpu_atom/cycles/. Each event has its own CPU mask.
If a event is opened on the unsupported CPU. The open fails. That's the
reason of the above error out.

Perf should only open the cycles event on the corresponding CPU. The
commit ef91871c960e ("perf evlist: Propagate user CPU maps intersecting
core PMU maps") intersect the requested CPU map with the CPU map of the
PMU. Use the evsel's cpus to replace user_requested_cpus.

The evlist's threads are also propagated to the evsel's threads in
__perf_evlist__propagate_maps(). For a system-wide event, perf appends
a dummy event and assign it to the evsel's threads. For a per-thread
event, the evlist's thread_map is assigned to the evsel's threads. The
same as the other tools, e.g., perf record, using the evsel's threads
when opening an event.

Reported-by: Arnaldo Carvalho de Melo <[email protected]>
Closes: https://lore.kernel.org/linux-perf-users/[email protected]/
Reviewed-by: Ian Rogers <[email protected]>
Signed-off-by: Kan Liang <[email protected]>
---

No changes since V2

tools/perf/builtin-top.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index ea8c7eca5eee..cce9350177e2 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1027,8 +1027,8 @@ static int perf_top__start_counters(struct perf_top *top)

evlist__for_each_entry(evlist, counter) {
try_again:
- if (evsel__open(counter, top->evlist->core.user_requested_cpus,
- top->evlist->core.threads) < 0) {
+ if (evsel__open(counter, counter->core.cpus,
+ counter->core.threads) < 0) {

/*
* Specially handle overwrite fall back.
--
2.35.1


2023-12-14 14:47:53

by Liang, Kan

[permalink] [raw]
Subject: [PATCH V4 2/2] perf top: Uniform the event name for the hybrid machine

From: Kan Liang <[email protected]>

It's hard to distinguish the default cycles events among hybrid PMUs.
For example,

$perf top
Available samples
385 cycles:P
903 cycles:P

The other tool, e.g., perf record, uniforms the event name and adds the
hybrid PMU name before opening the event. So the events can be easily
distinguished. Apply the same methodology for the perf top as well.

The evlist__uniquify_name() will be invoked by both record and top.
Move it to util/evlist.c

With the patch
$perf top
Available samples
148 cpu_atom/cycles:P/
1K cpu_core/cycles:P/

Reviewed-by: Ian Rogers <[email protected]>
Signed-off-by: Kan Liang <[email protected]>
---

Changes since V3:
- rename the function to evlist__uniquify_name() and move it to
util/evlist.c. (Namhyung)
- Add Reviewed-by from Ian
(The change is just a rename and relocation. I think the tag still
stands. Please let me know if there are other opinions.)

tools/perf/builtin-record.c | 28 +---------------------------
tools/perf/builtin-top.c | 1 +
tools/perf/util/evlist.c | 25 +++++++++++++++++++++++++
tools/perf/util/evlist.h | 1 +
4 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index dcf288a4fb9a..3a7e7484995d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2216,32 +2216,6 @@ static void hit_auxtrace_snapshot_trigger(struct record *rec)
}
}

-static void record__uniquify_name(struct record *rec)
-{
- struct evsel *pos;
- struct evlist *evlist = rec->evlist;
- char *new_name;
- int ret;
-
- if (perf_pmus__num_core_pmus() == 1)
- return;
-
- evlist__for_each_entry(evlist, pos) {
- if (!evsel__is_hybrid(pos))
- continue;
-
- if (strchr(pos->name, '/'))
- continue;
-
- ret = asprintf(&new_name, "%s/%s/",
- pos->pmu_name, pos->name);
- if (ret) {
- free(pos->name);
- pos->name = new_name;
- }
- }
-}
-
static int record__terminate_thread(struct record_thread *thread_data)
{
int err;
@@ -2475,7 +2449,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
if (data->is_pipe && rec->evlist->core.nr_entries == 1)
rec->opts.sample_id = true;

- record__uniquify_name(rec);
+ evlist__uniquify_name(rec->evlist);

/* Debug message used by test scripts */
pr_debug3("perf record opening and mmapping events\n");
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index cce9350177e2..cd64ae44ccbd 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1299,6 +1299,7 @@ static int __cmd_top(struct perf_top *top)
}
}

+ evlist__uniquify_name(top->evlist);
ret = perf_top__start_counters(top);
if (ret)
return ret;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index e36da58522ef..fda411ab41e7 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -2518,3 +2518,28 @@ void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_lis
}
perf_cpu_map__put(user_requested_cpus);
}
+
+void evlist__uniquify_name(struct evlist *evlist)
+{
+ struct evsel *pos;
+ char *new_name;
+ int ret;
+
+ if (perf_pmus__num_core_pmus() == 1)
+ return;
+
+ evlist__for_each_entry(evlist, pos) {
+ if (!evsel__is_hybrid(pos))
+ continue;
+
+ if (strchr(pos->name, '/'))
+ continue;
+
+ ret = asprintf(&new_name, "%s/%s/",
+ pos->pmu_name, pos->name);
+ if (ret) {
+ free(pos->name);
+ pos->name = new_name;
+ }
+ }
+}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 98e7ddb2bd30..cb91dc9117a2 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -442,5 +442,6 @@ struct evsel *evlist__find_evsel(struct evlist *evlist, int idx);
int evlist__scnprintf_evsels(struct evlist *evlist, size_t size, char *bf);
void evlist__check_mem_load_aux(struct evlist *evlist);
void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_list);
+void evlist__uniquify_name(struct evlist *evlist);

#endif /* __PERF_EVLIST_H */
--
2.35.1

2023-12-14 22:14:52

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH V4 2/2] perf top: Uniform the event name for the hybrid machine

Em Thu, Dec 14, 2023 at 06:46:12AM -0800, [email protected] escreveu:
> From: Kan Liang <[email protected]>
>
> It's hard to distinguish the default cycles events among hybrid PMUs.
> For example,
>
> $perf top
> Available samples
> 385 cycles:P
> 903 cycles:P

Thanks, tested and applied both patches to perf-tools-next.


Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>