* [PATCH 2/3] perf list: Get rid of json_print_state
2025-11-18 6:36 [PATCH 1/3] perf list: Print matching PMU events for --unit Namhyung Kim
@ 2025-11-18 6:36 ` Namhyung Kim
2025-11-19 16:40 ` Ian Rogers
2025-11-18 6:36 ` [PATCH 3/3] perf list: Support filtering in JSON output Namhyung Kim
2025-11-19 16:33 ` [PATCH 1/3] perf list: Print matching PMU events for --unit Ian Rogers
2 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2025-11-18 6:36 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
The JSON print state has only one different field (need_sep). Let's
move it to the default print state and use it. It would waste a little
bit of more spaces but who cares? :)
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-list.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 28bf1fc7f5eeff8f..1ab969ffe371c5cb 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -60,6 +60,8 @@ struct print_state {
bool metricgroups;
/** @exclude_abi: Exclude PMUs with types less than PERF_TYPE_MAX except PERF_TYPE_RAW. */
bool exclude_abi;
+ /** @need_sep: Should a separator be printed prior to the next item? (for JSON) */
+ bool need_sep;
/** @last_topic: The last printed event topic. */
char *last_topic;
/** @last_metricgroups: The last printed metric group. */
@@ -368,7 +370,7 @@ static void json_print_event(void *ps, const char *topic,
const char *desc, const char *long_desc,
const char *encoding_desc)
{
- struct json_print_state *print_state = ps;
+ struct print_state *print_state = ps;
bool need_sep = false;
FILE *fp = print_state->fp;
struct strbuf buf;
@@ -444,7 +446,7 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
const char *threshold, const char *unit,
const char *pmu_name)
{
- struct json_print_state *print_state = ps;
+ struct print_state *print_state = ps;
bool need_sep = false;
FILE *fp = print_state->fp;
struct strbuf buf;
@@ -521,9 +523,6 @@ int cmd_list(int argc, const char **argv)
.fp = stdout,
.desc = true,
};
- struct print_state json_ps = {
- .fp = stdout,
- };
void *ps = &default_ps;
struct print_callbacks print_cb = {
.print_start = default_print_start,
@@ -574,7 +573,6 @@ int cmd_list(int argc, const char **argv)
if (output_path) {
default_ps.fp = fopen(output_path, "w");
- json_ps.fp = default_ps.fp;
}
setup_pager();
@@ -590,7 +588,7 @@ int cmd_list(int argc, const char **argv)
.print_metric = json_print_metric,
.skip_duplicate_pmus = json_skip_duplicate_pmus,
};
- ps = &json_ps;
+ ps = &default_ps;
} else {
default_ps.last_topic = strdup("");
assert(default_ps.last_topic);
--
2.52.0.rc1.455.g30608eb744-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] perf list: Get rid of json_print_state
2025-11-18 6:36 ` [PATCH 2/3] perf list: Get rid of json_print_state Namhyung Kim
@ 2025-11-19 16:40 ` Ian Rogers
0 siblings, 0 replies; 6+ messages in thread
From: Ian Rogers @ 2025-11-19 16:40 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Mon, Nov 17, 2025 at 10:36 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> The JSON print state has only one different field (need_sep). Let's
> move it to the default print state and use it. It would waste a little
> bit of more spaces but who cares? :)
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-list.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 28bf1fc7f5eeff8f..1ab969ffe371c5cb 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -60,6 +60,8 @@ struct print_state {
> bool metricgroups;
> /** @exclude_abi: Exclude PMUs with types less than PERF_TYPE_MAX except PERF_TYPE_RAW. */
> bool exclude_abi;
> + /** @need_sep: Should a separator be printed prior to the next item? (for JSON) */
> + bool need_sep;
I'm concerned we might be reinventing something like the stat-display code :-)
Perhaps this would be cleaner as:
```
struct json_print_state {
/** State for regular filtering, etc. */
struct print_state ps;
/** Should a json separator ',' be printed prior to the next item? */
bool need_sep;
}
```
So we have the regular print state stuff in the json print_state but
don't expose json-ness things to other print_state users - and
hopefully avoid convoluted shared state, function pointers, etc.
Thanks,
Ian
> /** @last_topic: The last printed event topic. */
> char *last_topic;
> /** @last_metricgroups: The last printed metric group. */
> @@ -368,7 +370,7 @@ static void json_print_event(void *ps, const char *topic,
> const char *desc, const char *long_desc,
> const char *encoding_desc)
> {
> - struct json_print_state *print_state = ps;
> + struct print_state *print_state = ps;
> bool need_sep = false;
> FILE *fp = print_state->fp;
> struct strbuf buf;
> @@ -444,7 +446,7 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
> const char *threshold, const char *unit,
> const char *pmu_name)
> {
> - struct json_print_state *print_state = ps;
> + struct print_state *print_state = ps;
> bool need_sep = false;
> FILE *fp = print_state->fp;
> struct strbuf buf;
> @@ -521,9 +523,6 @@ int cmd_list(int argc, const char **argv)
> .fp = stdout,
> .desc = true,
> };
> - struct print_state json_ps = {
> - .fp = stdout,
> - };
> void *ps = &default_ps;
> struct print_callbacks print_cb = {
> .print_start = default_print_start,
> @@ -574,7 +573,6 @@ int cmd_list(int argc, const char **argv)
>
> if (output_path) {
> default_ps.fp = fopen(output_path, "w");
> - json_ps.fp = default_ps.fp;
> }
>
> setup_pager();
> @@ -590,7 +588,7 @@ int cmd_list(int argc, const char **argv)
> .print_metric = json_print_metric,
> .skip_duplicate_pmus = json_skip_duplicate_pmus,
> };
> - ps = &json_ps;
> + ps = &default_ps;
> } else {
> default_ps.last_topic = strdup("");
> assert(default_ps.last_topic);
> --
> 2.52.0.rc1.455.g30608eb744-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] perf list: Support filtering in JSON output
2025-11-18 6:36 [PATCH 1/3] perf list: Print matching PMU events for --unit Namhyung Kim
2025-11-18 6:36 ` [PATCH 2/3] perf list: Get rid of json_print_state Namhyung Kim
@ 2025-11-18 6:36 ` Namhyung Kim
2025-11-19 16:42 ` Ian Rogers
2025-11-19 16:33 ` [PATCH 1/3] perf list: Print matching PMU events for --unit Ian Rogers
2 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2025-11-18 6:36 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
Like regular output mode, it should honor command line arguments to
limit to a certain type of PMUs or events.
$ perf list -j hw
[
{
"Unit": "cpu",
"Topic": "legacy hardware",
"EventName": "branch-instructions",
"EventType": "Kernel PMU event",
"BriefDescription": "Retired branch instructions [This event is an alias of branches]",
"Encoding": "cpu/event=0xc4\n/"
},
{
"Unit": "cpu",
"Topic": "legacy hardware",
"EventName": "branch-misses",
"EventType": "Kernel PMU event",
"BriefDescription": "Mispredicted branch instructions",
"Encoding": "cpu/event=0xc5\n/"
},
...
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-list.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 1ab969ffe371c5cb..48c799007414bf6e 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -375,6 +375,21 @@ static void json_print_event(void *ps, const char *topic,
FILE *fp = print_state->fp;
struct strbuf buf;
+ if (deprecated && !print_state->deprecated)
+ return;
+
+ if (print_state->pmu_glob && (!pmu_name || !strglobmatch(pmu_name, print_state->pmu_glob)))
+ return;
+
+ if (print_state->exclude_abi && pmu_type < PERF_TYPE_MAX && pmu_type != PERF_TYPE_RAW)
+ return;
+
+ if (print_state->event_glob &&
+ (!event_name || !strglobmatch(event_name, print_state->event_glob)) &&
+ (!event_alias || !strglobmatch(event_alias, print_state->event_glob)) &&
+ (!topic || !strglobmatch_nocase(topic, print_state->event_glob)))
+ return;
+
strbuf_init(&buf, 0);
fprintf(fp, "%s{\n", print_state->need_sep ? ",\n" : "");
print_state->need_sep = true;
@@ -451,6 +466,11 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
FILE *fp = print_state->fp;
struct strbuf buf;
+ if (print_state->event_glob &&
+ (!print_state->metrics || !name || !strglobmatch(name, print_state->event_glob)) &&
+ (!print_state->metricgroups || !group || !strglobmatch(group, print_state->event_glob)))
+ return;
+
strbuf_init(&buf, 0);
fprintf(fp, "%s{\n", print_state->need_sep ? ",\n" : "");
print_state->need_sep = true;
--
2.52.0.rc1.455.g30608eb744-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] perf list: Support filtering in JSON output
2025-11-18 6:36 ` [PATCH 3/3] perf list: Support filtering in JSON output Namhyung Kim
@ 2025-11-19 16:42 ` Ian Rogers
0 siblings, 0 replies; 6+ messages in thread
From: Ian Rogers @ 2025-11-19 16:42 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Mon, Nov 17, 2025 at 10:36 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Like regular output mode, it should honor command line arguments to
> limit to a certain type of PMUs or events.
>
> $ perf list -j hw
> [
> {
> "Unit": "cpu",
> "Topic": "legacy hardware",
> "EventName": "branch-instructions",
> "EventType": "Kernel PMU event",
> "BriefDescription": "Retired branch instructions [This event is an alias of branches]",
> "Encoding": "cpu/event=0xc4\n/"
The new-line '\n' here looks like another bug, the bug is coming from
reading the event from sysfs but not stripping any '\n'.
This change lgtm but we need to figure out the struct json_print_state
in patch 2.
Thanks,
Ian
> },
> {
> "Unit": "cpu",
> "Topic": "legacy hardware",
> "EventName": "branch-misses",
> "EventType": "Kernel PMU event",
> "BriefDescription": "Mispredicted branch instructions",
> "Encoding": "cpu/event=0xc5\n/"
> },
> ...
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-list.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 1ab969ffe371c5cb..48c799007414bf6e 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -375,6 +375,21 @@ static void json_print_event(void *ps, const char *topic,
> FILE *fp = print_state->fp;
> struct strbuf buf;
>
> + if (deprecated && !print_state->deprecated)
> + return;
> +
> + if (print_state->pmu_glob && (!pmu_name || !strglobmatch(pmu_name, print_state->pmu_glob)))
> + return;
> +
> + if (print_state->exclude_abi && pmu_type < PERF_TYPE_MAX && pmu_type != PERF_TYPE_RAW)
> + return;
> +
> + if (print_state->event_glob &&
> + (!event_name || !strglobmatch(event_name, print_state->event_glob)) &&
> + (!event_alias || !strglobmatch(event_alias, print_state->event_glob)) &&
> + (!topic || !strglobmatch_nocase(topic, print_state->event_glob)))
> + return;
> +
> strbuf_init(&buf, 0);
> fprintf(fp, "%s{\n", print_state->need_sep ? ",\n" : "");
> print_state->need_sep = true;
> @@ -451,6 +466,11 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
> FILE *fp = print_state->fp;
> struct strbuf buf;
>
> + if (print_state->event_glob &&
> + (!print_state->metrics || !name || !strglobmatch(name, print_state->event_glob)) &&
> + (!print_state->metricgroups || !group || !strglobmatch(group, print_state->event_glob)))
> + return;
> +
> strbuf_init(&buf, 0);
> fprintf(fp, "%s{\n", print_state->need_sep ? ",\n" : "");
> print_state->need_sep = true;
> --
> 2.52.0.rc1.455.g30608eb744-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] perf list: Print matching PMU events for --unit
2025-11-18 6:36 [PATCH 1/3] perf list: Print matching PMU events for --unit Namhyung Kim
2025-11-18 6:36 ` [PATCH 2/3] perf list: Get rid of json_print_state Namhyung Kim
2025-11-18 6:36 ` [PATCH 3/3] perf list: Support filtering in JSON output Namhyung Kim
@ 2025-11-19 16:33 ` Ian Rogers
2 siblings, 0 replies; 6+ messages in thread
From: Ian Rogers @ 2025-11-19 16:33 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Mon, Nov 17, 2025 at 10:36 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> When --unit option is used, pmu_glob is set to the argument. It should
> match with event PMU and display the matching ones only. But it also
> shows raw events and metrics after that.
>
> $ perf list --unit tool
> List of pre-defined events (to be used in -e or -M):
>
> tool:
> core_wide
> [1 if not SMT,if SMT are events being gathered on all SMT threads 1 otherwise 0. Unit: tool]
> duration_time
> [Wall clock interval time in nanoseconds. Unit: tool]
> has_pmem
> [1 if persistent memory installed otherwise 0. Unit: tool]
> num_cores
> [Number of cores. A core consists of 1 or more thread,with each thread being associated with a logical Linux CPU. Unit: tool]
> num_cpus
> [Number of logical Linux CPUs. There may be multiple such CPUs on a core. Unit: tool]
> ...
> 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]
> drm_i915//modifier [Raw event descriptor]
> hwmon_acpitz//modifier [Raw event descriptor]
> hwmon_ac//modifier [Raw event descriptor]
> hwmon_bat0//modifier [Raw event descriptor]
> hwmon_coretemp//modifier [Raw event descriptor]
> ...
>
> Metric Groups:
>
> Backend: [Grouping from Top-down Microarchitecture Analysis Metrics spreadsheet]
> tma_core_bound
> [This metric represents fraction of slots where Core non-memory issues were of a bottleneck]
> tma_info_core_ilp
> [Instruction-Level-Parallelism (average number of uops executed when there is execution) per thread (logical-processor)]
> tma_info_memory_l2mpki
> [L2 cache true misses per kilo instruction for retired demand loads]
> ...
>
> This change makes it print the tool PMU events only.
Perhaps this can be used to simplify tests like:
https://lore.kernel.org/lkml/20251109005959.2540616-2-irogers@google.com/
Reviewed-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-list.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 16400366f8276a7a..28bf1fc7f5eeff8f 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -130,7 +130,7 @@ static void default_print_event(void *ps, const char *topic,
> if (deprecated && !print_state->deprecated)
> return;
>
> - if (print_state->pmu_glob && pmu_name && !strglobmatch(pmu_name, print_state->pmu_glob))
> + if (print_state->pmu_glob && (!pmu_name || !strglobmatch(pmu_name, print_state->pmu_glob)))
> return;
>
> if (print_state->exclude_abi && pmu_type < PERF_TYPE_MAX && pmu_type != PERF_TYPE_RAW)
> @@ -612,8 +612,10 @@ int cmd_list(int argc, const char **argv)
> print_cb.print_start(ps);
>
> if (argc == 0) {
> - default_ps.metrics = true;
> - default_ps.metricgroups = true;
> + if (!unit_name) {
> + default_ps.metrics = true;
> + default_ps.metricgroups = true;
> + }
> print_events(&print_cb, ps);
> goto out;
> }
> --
> 2.52.0.rc1.455.g30608eb744-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread