2024-03-08 00:19:45

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 0/6] Extra verbose/perf-list details

Add more encoding detail and raw event details in perf list. Add PMU
name and reverse lookup from config to event name to
perf_event_attr_fprintf. This makes the verbose output easier to read,
and the perf list information more specific.

v3. Fix to reverse lookup to ensure or aliases are loaded and if
getting the config value fails for an event/alias just continue to
the next one.
v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
becomes "Raw event descriptor" add assert to keep term numbers in
sync, fix a commit message.

Ian Rogers (6):
perf list: Add tracepoint encoding to detailed output
perf pmu: Drop "default_core" from alias names
perf list: Allow wordwrap to wrap on commas
perf list: Give more details about raw event encodings
perf tools: Use pmus to describe type from attribute
perf tools: Add/use PMU reverse lookup from config to name

tools/perf/builtin-list.c | 21 ++++-
tools/perf/util/perf_event_attr_fprintf.c | 26 +++++--
tools/perf/util/pmu.c | 82 +++++++++++++++++++-
tools/perf/util/pmu.h | 4 +
tools/perf/util/pmus.c | 94 +++++++++++++++++++++++
tools/perf/util/pmus.h | 1 +
tools/perf/util/print-events.c | 55 +++++++------
7 files changed, 242 insertions(+), 41 deletions(-)

--
2.44.0.278.ge034bb2e1d-goog



2024-03-08 00:21:39

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 3/6] perf list: Allow wordwrap to wrap on commas

A raw event encoding may be a block with terms separated by commas. If
wrapping such a string it would be useful to break at the commas, so
add this ability to wordwrap.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/builtin-list.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 02bf608d585e..e0fe3d178d63 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -22,6 +22,7 @@
#include <subcmd/pager.h>
#include <subcmd/parse-options.h>
#include <linux/zalloc.h>
+#include <ctype.h>
#include <stdarg.h>
#include <stdio.h>

@@ -76,26 +77,38 @@ static void default_print_start(void *ps)

static void default_print_end(void *print_state __maybe_unused) {}

+static const char *skip_spaces_or_commas(const char *str)
+{
+ while (isspace(*str) || *str == ',')
+ ++str;
+ return str;
+}
+
static void wordwrap(FILE *fp, const char *s, int start, int max, int corr)
{
int column = start;
int n;
bool saw_newline = false;
+ bool comma = false;

while (*s) {
- int wlen = strcspn(s, " \t\n");
+ int wlen = strcspn(s, " ,\t\n");
+ const char *sep = comma ? "," : " ";

if ((column + wlen >= max && column > start) || saw_newline) {
- fprintf(fp, "\n%*s", start, "");
+ fprintf(fp, comma ? ",\n%*s" : "\n%*s", start, "");
column = start + corr;
}
- n = fprintf(fp, "%s%.*s", column > start ? " " : "", wlen, s);
+ if (column <= start)
+ sep = "";
+ n = fprintf(fp, "%s%.*s", sep, wlen, s);
if (n <= 0)
break;
saw_newline = s[wlen] == '\n';
s += wlen;
+ comma = s[0] == ',';
column += n;
- s = skip_spaces(s);
+ s = skip_spaces_or_commas(s);
}
}

--
2.44.0.278.ge034bb2e1d-goog


2024-03-08 18:45:09

by Liang, Kan

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] Extra verbose/perf-list details



On 2024-03-07 7:19 p.m., Ian Rogers wrote:
> Add more encoding detail and raw event details in perf list. Add PMU
> name and reverse lookup from config to event name to
> perf_event_attr_fprintf. This makes the verbose output easier to read,
> and the perf list information more specific.
>
> v3. Fix to reverse lookup to ensure or aliases are loaded and if
> getting the config value fails for an event/alias just continue to
> the next one.
> v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
> becomes "Raw event descriptor" add assert to keep term numbers in
> sync, fix a commit message.
>
> Ian Rogers (6):
> perf list: Add tracepoint encoding to detailed output
> perf pmu: Drop "default_core" from alias names
> perf list: Allow wordwrap to wrap on commas
> perf list: Give more details about raw event encodings
> perf tools: Use pmus to describe type from attribute
> perf tools: Add/use PMU reverse lookup from config to name

The patch series look good to me.
I verified it on a hybrid machine. The new format is the same as the
advertise.

Tested-by: Kan Liang <[email protected]>

Thanks,
Kan
>
> tools/perf/builtin-list.c | 21 ++++-
> tools/perf/util/perf_event_attr_fprintf.c | 26 +++++--
> tools/perf/util/pmu.c | 82 +++++++++++++++++++-
> tools/perf/util/pmu.h | 4 +
> tools/perf/util/pmus.c | 94 +++++++++++++++++++++++
> tools/perf/util/pmus.h | 1 +
> tools/perf/util/print-events.c | 55 +++++++------
> 7 files changed, 242 insertions(+), 41 deletions(-)
>

2024-03-08 00:20:53

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 5/6] perf tools: Use pmus to describe type from attribute

When dumping a perf_event_attr, use pmus to find the PMU and its name
by the type number. This allows dynamically added PMUs to be described.

Before:
```
$ perf stat -vv -e data_read true
..
perf_event_attr:
type 24
size 136
config 0x20ff
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
..
```

After:
```
$ perf stat -vv -e data_read true
..
perf_event_attr:
type 24 (uncore_imc_free_running_0)
size 136
config 0x20ff
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
..
```

However, it also means that when we have a PMU name we prefer it to a
hard coded name:

Before:
```
$ perf stat -vv -e faults true
..
perf_event_attr:
type 1 (PERF_TYPE_SOFTWARE)
size 136
config 0x2 (PERF_COUNT_SW_PAGE_FAULTS)
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
enable_on_exec 1
exclude_guest 1
..
```

After:
```
$ perf stat -vv -e faults true
..
perf_event_attr:
type 1 (software)
size 136
config 0x2 (PERF_COUNT_SW_PAGE_FAULTS)
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
enable_on_exec 1
exclude_guest 1
..
```

It feels more consistent to do this, rather than only prefer a PMU
name when a hard coded name isn't available.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/perf_event_attr_fprintf.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 8f04d3b7f3ec..29e66835da3a 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -7,6 +7,8 @@
#include <linux/types.h>
#include <linux/perf_event.h>
#include "util/evsel_fprintf.h"
+#include "util/pmu.h"
+#include "util/pmus.h"
#include "trace-event.h"

struct bit_names {
@@ -75,9 +77,12 @@ static void __p_read_format(char *buf, size_t size, u64 value)
}

#define ENUM_ID_TO_STR_CASE(x) case x: return (#x);
-static const char *stringify_perf_type_id(u64 value)
+static const char *stringify_perf_type_id(struct perf_pmu *pmu, u32 type)
{
- switch (value) {
+ if (pmu)
+ return pmu->name;
+
+ switch (type) {
ENUM_ID_TO_STR_CASE(PERF_TYPE_HARDWARE)
ENUM_ID_TO_STR_CASE(PERF_TYPE_SOFTWARE)
ENUM_ID_TO_STR_CASE(PERF_TYPE_TRACEPOINT)
@@ -175,9 +180,9 @@ do { \
#define print_id_unsigned(_s) PRINT_ID(_s, "%"PRIu64)
#define print_id_hex(_s) PRINT_ID(_s, "%#"PRIx64)

-static void __p_type_id(char *buf, size_t size, u64 value)
+static void __p_type_id(struct perf_pmu *pmu, char *buf, size_t size, u64 value)
{
- print_id_unsigned(stringify_perf_type_id(value));
+ print_id_unsigned(stringify_perf_type_id(pmu, value));
}

static void __p_config_hw_id(char *buf, size_t size, u64 value)
@@ -246,7 +251,7 @@ static void __p_config_id(char *buf, size_t size, u32 type, u64 value)
#define p_sample_type(val) __p_sample_type(buf, BUF_SIZE, val)
#define p_branch_sample_type(val) __p_branch_sample_type(buf, BUF_SIZE, val)
#define p_read_format(val) __p_read_format(buf, BUF_SIZE, val)
-#define p_type_id(val) __p_type_id(buf, BUF_SIZE, val)
+#define p_type_id(val) __p_type_id(pmu, buf, BUF_SIZE, val)
#define p_config_id(val) __p_config_id(buf, BUF_SIZE, attr->type, val)

#define PRINT_ATTRn(_n, _f, _p, _a) \
@@ -262,6 +267,7 @@ do { \
int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
attr__fprintf_f attr__fprintf, void *priv)
{
+ struct perf_pmu *pmu = perf_pmus__find_by_type(attr->type);
char buf[BUF_SIZE];
int ret = 0;

--
2.44.0.278.ge034bb2e1d-goog


2024-03-08 00:20:13

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 2/6] perf pmu: Drop "default_core" from alias names

"default_core" is used by jevents.py for json events' PMU name when
none is specified. On x86 the "default_core" is typically the PMU
"cpu". When creating an alias see if the event's PMU name is
"default_core" in which case don't record it. This means in places
like "perf list" the PMU's name will be used in its place.

Before:
```
$ perf list --details
..
cache:
l1d.replacement
[Counts the number of cache lines replaced in L1 data cache]
default_core/event=0x51,period=0x186a3,umask=0x1/
..
```

After:
```
$ perf list --details
..
cache:
l1d.replacement
[Counts the number of cache lines replaced in L1 data cache. Unit: cpu]
cpu/event=0x51,period=0x186a3,umask=0x1/
..
```

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/pmu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index f39cbbc1a7ec..24be587e3537 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -518,7 +518,8 @@ static int perf_pmu__new_alias(struct perf_pmu *pmu, const char *name,
unit = pe->unit;
perpkg = pe->perpkg;
deprecated = pe->deprecated;
- pmu_name = pe->pmu;
+ if (pe->pmu && strcmp(pe->pmu, "default_core"))
+ pmu_name = pe->pmu;
}

alias = zalloc(sizeof(*alias));
--
2.44.0.278.ge034bb2e1d-goog


2024-03-08 00:20:00

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 1/6] perf list: Add tracepoint encoding to detailed output

The tracepoint id holds the config value and is probed in determining
what an event is. Add reading of the id so that we can display the
event encoding as:

```
$ perf list --details
..
alarmtimer:alarmtimer_cancel [Tracepoint event]
tracepoint/config=0x18c/
..
```

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/print-events.c | 35 ++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
index 7b54e9385442..e0d2b49bab66 100644
--- a/tools/perf/util/print-events.c
+++ b/tools/perf/util/print-events.c
@@ -9,6 +9,7 @@
#include <unistd.h>

#include <api/fs/tracing_path.h>
+#include <api/io.h>
#include <linux/stddef.h>
#include <linux/perf_event.h>
#include <linux/zalloc.h>
@@ -92,34 +93,48 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus

evt_items = scandirat(events_fd, sys_dirent->d_name, &evt_namelist, NULL, alphasort);
for (int j = 0; j < evt_items; j++) {
+ /*
+ * Buffer sized at twice the max filename length + 1
+ * separator + 1 \0 terminator.
+ */
+ char buf[NAME_MAX * 2 + 2];
+ /* 16 possible hex digits and 22 other characters and \0. */
+ char encoding[16 + 22];
struct dirent *evt_dirent = evt_namelist[j];
- char evt_path[MAXPATHLEN];
- int evt_fd;
+ struct io id;
+ __u64 config;

if (evt_dirent->d_type != DT_DIR ||
!strcmp(evt_dirent->d_name, ".") ||
!strcmp(evt_dirent->d_name, ".."))
goto next_evt;

- snprintf(evt_path, sizeof(evt_path), "%s/id", evt_dirent->d_name);
- evt_fd = openat(dir_fd, evt_path, O_RDONLY);
- if (evt_fd < 0)
+ snprintf(buf, sizeof(buf), "%s/id", evt_dirent->d_name);
+ io__init(&id, openat(dir_fd, buf, O_RDONLY), buf, sizeof(buf));
+
+ if (id.fd < 0)
+ goto next_evt;
+
+ if (io__get_dec(&id, &config) < 0) {
+ close(id.fd);
goto next_evt;
- close(evt_fd);
+ }
+ close(id.fd);

- snprintf(evt_path, MAXPATHLEN, "%s:%s",
+ snprintf(buf, sizeof(buf), "%s:%s",
sys_dirent->d_name, evt_dirent->d_name);
+ snprintf(encoding, sizeof(encoding), "tracepoint/config=0x%llx/", config);
print_cb->print_event(print_state,
/*topic=*/NULL,
- /*pmu_name=*/NULL,
- evt_path,
+ /*pmu_name=*/NULL, /* really "tracepoint" */
+ /*event_name=*/buf,
/*event_alias=*/NULL,
/*scale_unit=*/NULL,
/*deprecated=*/false,
"Tracepoint event",
/*desc=*/NULL,
/*long_desc=*/NULL,
- /*encoding_desc=*/NULL);
+ encoding);
next_evt:
free(evt_namelist[j]);
}
--
2.44.0.278.ge034bb2e1d-goog


2024-03-08 00:21:17

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 6/6] perf tools: Add/use PMU reverse lookup from config to name

Add perf_pmu__name_from_config that does a reverse lookup from a
config number to an alias name. The lookup is expensive as the config
is computed for every alias by filling in a perf_event_attr, but this
is only done when verbose output is enabled. The lookup also only
considers config, and not config1, config2 or config3.

An example of the output:
```
$ perf stat -vv -e data_read true
..
perf_event_attr:
type 24 (uncore_imc_free_running_0)
size 136
config 0x20ff (data_read)
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
..
```

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/perf_event_attr_fprintf.c | 10 ++++++++--
tools/perf/util/pmu.c | 18 ++++++++++++++++++
tools/perf/util/pmu.h | 1 +
3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 29e66835da3a..59fbbba79697 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -222,8 +222,14 @@ static void __p_config_tracepoint_id(char *buf, size_t size, u64 value)
}
#endif

-static void __p_config_id(char *buf, size_t size, u32 type, u64 value)
+static void __p_config_id(struct perf_pmu *pmu, char *buf, size_t size, u32 type, u64 value)
{
+ const char *name = perf_pmu__name_from_config(pmu, value);
+
+ if (name) {
+ print_id_hex(name);
+ return;
+ }
switch (type) {
case PERF_TYPE_HARDWARE:
return __p_config_hw_id(buf, size, value);
@@ -252,7 +258,7 @@ static void __p_config_id(char *buf, size_t size, u32 type, u64 value)
#define p_branch_sample_type(val) __p_branch_sample_type(buf, BUF_SIZE, val)
#define p_read_format(val) __p_read_format(buf, BUF_SIZE, val)
#define p_type_id(val) __p_type_id(pmu, buf, BUF_SIZE, val)
-#define p_config_id(val) __p_config_id(buf, BUF_SIZE, attr->type, val)
+#define p_config_id(val) __p_config_id(pmu, buf, BUF_SIZE, attr->type, val)

#define PRINT_ATTRn(_n, _f, _p, _a) \
do { \
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index e24bc3b8f696..97ad299f463f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -2145,3 +2145,21 @@ void perf_pmu__delete(struct perf_pmu *pmu)
zfree(&pmu->id);
free(pmu);
}
+
+const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config)
+{
+ struct perf_pmu_alias *event;
+
+ if (!pmu)
+ return NULL;
+
+ pmu_add_cpu_aliases(pmu);
+ list_for_each_entry(event, &pmu->aliases, list) {
+ struct perf_event_attr attr = {.config = 0,};
+ int ret = perf_pmu__config(pmu, &attr, &event->terms, NULL);
+
+ if (ret == 0 && config == attr.config)
+ return event->name;
+ }
+ return NULL;
+}
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 9f5284b29ecf..152700f78455 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -276,5 +276,6 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pmus);
void perf_pmu__delete(struct perf_pmu *pmu);
struct perf_pmu *perf_pmus__find_core_pmu(void);
+const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config);

#endif /* __PMU_H */
--
2.44.0.278.ge034bb2e1d-goog


2024-03-08 00:21:04

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 4/6] perf list: Give more details about raw event encodings

List all the PMUs, not just the first core one, and list real format
specifiers with value ranges.

Before:
```
$ perf list
..
rNNN [Raw hardware event descriptor]
cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
[(see 'man perf-list' on how to encode it)]
mem:<addr>[/len][:access] [Hardware breakpoint]
..
```

After:
```
$ perf list
..
rNNN [Raw event descriptor]
cpu/event=0..255,pc,edge,.../modifier [Raw event descriptor]
[(see 'man perf-list' or 'man perf-record' on how to encode it)]
breakpoint//modifier [Raw event descriptor]
cstate_core/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
i915/i915_eventid=0..0x1fffff/modifier [Raw event descriptor]
intel_bts//modifier [Raw event descriptor]
intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw event descriptor]
kprobe/retprobe/modifier [Raw event descriptor]
msr/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
power/event=0..255/modifier [Raw event descriptor]
software//modifier [Raw event descriptor]
tracepoint//modifier [Raw event descriptor]
uncore_arb/event=0..255,edge,inv,.../modifier [Raw event descriptor]
uncore_cbox/event=0..255,edge,inv,.../modifier [Raw event descriptor]
uncore_clock/event=0..255/modifier [Raw event descriptor]
uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
mem:<addr>[/len][:access] [Hardware breakpoint]
..
```

With '--details' provide more details on the formats encoding:
```
cpu/event=0..255,pc,edge,.../modifier [Raw event descriptor]
[(see 'man perf-list' or 'man perf-record' on how to encode it)]
cpu/event=0..255,pc,edge,offcore_rsp=0..0xffffffffffffffff,ldlat=0..0xffff,inv,
umask=0..255,frontend=0..0xffffff,cmask=0..255,config=0..0xffffffffffffffff,
config1=0..0xffffffffffffffff,config2=0..0xffffffffffffffff,config3=0..0xffffffffffffffff,
name=string,period=number,freq=number,branch_type=(u|k|hv|any|...),time,
call-graph=(fp|dwarf|lbr),stack-size=number,max-stack=number,nr=number,inherit,no-inherit,
overwrite,no-overwrite,percore,aux-output,aux-sample-size=number/modifier
breakpoint//modifier [Raw event descriptor]
breakpoint//modifier
cstate_core/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
cstate_core/event=0..0xffffffffffffffff/modifier
cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
cstate_pkg/event=0..0xffffffffffffffff/modifier
i915/i915_eventid=0..0x1fffff/modifier [Raw event descriptor]
i915/i915_eventid=0..0x1fffff/modifier
intel_bts//modifier [Raw event descriptor]
intel_bts//modifier
intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw event descriptor]
intel_pt/ptw,event,cyc_thresh=0..15,pt,notnt,branch,tsc,pwr_evt,fup_on_ptw,cyc,noretcomp,
mtc,psb_period=0..15,mtc_period=0..15/modifier
kprobe/retprobe/modifier [Raw event descriptor]
kprobe/retprobe/modifier
msr/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
msr/event=0..0xffffffffffffffff/modifier
power/event=0..255/modifier [Raw event descriptor]
power/event=0..255/modifier
software//modifier [Raw event descriptor]
software//modifier
tracepoint//modifier [Raw event descriptor]
tracepoint//modifier
uncore_arb/event=0..255,edge,inv,.../modifier [Raw event descriptor]
uncore_arb/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
uncore_cbox/event=0..255,edge,inv,.../modifier [Raw event descriptor]
uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
uncore_clock/event=0..255/modifier [Raw event descriptor]
uncore_clock/event=0..255/modifier
uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
uncore_imc_free_running/event=0..255,umask=0..255/modifier
uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
```

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/pmu.c | 61 +++++++++++++++++++++-
tools/perf/util/pmu.h | 3 ++
tools/perf/util/pmus.c | 94 ++++++++++++++++++++++++++++++++++
tools/perf/util/pmus.h | 1 +
tools/perf/util/print-events.c | 20 +-------
5 files changed, 160 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 24be587e3537..e24bc3b8f696 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1603,6 +1603,61 @@ bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name)
return false;
}

+int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb)
+{
+ static const char *const terms[] = {
+ "config=0..0xffffffffffffffff",
+ "config1=0..0xffffffffffffffff",
+ "config2=0..0xffffffffffffffff",
+ "config3=0..0xffffffffffffffff",
+ "name=string",
+ "period=number",
+ "freq=number",
+ "branch_type=(u|k|hv|any|...)",
+ "time",
+ "call-graph=(fp|dwarf|lbr)",
+ "stack-size=number",
+ "max-stack=number",
+ "nr=number",
+ "inherit",
+ "no-inherit",
+ "overwrite",
+ "no-overwrite",
+ "percore",
+ "aux-output",
+ "aux-sample-size=number",
+ };
+ struct perf_pmu_format *format;
+ int ret;
+
+ /*
+ * max-events and driver-config are missing above as are the internal
+ * types user, metric-id, raw, legacy cache and hardware. Assert against
+ * the enum parse_events__term_type so they are kept in sync.
+ */
+ _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);
+ list_for_each_entry(format, &pmu->format, list) {
+ perf_pmu_format__load(pmu, format);
+ ret = cb(state, format->name, (int)format->value, format->bits);
+ if (ret)
+ return ret;
+ }
+ if (!pmu->is_core)
+ return 0;
+
+ for (size_t i = 0; i < ARRAY_SIZE(terms); i++) {
+ int config = PERF_PMU_FORMAT_VALUE_CONFIG;
+
+ if (i < PERF_PMU_FORMAT_VALUE_CONFIG_END)
+ config = i;
+
+ ret = cb(state, terms[i], config, /*bits=*/NULL);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
bool is_pmu_core(const char *name)
{
return !strcmp(name, "cpu") || !strcmp(name, "cpum_cf") || is_sysfs_pmu_core(name);
@@ -1697,8 +1752,12 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
pmu_add_cpu_aliases(pmu);
list_for_each_entry(event, &pmu->aliases, list) {
size_t buf_used;
+ int pmu_name_len;

info.pmu_name = event->pmu_name ?: pmu->name;
+ pmu_name_len = skip_duplicate_pmus
+ ? pmu_name_len_no_suffix(info.pmu_name, /*num=*/NULL)
+ : (int)strlen(info.pmu_name);
info.alias = NULL;
if (event->desc) {
info.name = event->name;
@@ -1723,7 +1782,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
info.encoding_desc = buf + buf_used;
parse_events_terms__to_strbuf(&event->terms, &sb);
buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
- "%s/%s/", info.pmu_name, sb.buf) + 1;
+ "%.*s/%s/", pmu_name_len, info.pmu_name, sb.buf) + 1;
info.topic = event->topic;
info.str = sb.buf;
info.deprecated = event->deprecated;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index e35d985206db..9f5284b29ecf 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -196,6 +196,8 @@ struct pmu_event_info {
};

typedef int (*pmu_event_callback)(void *state, struct pmu_event_info *info);
+typedef int (*pmu_format_callback)(void *state, const char *name, int config,
+ const unsigned long *bits);

void pmu_add_sys_aliases(struct perf_pmu *pmu);
int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
@@ -215,6 +217,7 @@ int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, p
int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd, bool eager_load);
void perf_pmu_format__set_value(void *format, int config, unsigned long *bits);
bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name);
+int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb);

bool is_pmu_core(const char *name);
bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu);
diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index 16505071d362..2fd369e45832 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -16,6 +16,7 @@
#include "pmus.h"
#include "pmu.h"
#include "print-events.h"
+#include "strbuf.h"

/*
* core_pmus: A PMU belongs to core_pmus if it's name is "cpu" or it's sysfs
@@ -503,6 +504,99 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
zfree(&aliases);
}

+struct build_format_string_args {
+ struct strbuf short_string;
+ struct strbuf long_string;
+ int num_formats;
+};
+
+static int build_format_string(void *state, const char *name, int config,
+ const unsigned long *bits)
+{
+ struct build_format_string_args *args = state;
+ unsigned int num_bits;
+ int ret1, ret2 = 0;
+
+ (void)config;
+ args->num_formats++;
+ if (args->num_formats > 1) {
+ strbuf_addch(&args->long_string, ',');
+ if (args->num_formats < 4)
+ strbuf_addch(&args->short_string, ',');
+ }
+ num_bits = bits ? bitmap_weight(bits, PERF_PMU_FORMAT_BITS) : 0;
+ if (num_bits <= 1) {
+ ret1 = strbuf_addf(&args->long_string, "%s", name);
+ if (args->num_formats < 4)
+ ret2 = strbuf_addf(&args->short_string, "%s", name);
+ } else if (num_bits > 8) {
+ ret1 = strbuf_addf(&args->long_string, "%s=0..0x%llx", name,
+ ULLONG_MAX >> (64 - num_bits));
+ if (args->num_formats < 4) {
+ ret2 = strbuf_addf(&args->short_string, "%s=0..0x%llx", name,
+ ULLONG_MAX >> (64 - num_bits));
+ }
+ } else {
+ ret1 = strbuf_addf(&args->long_string, "%s=0..%llu", name,
+ ULLONG_MAX >> (64 - num_bits));
+ if (args->num_formats < 4) {
+ ret2 = strbuf_addf(&args->short_string, "%s=0..%llu", name,
+ ULLONG_MAX >> (64 - num_bits));
+ }
+ }
+ return ret1 < 0 ? ret1 : (ret2 < 0 ? ret2 : 0);
+}
+
+void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, void *print_state)
+{
+ bool skip_duplicate_pmus = print_cb->skip_duplicate_pmus(print_state);
+ struct perf_pmu *(*scan_fn)(struct perf_pmu *);
+ struct perf_pmu *pmu = NULL;
+
+ if (skip_duplicate_pmus)
+ scan_fn = perf_pmus__scan_skip_duplicates;
+ else
+ scan_fn = perf_pmus__scan;
+
+ while ((pmu = scan_fn(pmu)) != NULL) {
+ struct build_format_string_args format_args = {
+ .short_string = STRBUF_INIT,
+ .long_string = STRBUF_INIT,
+ .num_formats = 0,
+ };
+ int len = pmu_name_len_no_suffix(pmu->name, /*num=*/NULL);
+ const char *desc = "(see 'man perf-list' or 'man perf-record' on how to encode it)";
+
+ if (!pmu->is_core)
+ desc = NULL;
+
+ strbuf_addf(&format_args.short_string, "%.*s/", len, pmu->name);
+ strbuf_addf(&format_args.long_string, "%.*s/", len, pmu->name);
+ perf_pmu__for_each_format(pmu, &format_args, build_format_string);
+
+ if (format_args.num_formats > 3)
+ strbuf_addf(&format_args.short_string, ",.../modifier");
+ else
+ strbuf_addf(&format_args.short_string, "/modifier");
+
+ strbuf_addf(&format_args.long_string, "/modifier");
+ print_cb->print_event(print_state,
+ /*topic=*/NULL,
+ /*pmu_name=*/NULL,
+ format_args.short_string.buf,
+ /*event_alias=*/NULL,
+ /*scale_unit=*/NULL,
+ /*deprecated=*/false,
+ "Raw event descriptor",
+ desc,
+ /*long_desc=*/NULL,
+ format_args.long_string.buf);
+
+ strbuf_release(&format_args.short_string);
+ strbuf_release(&format_args.long_string);
+ }
+}
+
bool perf_pmus__have_event(const char *pname, const char *name)
{
struct perf_pmu *pmu = perf_pmus__find(pname);
diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
index 94d2a08d894b..eec599d8aebd 100644
--- a/tools/perf/util/pmus.h
+++ b/tools/perf/util/pmus.h
@@ -18,6 +18,7 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu);
const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str);

void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *print_state);
+void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, void *print_state);
bool perf_pmus__have_event(const char *pname, const char *name);
int perf_pmus__num_core_pmus(void);
bool perf_pmus__supports_extended_type(void);
diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
index e0d2b49bab66..3f38c27f0157 100644
--- a/tools/perf/util/print-events.c
+++ b/tools/perf/util/print-events.c
@@ -39,7 +39,7 @@ static const char * const event_type_descriptors[] = {
"Software event",
"Tracepoint event",
"Hardware cache event",
- "Raw hardware event descriptor",
+ "Raw event descriptor",
"Hardware breakpoint",
};

@@ -416,8 +416,6 @@ void print_symbol_events(const struct print_callbacks *print_cb, void *print_sta
*/
void print_events(const struct print_callbacks *print_cb, void *print_state)
{
- char *tmp;
-
print_symbol_events(print_cb, print_state, PERF_TYPE_HARDWARE,
event_symbols_hw, PERF_COUNT_HW_MAX);
print_symbol_events(print_cb, print_state, PERF_TYPE_SOFTWARE,
@@ -441,21 +439,7 @@ void print_events(const struct print_callbacks *print_cb, void *print_state)
/*long_desc=*/NULL,
/*encoding_desc=*/NULL);

- if (asprintf(&tmp, "%s/t1=v1[,t2=v2,t3 ...]/modifier",
- perf_pmus__scan_core(/*pmu=*/NULL)->name) > 0) {
- print_cb->print_event(print_state,
- /*topic=*/NULL,
- /*pmu_name=*/NULL,
- tmp,
- /*event_alias=*/NULL,
- /*scale_unit=*/NULL,
- /*deprecated=*/false,
- event_type_descriptors[PERF_TYPE_RAW],
- "(see 'man perf-list' on how to encode it)",
- /*long_desc=*/NULL,
- /*encoding_desc=*/NULL);
- free(tmp);
- }
+ perf_pmus__print_raw_pmu_events(print_cb, print_state);

print_cb->print_event(print_state,
/*topic=*/NULL,
--
2.44.0.278.ge034bb2e1d-goog


2024-03-20 01:02:01

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] Extra verbose/perf-list details

On Fri, Mar 8, 2024 at 8:34 AM Ian Rogers <[email protected]> wrote:
>
> On Fri, Mar 8, 2024 at 7:39 AM Liang, Kan <[email protected]> wrote:
>>
>>
>>
>> On 2024-03-07 7:19 p.m., Ian Rogers wrote:
>> > Add more encoding detail and raw event details in perf list. Add PMU
>> > name and reverse lookup from config to event name to
>> > perf_event_attr_fprintf. This makes the verbose output easier to read,
>> > and the perf list information more specific.
>> >
>> > v3. Fix to reverse lookup to ensure or aliases are loaded and if
>> > getting the config value fails for an event/alias just continue to
>> > the next one.
>> > v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
>> > becomes "Raw event descriptor" add assert to keep term numbers in
>> > sync, fix a commit message.
>> >
>> > Ian Rogers (6):
>> > perf list: Add tracepoint encoding to detailed output
>> > perf pmu: Drop "default_core" from alias names
>> > perf list: Allow wordwrap to wrap on commas
>> > perf list: Give more details about raw event encodings
>> > perf tools: Use pmus to describe type from attribute
>> > perf tools: Add/use PMU reverse lookup from config to name
>>
>> The patch series look good to me.
>> I verified it on a hybrid machine. The new format is the same as the
>> advertise.
>>
>> Tested-by: Kan Liang <[email protected]>

Ping.

Thanks,
Ian

>
> Thanks Kan! Just to add another feature of perf_event_attr_fprintf changes is that it shows up in perf trace output:
>
> ```
> $ perf trace -e perf_event_open perf stat -e data_read true
> 0.000 ( 0.011 ms): :36857/36857 perf_event_open(attr_uptr: { type: 24 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_fd: -1, flags: FD_CLOEXEC) = -1 EINVAL (Invalid argument)
> 0.014 ( 0.002 ms): :36857/36857 perf_event_open(attr_uptr: { type: 24 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_fd: -1) = -1 EINVAL (Invalid argument)
> 0.020 ( 0.008 ms): :36857/36857 perf_event_open(attr_uptr: { type: 24 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1 }, pid: -1, group_fd: -1) = 8
> 0.031 ( 0.006 ms): :36857/36857 perf_event_open(attr_uptr: { type: 25 (uncore_imc_free_running_1), size: 136, config: 0x20ff (unc_mc1_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_fd: -1) = -1 EINVAL (Invalid argument)
> 0.044 ( 0.004 ms): :36857/36857 perf_event_open(attr_uptr: { type: 25 (uncore_imc_free_running_1), size: 136, config: 0x20ff (unc_mc1_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1 }, pid: -1, group_fd: -1) = 9
>
> Performance counter stats for 'system wide':
>
> 5.07 MiB data_read
>
> 0.001178187 seconds time elapsed
> ```
>
> Thanks,
> Ian
>
>>
>> Thanks,
>> Kan
>> >
>> > tools/perf/builtin-list.c | 21 ++++-
>> > tools/perf/util/perf_event_attr_fprintf.c | 26 +++++--
>> > tools/perf/util/pmu.c | 82 +++++++++++++++++++-
>> > tools/perf/util/pmu.h | 4 +
>> > tools/perf/util/pmus.c | 94 +++++++++++++++++++++++
>> > tools/perf/util/pmus.h | 1 +
>> > tools/perf/util/print-events.c | 55 +++++++------
>> > 7 files changed, 242 insertions(+), 41 deletions(-)
>> >

2024-03-20 14:42:30

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] Extra verbose/perf-list details

On Tue, Mar 19, 2024 at 06:01:35PM -0700, Ian Rogers wrote:
> On Fri, Mar 8, 2024 at 8:34 AM Ian Rogers <[email protected]> wrote:
> > On Fri, Mar 8, 2024 at 7:39 AM Liang, Kan <[email protected]> wrote:
> >> On 2024-03-07 7:19 p.m., Ian Rogers wrote:
> >> > Add more encoding detail and raw event details in perf list. Add PMU
> >> > name and reverse lookup from config to event name to
> >> > perf_event_attr_fprintf. This makes the verbose output easier to read,
> >> > and the perf list information more specific.
> >> >
> >> > v3. Fix to reverse lookup to ensure or aliases are loaded and if
> >> > getting the config value fails for an event/alias just continue to
> >> > the next one.
> >> > v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
> >> > becomes "Raw event descriptor" add assert to keep term numbers in
> >> > sync, fix a commit message.
> >> >
> >> > Ian Rogers (6):
> >> > perf list: Add tracepoint encoding to detailed output
> >> > perf pmu: Drop "default_core" from alias names
> >> > perf list: Allow wordwrap to wrap on commas
> >> > perf list: Give more details about raw event encodings
> >> > perf tools: Use pmus to describe type from attribute
> >> > perf tools: Add/use PMU reverse lookup from config to name
> >>
> >> The patch series look good to me.
> >> I verified it on a hybrid machine. The new format is the same as the
> >> advertise.
> >>
> >> Tested-by: Kan Liang <[email protected]>
>
> Ping.

Thanks, applied to perf-tools-next,

- Arnaldo

2024-03-21 13:31:46

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 4/6] perf list: Give more details about raw event encodings

On Wed, Mar 20, 2024 at 07:59:06PM -0700, Ian Rogers wrote:
> On Thu, Mar 7, 2024 at 4:19 PM Ian Rogers <[email protected]> wrote:
> >
> > List all the PMUs, not just the first core one, and list real format
> > specifiers with value ranges.
> >
> > Before:
> > ```
> > $ perf list
> > ...
> > rNNN [Raw hardware event descriptor]
> > cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
> > [(see 'man perf-list' on how to encode it)]
> > mem:<addr>[/len][:access] [Hardware breakpoint]
> > ...
> > ```
> >
> > After:
> > ```
> > $ perf list
> > ...
> > rNNN [Raw event descriptor]
> > cpu/event=0..255,pc,edge,.../modifier [Raw event descriptor]
> > [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> > breakpoint//modifier [Raw event descriptor]
> > cstate_core/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> > cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> > i915/i915_eventid=0..0x1fffff/modifier [Raw event descriptor]
> > intel_bts//modifier [Raw event descriptor]
> > intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw event descriptor]
> > kprobe/retprobe/modifier [Raw event descriptor]
> > msr/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> > power/event=0..255/modifier [Raw event descriptor]
> > software//modifier [Raw event descriptor]
> > tracepoint//modifier [Raw event descriptor]
> > uncore_arb/event=0..255,edge,inv,.../modifier [Raw event descriptor]
> > uncore_cbox/event=0..255,edge,inv,.../modifier [Raw event descriptor]
> > uncore_clock/event=0..255/modifier [Raw event descriptor]
> > uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
> > uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
> > mem:<addr>[/len][:access] [Hardware breakpoint]
> > ...
> > ```
> >
> > With '--details' provide more details on the formats encoding:
> > ```
> > cpu/event=0..255,pc,edge,.../modifier [Raw event descriptor]
> > [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> > cpu/event=0..255,pc,edge,offcore_rsp=0..0xffffffffffffffff,ldlat=0..0xffff,inv,
> > umask=0..255,frontend=0..0xffffff,cmask=0..255,config=0..0xffffffffffffffff,
> > config1=0..0xffffffffffffffff,config2=0..0xffffffffffffffff,config3=0..0xffffffffffffffff,
> > name=string,period=number,freq=number,branch_type=(u|k|hv|any|...),time,
> > call-graph=(fp|dwarf|lbr),stack-size=number,max-stack=number,nr=number,inherit,no-inherit,
> > overwrite,no-overwrite,percore,aux-output,aux-sample-size=number/modifier
> > breakpoint//modifier [Raw event descriptor]
> > breakpoint//modifier
> > cstate_core/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> > cstate_core/event=0..0xffffffffffffffff/modifier
> > cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> > cstate_pkg/event=0..0xffffffffffffffff/modifier
> > i915/i915_eventid=0..0x1fffff/modifier [Raw event descriptor]
> > i915/i915_eventid=0..0x1fffff/modifier
> > intel_bts//modifier [Raw event descriptor]
> > intel_bts//modifier
> > intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw event descriptor]
> > intel_pt/ptw,event,cyc_thresh=0..15,pt,notnt,branch,tsc,pwr_evt,fup_on_ptw,cyc,noretcomp,
> > mtc,psb_period=0..15,mtc_period=0..15/modifier
> > kprobe/retprobe/modifier [Raw event descriptor]
> > kprobe/retprobe/modifier
> > msr/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> > msr/event=0..0xffffffffffffffff/modifier
> > power/event=0..255/modifier [Raw event descriptor]
> > power/event=0..255/modifier
> > software//modifier [Raw event descriptor]
> > software//modifier
> > tracepoint//modifier [Raw event descriptor]
> > tracepoint//modifier
> > uncore_arb/event=0..255,edge,inv,.../modifier [Raw event descriptor]
> > uncore_arb/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> > uncore_cbox/event=0..255,edge,inv,.../modifier [Raw event descriptor]
> > uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> > uncore_clock/event=0..255/modifier [Raw event descriptor]
> > uncore_clock/event=0..255/modifier
> > uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
> > uncore_imc_free_running/event=0..255,umask=0..255/modifier
> > uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
> > uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
> > ```
> >
> > Signed-off-by: Ian Rogers <[email protected]>
> > ---
> > tools/perf/util/pmu.c | 61 +++++++++++++++++++++-
> > tools/perf/util/pmu.h | 3 ++
> > tools/perf/util/pmus.c | 94 ++++++++++++++++++++++++++++++++++
> > tools/perf/util/pmus.h | 1 +
> > tools/perf/util/print-events.c | 20 +-------
> > 5 files changed, 160 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 24be587e3537..e24bc3b8f696 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -1603,6 +1603,61 @@ bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name)
> > return false;
> > }
> >
> > +int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb)
> > +{
> > + static const char *const terms[] = {
> > + "config=0..0xffffffffffffffff",
> > + "config1=0..0xffffffffffffffff",
> > + "config2=0..0xffffffffffffffff",
> > + "config3=0..0xffffffffffffffff",
> > + "name=string",
> > + "period=number",
> > + "freq=number",
> > + "branch_type=(u|k|hv|any|...)",
> > + "time",
> > + "call-graph=(fp|dwarf|lbr)",
> > + "stack-size=number",
> > + "max-stack=number",
> > + "nr=number",
> > + "inherit",
> > + "no-inherit",
> > + "overwrite",
> > + "no-overwrite",
> > + "percore",
> > + "aux-output",
> > + "aux-sample-size=number",
> > + };
> > + struct perf_pmu_format *format;
> > + int ret;
> > +
> > + /*
> > + * max-events and driver-config are missing above as are the internal
> > + * types user, metric-id, raw, legacy cache and hardware. Assert against
> > + * the enum parse_events__term_type so they are kept in sync.
> > + */
> > + _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);
>
> For C11 it is required that a 2nd message argument be passed:
> https://en.cppreference.com/w/c/language/_Static_assert
>
> This line needs to be something like:
> _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6,
> "Unexpected array size");
>
> Let me know if we I should send a fix, resend all the patches or if
> you can fix in tmp.perf-tools-next.

I added this, no need to resend.

- Arnaldo

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index e24bc3b8f696a9f5..81952c6cd3c6f5e8 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1635,7 +1635,8 @@ int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_call
* types user, metric-id, raw, legacy cache and hardware. Assert against
* the enum parse_events__term_type so they are kept in sync.
*/
- _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);
+ _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6,
+ "perf_pmu__for_each_format()'s terms must be kept in sync with enum parse_events__term_type");
list_for_each_entry(format, &pmu->format, list) {
perf_pmu_format__load(pmu, format);
ret = cb(state, format->name, (int)format->value, format->bits);

2024-03-21 02:59:29

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v3 4/6] perf list: Give more details about raw event encodings

On Thu, Mar 7, 2024 at 4:19 PM Ian Rogers <[email protected]> wrote:
>
> List all the PMUs, not just the first core one, and list real format
> specifiers with value ranges.
>
> Before:
> ```
> $ perf list
> ...
> rNNN [Raw hardware event descriptor]
> cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
> [(see 'man perf-list' on how to encode it)]
> mem:<addr>[/len][:access] [Hardware breakpoint]
> ...
> ```
>
> After:
> ```
> $ perf list
> ...
> rNNN [Raw event descriptor]
> cpu/event=0..255,pc,edge,.../modifier [Raw event descriptor]
> [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> breakpoint//modifier [Raw event descriptor]
> cstate_core/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> i915/i915_eventid=0..0x1fffff/modifier [Raw event descriptor]
> intel_bts//modifier [Raw event descriptor]
> intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw event descriptor]
> kprobe/retprobe/modifier [Raw event descriptor]
> msr/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> power/event=0..255/modifier [Raw event descriptor]
> software//modifier [Raw event descriptor]
> tracepoint//modifier [Raw event descriptor]
> uncore_arb/event=0..255,edge,inv,.../modifier [Raw event descriptor]
> uncore_cbox/event=0..255,edge,inv,.../modifier [Raw event descriptor]
> uncore_clock/event=0..255/modifier [Raw event descriptor]
> uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
> uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
> mem:<addr>[/len][:access] [Hardware breakpoint]
> ...
> ```
>
> With '--details' provide more details on the formats encoding:
> ```
> cpu/event=0..255,pc,edge,.../modifier [Raw event descriptor]
> [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> cpu/event=0..255,pc,edge,offcore_rsp=0..0xffffffffffffffff,ldlat=0..0xffff,inv,
> umask=0..255,frontend=0..0xffffff,cmask=0..255,config=0..0xffffffffffffffff,
> config1=0..0xffffffffffffffff,config2=0..0xffffffffffffffff,config3=0..0xffffffffffffffff,
> name=string,period=number,freq=number,branch_type=(u|k|hv|any|...),time,
> call-graph=(fp|dwarf|lbr),stack-size=number,max-stack=number,nr=number,inherit,no-inherit,
> overwrite,no-overwrite,percore,aux-output,aux-sample-size=number/modifier
> breakpoint//modifier [Raw event descriptor]
> breakpoint//modifier
> cstate_core/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> cstate_core/event=0..0xffffffffffffffff/modifier
> cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> cstate_pkg/event=0..0xffffffffffffffff/modifier
> i915/i915_eventid=0..0x1fffff/modifier [Raw event descriptor]
> i915/i915_eventid=0..0x1fffff/modifier
> intel_bts//modifier [Raw event descriptor]
> intel_bts//modifier
> intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw event descriptor]
> intel_pt/ptw,event,cyc_thresh=0..15,pt,notnt,branch,tsc,pwr_evt,fup_on_ptw,cyc,noretcomp,
> mtc,psb_period=0..15,mtc_period=0..15/modifier
> kprobe/retprobe/modifier [Raw event descriptor]
> kprobe/retprobe/modifier
> msr/event=0..0xffffffffffffffff/modifier [Raw event descriptor]
> msr/event=0..0xffffffffffffffff/modifier
> power/event=0..255/modifier [Raw event descriptor]
> power/event=0..255/modifier
> software//modifier [Raw event descriptor]
> software//modifier
> tracepoint//modifier [Raw event descriptor]
> tracepoint//modifier
> uncore_arb/event=0..255,edge,inv,.../modifier [Raw event descriptor]
> uncore_arb/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> uncore_cbox/event=0..255,edge,inv,.../modifier [Raw event descriptor]
> uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> uncore_clock/event=0..255/modifier [Raw event descriptor]
> uncore_clock/event=0..255/modifier
> uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
> uncore_imc_free_running/event=0..255,umask=0..255/modifier
> uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
> uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
> ```
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/util/pmu.c | 61 +++++++++++++++++++++-
> tools/perf/util/pmu.h | 3 ++
> tools/perf/util/pmus.c | 94 ++++++++++++++++++++++++++++++++++
> tools/perf/util/pmus.h | 1 +
> tools/perf/util/print-events.c | 20 +-------
> 5 files changed, 160 insertions(+), 19 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 24be587e3537..e24bc3b8f696 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1603,6 +1603,61 @@ bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name)
> return false;
> }
>
> +int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb)
> +{
> + static const char *const terms[] = {
> + "config=0..0xffffffffffffffff",
> + "config1=0..0xffffffffffffffff",
> + "config2=0..0xffffffffffffffff",
> + "config3=0..0xffffffffffffffff",
> + "name=string",
> + "period=number",
> + "freq=number",
> + "branch_type=(u|k|hv|any|...)",
> + "time",
> + "call-graph=(fp|dwarf|lbr)",
> + "stack-size=number",
> + "max-stack=number",
> + "nr=number",
> + "inherit",
> + "no-inherit",
> + "overwrite",
> + "no-overwrite",
> + "percore",
> + "aux-output",
> + "aux-sample-size=number",
> + };
> + struct perf_pmu_format *format;
> + int ret;
> +
> + /*
> + * max-events and driver-config are missing above as are the internal
> + * types user, metric-id, raw, legacy cache and hardware. Assert against
> + * the enum parse_events__term_type so they are kept in sync.
> + */
> + _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);

For C11 it is required that a 2nd message argument be passed:
https://en.cppreference.com/w/c/language/_Static_assert

This line needs to be something like:
_Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6,
"Unexpected array size");

Let me know if we I should send a fix, resend all the patches or if
you can fix in tmp.perf-tools-next.

Thanks,
Ian

> + list_for_each_entry(format, &pmu->format, list) {
> + perf_pmu_format__load(pmu, format);
> + ret = cb(state, format->name, (int)format->value, format->bits);
> + if (ret)
> + return ret;
> + }
> + if (!pmu->is_core)
> + return 0;
> +
> + for (size_t i = 0; i < ARRAY_SIZE(terms); i++) {
> + int config = PERF_PMU_FORMAT_VALUE_CONFIG;
> +
> + if (i < PERF_PMU_FORMAT_VALUE_CONFIG_END)
> + config = i;
> +
> + ret = cb(state, terms[i], config, /*bits=*/NULL);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +
> bool is_pmu_core(const char *name)
> {
> return !strcmp(name, "cpu") || !strcmp(name, "cpum_cf") || is_sysfs_pmu_core(name);
> @@ -1697,8 +1752,12 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> pmu_add_cpu_aliases(pmu);
> list_for_each_entry(event, &pmu->aliases, list) {
> size_t buf_used;
> + int pmu_name_len;
>
> info.pmu_name = event->pmu_name ?: pmu->name;
> + pmu_name_len = skip_duplicate_pmus
> + ? pmu_name_len_no_suffix(info.pmu_name, /*num=*/NULL)
> + : (int)strlen(info.pmu_name);
> info.alias = NULL;
> if (event->desc) {
> info.name = event->name;
> @@ -1723,7 +1782,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> info.encoding_desc = buf + buf_used;
> parse_events_terms__to_strbuf(&event->terms, &sb);
> buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
> - "%s/%s/", info.pmu_name, sb.buf) + 1;
> + "%.*s/%s/", pmu_name_len, info.pmu_name, sb.buf) + 1;
> info.topic = event->topic;
> info.str = sb.buf;
> info.deprecated = event->deprecated;
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index e35d985206db..9f5284b29ecf 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -196,6 +196,8 @@ struct pmu_event_info {
> };
>
> typedef int (*pmu_event_callback)(void *state, struct pmu_event_info *info);
> +typedef int (*pmu_format_callback)(void *state, const char *name, int config,
> + const unsigned long *bits);
>
> void pmu_add_sys_aliases(struct perf_pmu *pmu);
> int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
> @@ -215,6 +217,7 @@ int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, p
> int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd, bool eager_load);
> void perf_pmu_format__set_value(void *format, int config, unsigned long *bits);
> bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name);
> +int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb);
>
> bool is_pmu_core(const char *name);
> bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu);
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index 16505071d362..2fd369e45832 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -16,6 +16,7 @@
> #include "pmus.h"
> #include "pmu.h"
> #include "print-events.h"
> +#include "strbuf.h"
>
> /*
> * core_pmus: A PMU belongs to core_pmus if it's name is "cpu" or it's sysfs
> @@ -503,6 +504,99 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> zfree(&aliases);
> }
>
> +struct build_format_string_args {
> + struct strbuf short_string;
> + struct strbuf long_string;
> + int num_formats;
> +};
> +
> +static int build_format_string(void *state, const char *name, int config,
> + const unsigned long *bits)
> +{
> + struct build_format_string_args *args = state;
> + unsigned int num_bits;
> + int ret1, ret2 = 0;
> +
> + (void)config;
> + args->num_formats++;
> + if (args->num_formats > 1) {
> + strbuf_addch(&args->long_string, ',');
> + if (args->num_formats < 4)
> + strbuf_addch(&args->short_string, ',');
> + }
> + num_bits = bits ? bitmap_weight(bits, PERF_PMU_FORMAT_BITS) : 0;
> + if (num_bits <= 1) {
> + ret1 = strbuf_addf(&args->long_string, "%s", name);
> + if (args->num_formats < 4)
> + ret2 = strbuf_addf(&args->short_string, "%s", name);
> + } else if (num_bits > 8) {
> + ret1 = strbuf_addf(&args->long_string, "%s=0..0x%llx", name,
> + ULLONG_MAX >> (64 - num_bits));
> + if (args->num_formats < 4) {
> + ret2 = strbuf_addf(&args->short_string, "%s=0.0x%llx", name,
> + ULLONG_MAX >> (64 - num_bits));
> + }
> + } else {
> + ret1 = strbuf_addf(&args->long_string, "%s=0..%llu", name,
> + ULLONG_MAX >> (64 - num_bits));
> + if (args->num_formats < 4) {
> + ret2 = strbuf_addf(&args->short_string, "%s=0.%llu", name,
> + ULLONG_MAX >> (64 - num_bits));
> + }
> + }
> + return ret1 < 0 ? ret1 : (ret2 < 0 ? ret2 : 0);
> +}
> +
> +void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, void *print_state)
> +{
> + bool skip_duplicate_pmus = print_cb->skip_duplicate_pmus(print_state);
> + struct perf_pmu *(*scan_fn)(struct perf_pmu *);
> + struct perf_pmu *pmu = NULL;
> +
> + if (skip_duplicate_pmus)
> + scan_fn = perf_pmus__scan_skip_duplicates;
> + else
> + scan_fn = perf_pmus__scan;
> +
> + while ((pmu = scan_fn(pmu)) != NULL) {
> + struct build_format_string_args format_args = {
> + .short_string = STRBUF_INIT,
> + .long_string = STRBUF_INIT,
> + .num_formats = 0,
> + };
> + int len = pmu_name_len_no_suffix(pmu->name, /*num=*/NULL);
> + const char *desc = "(see 'man perf-list' or 'man perf-record' on how to encode it)";
> +
> + if (!pmu->is_core)
> + desc = NULL;
> +
> + strbuf_addf(&format_args.short_string, "%.*s/", len, pmu->name);
> + strbuf_addf(&format_args.long_string, "%.*s/", len, pmu->name);
> + perf_pmu__for_each_format(pmu, &format_args, build_format_string);
> +
> + if (format_args.num_formats > 3)
> + strbuf_addf(&format_args.short_string, ",.../modifier");
> + else
> + strbuf_addf(&format_args.short_string, "/modifier");
> +
> + strbuf_addf(&format_args.long_string, "/modifier");
> + print_cb->print_event(print_state,
> + /*topic=*/NULL,
> + /*pmu_name=*/NULL,
> + format_args.short_string.buf,
> + /*event_alias=*/NULL,
> + /*scale_unit=*/NULL,
> + /*deprecated=*/false,
> + "Raw event descriptor",
> + desc,
> + /*long_desc=*/NULL,
> + format_args.long_string.buf);
> +
> + strbuf_release(&format_args.short_string);
> + strbuf_release(&format_args.long_string);
> + }
> +}
> +
> bool perf_pmus__have_event(const char *pname, const char *name)
> {
> struct perf_pmu *pmu = perf_pmus__find(pname);
> diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
> index 94d2a08d894b..eec599d8aebd 100644
> --- a/tools/perf/util/pmus.h
> +++ b/tools/perf/util/pmus.h
> @@ -18,6 +18,7 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu);
> const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str);
>
> void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *print_state);
> +void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, void *print_state);
> bool perf_pmus__have_event(const char *pname, const char *name);
> int perf_pmus__num_core_pmus(void);
> bool perf_pmus__supports_extended_type(void);
> diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> index e0d2b49bab66..3f38c27f0157 100644
> --- a/tools/perf/util/print-events.c
> +++ b/tools/perf/util/print-events.c
> @@ -39,7 +39,7 @@ static const char * const event_type_descriptors[] = {
> "Software event",
> "Tracepoint event",
> "Hardware cache event",
> - "Raw hardware event descriptor",
> + "Raw event descriptor",
> "Hardware breakpoint",
> };
>
> @@ -416,8 +416,6 @@ void print_symbol_events(const struct print_callbacks *print_cb, void *print_sta
> */
> void print_events(const struct print_callbacks *print_cb, void *print_state)
> {
> - char *tmp;
> -
> print_symbol_events(print_cb, print_state, PERF_TYPE_HARDWARE,
> event_symbols_hw, PERF_COUNT_HW_MAX);
> print_symbol_events(print_cb, print_state, PERF_TYPE_SOFTWARE,
> @@ -441,21 +439,7 @@ void print_events(const struct print_callbacks *print_cb, void *print_state)
> /*long_desc=*/NULL,
> /*encoding_desc=*/NULL);
>
> - if (asprintf(&tmp, "%s/t1=v1[,t2=v2,t3 ...]/modifier",
> - perf_pmus__scan_core(/*pmu=*/NULL)->name) > 0) {
> - print_cb->print_event(print_state,
> - /*topic=*/NULL,
> - /*pmu_name=*/NULL,
> - tmp,
> - /*event_alias=*/NULL,
> - /*scale_unit=*/NULL,
> - /*deprecated=*/false,
> - event_type_descriptors[PERF_TYPE_RAW],
> - "(see 'man perf-list' on how to encode it)",
> - /*long_desc=*/NULL,
> - /*encoding_desc=*/NULL);
> - free(tmp);
> - }
> + perf_pmus__print_raw_pmu_events(print_cb, print_state);
>
> print_cb->print_event(print_state,
> /*topic=*/NULL,
> --
> 2.44.0.278.ge034bb2e1d-goog
>