Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <563274be-5acf-3452-4031-797b1ca9c30c@arm.com>
Date:   Tue, 17 Jan 2023 12:04:21 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v7 09/15] perf: cs-etm: Move mapping of Trace ID and cpu
 into helper function
Content-Language: en-US
To:     Mike Leach <mike.leach@linaro.org>, coresight@lists.linaro.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Cc:     mathieu.poirier@linaro.org, peterz@infradead.org, mingo@redhat.com,
        acme@kernel.org, linux-perf-users@vger.kernel.org,
        leo.yan@linaro.org, quic_jinlmao@quicinc.com
References: <20230116124928.5440-1-mike.leach@linaro.org>
 <20230116124928.5440-10-mike.leach@linaro.org>
From:   Suzuki K Poulose <suzuki.poulose@arm.com>
In-Reply-To: <20230116124928.5440-10-mike.leach@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Mike


On 16/01/2023 12:49, Mike Leach wrote:
> The information to associate Trace ID and CPU will be changing.
> 
> Drivers will start outputting this as a hardware ID packet in the data
> file which if present will be used in preference to the AUXINFO values.
> 
> To prepare for this we provide a helper functions to do the individual ID
> mapping, and one to extract the IDs from the completed metadata blocks.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> Reviewed-by: James Clark <james.clark@arm.com>
> Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>   tools/include/linux/coresight-pmu.h |  5 ++
>   tools/perf/util/cs-etm.c            | 91 +++++++++++++++++++----------
>   tools/perf/util/cs-etm.h            | 14 ++++-
>   3 files changed, 77 insertions(+), 33 deletions(-)
> 

...

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 33303d03c2fa..f77260b9253e 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c

...

>   int cs_etm__process_auxtrace_info_full(union perf_event *event,
>   				       struct perf_session *session)
>   {
> @@ -2755,7 +2809,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
>   	int event_header_size = sizeof(struct perf_event_header);
>   	int total_size = auxtrace_info->header.size;
>   	int priv_size = 0;
> -	int num_cpu, trcidr_idx;
> +	int num_cpu;
>   	int err = 0;
>   	int i, j;
>   	u64 *ptr = NULL;
> @@ -2794,23 +2848,13 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
>   				cs_etm__create_meta_blk(ptr, &i,
>   							CS_ETM_PRIV_MAX,
>   							CS_ETM_NR_TRC_PARAMS_V0);
> -
> -			/* The traceID is our handle */
> -			trcidr_idx = CS_ETM_ETMTRACEIDR;
> -
>   		} else if (ptr[i] == __perf_cs_etmv4_magic) {
>   			metadata[j] =
>   				cs_etm__create_meta_blk(ptr, &i,
>   							CS_ETMV4_PRIV_MAX,
>   							CS_ETMV4_NR_TRC_PARAMS_V0);
> -
> -			/* The traceID is our handle */
> -			trcidr_idx = CS_ETMV4_TRCTRACEIDR;
>   		} else if (ptr[i] == __perf_cs_ete_magic) {
>   			metadata[j] = cs_etm__create_meta_blk(ptr, &i, CS_ETE_PRIV_MAX, -1);
> -
> -			/* ETE shares first part of metadata with ETMv4 */
> -			trcidr_idx = CS_ETMV4_TRCTRACEIDR;
>   		} else {
>   			ui__error("CS ETM Trace: Unrecognised magic number %#"PRIx64". File could be from a newer version of perf.\n",
>   				  ptr[i]);
> @@ -2822,26 +2866,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
>   			err = -ENOMEM;
>   			goto err_free_metadata;
>   		}
> -
> -		/* Get an RB node for this CPU */
> -		inode = intlist__findnew(traceid_list, metadata[j][trcidr_idx]);
> -
> -		/* Something went wrong, no need to continue */
> -		if (!inode) {
> -			err = -ENOMEM;
> -			goto err_free_metadata;
> -		}
> -
> -		/*
> -		 * The node for that CPU should not be taken.
> -		 * Back out if that's the case.
> -		 */
> -		if (inode->priv) {
> -			err = -EINVAL;
> -			goto err_free_metadata;
> -		}
> -		/* All good, associate the traceID with the metadata pointer */
> -		inode->priv = metadata[j];


This gives me a build error as below :

util/cs-etm.c: In function ‘cs_etm__process_auxtrace_info_full’:
util/cs-etm.c:2808:19: error: unused variable ‘inode’ 
[-Werror=unused-variable]
  2808 |  struct int_node *inode;
       |                   ^~~~~


I have folded the following fix in:

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index f77260b9253e..413a89b22fec 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -2805,7 +2805,6 @@ int cs_etm__process_auxtrace_info_full(union 
perf_event *event,
  {
         struct perf_record_auxtrace_info *auxtrace_info = 
&event->auxtrace_info;
         struct cs_etm_auxtrace *etm = NULL;
-       struct int_node *inode;
         int event_header_size = sizeof(struct perf_event_header);
         int total_size = auxtrace_info->header.size;
         int priv_size = 0;


Thanks
Suzuki