Re: [PATCH v2 3/3] perf pmus: Don't print duplicate PMU suffix in list by default

[John Garry]

On 10/08/2023 22:49, Ian Rogers wrote:
> Duplicate PMUs are no longer printed by default but the suffix of the
> first is printed. When duplicate PMUs are being skipped avoid printing
> the suffix.
>
> Before:
> ```
> $ perf list
> ...
>    uncore_imc_free_running_0/data_read/               [Kernel PMU event]
>    uncore_imc_free_running_0/data_total/              [Kernel PMU event]
>    uncore_imc_free_running_0/data_write/              [Kernel PMU event]
> ```
>
> After:
> ```
> $ perf list
> ...
>    uncore_imc_free_running/data_read/                 [Kernel PMU event]
>    uncore_imc_free_running/data_total/                [Kernel PMU event]
>    uncore_imc_free_running/data_write/                [Kernel PMU event]
> ...
> $ perf list -v
>    uncore_imc_free_running_0/data_read/               [Kernel PMU event]
>    uncore_imc_free_running_0/data_total/              [Kernel PMU event]
>    uncore_imc_free_running_0/data_write/              [Kernel PMU event]
>    uncore_imc_free_running_1/data_read/               [Kernel PMU event]
>    uncore_imc_free_running_1/data_total/              [Kernel PMU event]
>    uncore_imc_free_running_1/data_write/              [Kernel PMU event]
> ...
> ```
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
>   tools/perf/util/pmus.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index 5073843aca19..b0ecb2e5bdcc 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -440,10 +440,13 @@ static int sub_non_neg(int a, int b)
>   }
>   
>   static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
> -			  const struct perf_pmu_alias *alias)
> +			  const struct perf_pmu_alias *alias, bool skip_duplicate_pmus)
>   {
>   	struct parse_events_term *term;
> -	int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
> +	int pmu_name_len = skip_duplicate_pmus
> +		? pmu_name_len_no_suffix(pmu->name, /*num=*/NULL)
> +		: (int)strlen(pmu->name);
> +	int used = snprintf(buf, len, "%.*s/%s", pmu_name_len, pmu->name, alias->name);
>   
>   	list_for_each_entry(term, &alias->terms, list) {
>   		if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
> @@ -473,9 +476,10 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>   	int printed = 0;
>   	int len, j;
>   	struct sevent *aliases;
> +	bool skip_duplicate_pmus = print_cb->skip_duplicate_pmus(print_state);

nit: that code could have been in the previous patch

>   	struct perf_pmu *(*scan_fn)(struct perf_pmu *);
>   
> -	if (print_cb->skip_duplicate_pmus(print_state))
> +	if (skip_duplicate_pmus)
>   		scan_fn = perf_pmus__scan_skip_duplicates;
>   	else
>   		scan_fn = perf_pmus__scan;
> @@ -518,6 +522,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>   			*desc = NULL, *long_desc = NULL,
>   			*encoding_desc = NULL, *topic = NULL,
>   			*pmu_name = NULL;
> +		int pmu_name_len;
>   		bool deprecated = false;
>   		size_t buf_used;
>   
> @@ -528,7 +533,8 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>   		if (!aliases[j].event) {
>   			/* A selectable event. */
>   			pmu_name = aliases[j].pmu->name;
> -			buf_used = snprintf(buf, sizeof(buf), "%s//", pmu_name) + 1;
> +			pmu_name_len = pmu_name_len_no_suffix(pmu_name, /*num=*/NULL);
> +			buf_used = snprintf(buf, sizeof(buf), "%.*s//", pmu_name_len, pmu_name) + 1;
>   			name = buf;
>   		} else {
>   			if (aliases[j].event->desc) {
> @@ -536,7 +542,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>   				buf_used = 0;
>   			} else {
>   				name = format_alias(buf, sizeof(buf), aliases[j].pmu,
> -						    aliases[j].event);
> +						    aliases[j].event, skip_duplicate_pmus);
>   				if (aliases[j].is_cpu) {
>   					alias = name;
>   					name = aliases[j].event->name;
> @@ -554,8 +560,10 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>   			long_desc = aliases[j].event->long_desc;
>   			topic = aliases[j].event->topic;
>   			encoding_desc = buf + buf_used;
> +			pmu_name_len = pmu_name_len_no_suffix(pmu_name, /*num=*/NULL);
>   			buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
> -					"%s/%s/", pmu_name, aliases[j].event->str) + 1;
> +					"%.*s/%s/", pmu_name_len, pmu_name,
> +					aliases[j].event->str) + 1;
>   			deprecated = aliases[j].event->deprecated;
>   		}
>   		print_cb->print_event(print_state,