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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 11 Aug 2023 16:53:56 +0100
From:   John Garry <john.g.garry@...cle.com>
To:     Ian Rogers <irogers@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Ravi Bangoria <ravi.bangoria@....com>,
        Kajol Jain <kjain@...ux.ibm.com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] perf pmus: Don't print duplicate PMU suffix in
 list by default

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@...gle.com>
> ---
>   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,