2022-04-21 14:45:22

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] perf list: print all available tool events

Em Wed, Apr 20, 2022 at 12:23:54PM +0200, Florian Fischer escreveu:
> Introduce names for the new tool events 'user_time' and 'system_time'.

Before:

duration_time [Tool event]

After:

$ perf list | grep Tool
duration_time OR [Tool event]
user_time OR [Tool event]
system_time OR [Tool event]
$

So now we have that extraneous "OR", can you please take a look?
Probably needs to check if there is an alias?

- Arnaldo

> Signed-off-by: Florian Fischer <[email protected]>
> ---
> tools/perf/util/evsel.c | 19 ++++++++++------
> tools/perf/util/evsel.h | 1 +
> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++-----
> 3 files changed, 47 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 2a1729e7aee4..d38722560e80 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -597,6 +597,17 @@ static int evsel__sw_name(struct evsel *evsel, char *bf, size_t size)
> return r + evsel__add_modifiers(evsel, bf + r, size - r);
> }
>
> +const char *evsel__tool_names[PERF_TOOL_MAX] = {
> + "duration_time",
> + "user_time",
> + "system_time",
> +};
> +
> +static int evsel__tool_name(enum perf_tool_event ev, char *bf, size_t size)
> +{
> + return scnprintf(bf, size, "%s", evsel__tool_names[ev]);
> +}
> +
> static int __evsel__bp_name(char *bf, size_t size, u64 addr, u64 type)
> {
> int r;
> @@ -723,12 +734,6 @@ static int evsel__raw_name(struct evsel *evsel, char *bf, size_t size)
> return ret + evsel__add_modifiers(evsel, bf + ret, size - ret);
> }
>
> -static int evsel__tool_name(char *bf, size_t size)
> -{
> - int ret = scnprintf(bf, size, "duration_time");
> - return ret;
> -}
> -
> const char *evsel__name(struct evsel *evsel)
> {
> char bf[128];
> @@ -754,7 +759,7 @@ const char *evsel__name(struct evsel *evsel)
>
> case PERF_TYPE_SOFTWARE:
> if (evsel->tool_event)
> - evsel__tool_name(bf, sizeof(bf));
> + evsel__tool_name(evsel->tool_event, bf, sizeof(bf));
> else
> evsel__sw_name(evsel, bf, sizeof(bf));
> break;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 7e2209b47b39..45d674812239 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -262,6 +262,7 @@ extern const char *evsel__hw_cache_op[PERF_COUNT_HW_CACHE_OP_MAX][EVSEL__MAX_ALI
> extern const char *evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX][EVSEL__MAX_ALIASES];
> extern const char *evsel__hw_names[PERF_COUNT_HW_MAX];
> extern const char *evsel__sw_names[PERF_COUNT_SW_MAX];
> +extern const char *evsel__tool_names[PERF_TOOL_MAX];
> extern char *evsel__bpf_counter_events;
> bool evsel__match_bpf_counter_events(const char *name);
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 064ec368a90b..c0e88d3b3e3c 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -154,6 +154,21 @@ struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
> },
> };
>
> +struct event_symbol event_symbols_tool[PERF_TOOL_MAX] = {
> + [PERF_TOOL_DURATION_TIME] = {
> + .symbol = "duration_time",
> + .alias = "",
> + },
> + [PERF_TOOL_USER_TIME] = {
> + .symbol = "user_time",
> + .alias = "",
> + },
> + [PERF_TOOL_SYSTEM_TIME] = {
> + .symbol = "system_time",
> + .alias = "",
> + },
> +};
> +
> #define __PERF_EVENT_FIELD(config, name) \
> ((config & PERF_EVENT_##name##_MASK) >> PERF_EVENT_##name##_SHIFT)
>
> @@ -3058,21 +3073,34 @@ int print_hwcache_events(const char *event_glob, bool name_only)
> return evt_num;
> }
>
> -static void print_tool_event(const char *name, const char *event_glob,
> +static void print_tool_event(const struct event_symbol *syms, const char *event_glob,
> bool name_only)
> {
> - if (event_glob && !strglobmatch(name, event_glob))
> + if (syms->symbol == NULL)
> + return;
> +
> + if (event_glob && !(strglobmatch(syms->symbol, event_glob) ||
> + (syms->alias && strglobmatch(syms->alias, event_glob))))
> return;
> +
> if (name_only)
> - printf("%s ", name);
> - else
> + printf("%s ", syms->symbol);
> + else {
> + char name[MAX_NAME_LEN];
> + if (syms->alias)
> + snprintf(name, MAX_NAME_LEN, "%s OR %s", syms->symbol, syms->alias);
> + else
> + strlcpy(name, syms->symbol, MAX_NAME_LEN);
> printf(" %-50s [%s]\n", name, "Tool event");
> -
> + }
> }
>
> void print_tool_events(const char *event_glob, bool name_only)
> {
> - print_tool_event("duration_time", event_glob, name_only);
> + // Start at 1 because the first enum entry symbols no tool event
> + for (int i = 1; i < PERF_TOOL_MAX; ++i) {
> + print_tool_event(event_symbols_tool + i, event_glob, name_only);
> + }
> if (pager_in_use())
> printf("\n");
> }
> --
> 2.36.0

--

- Arnaldo


2022-04-22 17:36:07

by Florian Fischer

[permalink] [raw]
Subject: [PATCH v5] perf list: print all available tool events

>> Introduce names for the new tool events 'user_time' and 'system_time'.
>
> Before:
>
> duration_time [Tool event]
>
> After:
>
> $ perf list | grep Tool
> duration_time OR [Tool event]
> user_time OR [Tool event]
> system_time OR [Tool event]
> $
>
> So now we have that extraneous "OR", can you please take a look?
> Probably needs to check if there is an alias?
>
> - Arnaldo

Sorry about that. Exactly as you guessed the code was missing a check if the tool
event actually has an alias.

Fixed Output:

$ perf list | grep Tool
duration_time [Tool event]
user_time [Tool event]
system_time [Tool event]

[v5]: Add missing alias check.

Florian Fischer