linux-kernel - Re: [PATCH v1 14/40] perf print-events: Avoid unnecessary strlist

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]
Message-ID: <528dd99d-6a2f-8b10-e3e3-21f682ab9101@linux.intel.com>
Date:   Thu, 27 Apr 2023 16:01:18 -0400
From:   "Liang, Kan" <kan.liang@...ux.intel.com>
To:     Ian Rogers <irogers@...gle.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Ahmad Yasin <ahmad.yasin@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Stephane Eranian <eranian@...gle.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Perry Taylor <perry.taylor@...el.com>,
        Samantha Alt <samantha.alt@...el.com>,
        Caleb Biggers <caleb.biggers@...el.com>,
        Weilin Wang <weilin.wang@...el.com>,
        Edward Baker <edward.baker@...el.com>,
        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>,
        Florian Fischer <florian.fischer@...q.space>,
        Rob Herring <robh@...nel.org>,
        Zhengjun Xing <zhengjun.xing@...ux.intel.com>,
        John Garry <john.g.garry@...cle.com>,
        Kajol Jain <kjain@...ux.ibm.com>,
        Sumanth Korikkar <sumanthk@...ux.ibm.com>,
        Thomas Richter <tmricht@...ux.ibm.com>,
        Tiezhu Yang <yangtiezhu@...ngson.cn>,
        Ravi Bangoria <ravi.bangoria@....com>,
        Leo Yan <leo.yan@...aro.org>,
        Yang Jihong <yangjihong1@...wei.com>,
        James Clark <james.clark@....com>,
        Suzuki Poulouse <suzuki.poulose@....com>,
        Kang Minchul <tegongkang@...il.com>,
        Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 14/40] perf print-events: Avoid unnecessary strlist



On 2023-04-26 3:00 a.m., Ian Rogers wrote:
> The strlist in print_hwcache_events holds the event names as they are
> generated, and then it is iterated and printed. This is unnecessary
> and each event can just be printed as it is processed.
> Rename the variable i to res, to be more intention revealing and
> consistent with other code.
> 

Looks good to me.

Reviewed-by: Kan Liang <kan.liang@...ux.intel.com>

Thanks,
Kan

> Signed-off-by: Ian Rogers <irogers@...gle.com>> ---
>  tools/perf/util/print-events.c | 60 ++++++++++++++++++----------------
>  1 file changed, 31 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> index 386b1ab0b60e..93bbb868d400 100644
> --- a/tools/perf/util/print-events.c
> +++ b/tools/perf/util/print-events.c
> @@ -226,58 +226,60 @@ void print_sdt_events(const struct print_callbacks *print_cb, void *print_state)
>  
>  int print_hwcache_events(const struct print_callbacks *print_cb, void *print_state)
>  {
> -	struct strlist *evt_name_list = strlist__new(NULL, NULL);
> -	struct str_node *nd;
> +	const char *event_type_descriptor = event_type_descriptors[PERF_TYPE_HW_CACHE];
>  
> -	if (!evt_name_list) {
> -		pr_debug("Failed to allocate new strlist for hwcache events\n");
> -		return -ENOMEM;
> -	}
>  	for (int type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
>  		for (int op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) {
>  			/* skip invalid cache type */
>  			if (!evsel__is_cache_op_valid(type, op))
>  				continue;
>  
> -			for (int i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
> +			for (int res = 0; res < PERF_COUNT_HW_CACHE_RESULT_MAX; res++) {
>  				struct perf_pmu *pmu = NULL;
>  				char name[64];
>  
> -				__evsel__hw_cache_type_op_res_name(type, op, i, name, sizeof(name));
> +				__evsel__hw_cache_type_op_res_name(type, op, res,
> +								   name, sizeof(name));
>  				if (!perf_pmu__has_hybrid()) {
>  					if (is_event_supported(PERF_TYPE_HW_CACHE,
> -							       type | (op << 8) | (i << 16)))
> -						strlist__add(evt_name_list, name);
> +								type | (op << 8) | (res << 16))) {
> +						print_cb->print_event(print_state,
> +								"cache",
> +								/*pmu_name=*/NULL,
> +								name,
> +								/*event_alias=*/NULL,
> +								/*scale_unit=*/NULL,
> +								/*deprecated=*/false,
> +								event_type_descriptor,
> +								/*desc=*/NULL,
> +								/*long_desc=*/NULL,
> +								/*encoding_desc=*/NULL);
> +					}
>  					continue;
>  				}
>  				perf_pmu__for_each_hybrid_pmu(pmu) {
>  					if (is_event_supported(PERF_TYPE_HW_CACHE,
> -					    type | (op << 8) | (i << 16) |
> +					    type | (op << 8) | (res << 16) |
>  					    ((__u64)pmu->type << PERF_PMU_TYPE_SHIFT))) {
>  						char new_name[128];
> -							snprintf(new_name, sizeof(new_name),
> -								 "%s/%s/", pmu->name, name);
> -							strlist__add(evt_name_list, new_name);
> +						snprintf(new_name, sizeof(new_name),
> +							"%s/%s/", pmu->name, name);
> +						print_cb->print_event(print_state,
> +								"cache",
> +								pmu->name,
> +								name,
> +								new_name,
> +								/*scale_unit=*/NULL,
> +								/*deprecated=*/false,
> +								event_type_descriptor,
> +								/*desc=*/NULL,
> +								/*long_desc=*/NULL,
> +								/*encoding_desc=*/NULL);
>  					}
>  				}
>  			}
>  		}
>  	}
> -
> -	strlist__for_each_entry(nd, evt_name_list) {
> -		print_cb->print_event(print_state,
> -				"cache",
> -				/*pmu_name=*/NULL,
> -				nd->s,
> -				/*event_alias=*/NULL,
> -				/*scale_unit=*/NULL,
> -				/*deprecated=*/false,
> -				event_type_descriptors[PERF_TYPE_HW_CACHE],
> -				/*desc=*/NULL,
> -				/*long_desc=*/NULL,
> -				/*encoding_desc=*/NULL);
> -	}
> -	strlist__delete(evt_name_list);
>  	return 0;
>  }
>