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;
Date:   Wed, 6 Apr 2022 11:11:32 -0600
From:   Mathieu Poirier <mathieu.poirier@linaro.org>
To:     Mike Leach <mike.leach@linaro.org>
Cc:     suzuki.poulose@arm.com, coresight@lists.linaro.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
        linux-perf-users@vger.kernel.org, leo.yan@linaro.org
Subject: Re: [PATCH 06/10] coresight: perf: traceid: Add perf notifiers for
 trace ID
Message-ID: <20220406171132.GA16110@p14s>
References: <20220308205000.27646-1-mike.leach@linaro.org>
 <20220308205000.27646-7-mike.leach@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220308205000.27646-7-mike.leach@linaro.org>
Precedence: bulk

On Tue, Mar 08, 2022 at 08:49:56PM +0000, Mike Leach wrote:
> Adds in notifier calls to the trace ID allocator that perf
> events are starting and stopping.
> 
> This ensures that Trace IDs associated with CPUs remain the same
> throughout the perf session, and are only release when all perf
> sessions are complete.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-etm-perf.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index c039b6ae206f..008f9dac429d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -22,6 +22,7 @@
>  #include "coresight-etm-perf.h"
>  #include "coresight-priv.h"
>  #include "coresight-syscfg.h"
> +#include "coresight-trace-id.h"
>  
>  static struct pmu etm_pmu;
>  static bool etm_perf_up;
> @@ -223,11 +224,21 @@ static void free_event_data(struct work_struct *work)
>  		struct list_head **ppath;
>  
>  		ppath = etm_event_cpu_path_ptr(event_data, cpu);
> -		if (!(IS_ERR_OR_NULL(*ppath)))
> +		if (!(IS_ERR_OR_NULL(*ppath))) {
>  			coresight_release_path(*ppath);
> +			/*
> +			 * perf may have read a trace id for a cpu, but never actually
> +			 * executed code on that cpu - which means the trace id would
> +			 * not release on disable. Re-release here to be sure.
> +			 */
> +			coresight_trace_id_put_cpu_id(cpu, coresight_get_trace_id_map());

A CPU gets a traceID in event_etm_start() when the event is installed for
running.  Do you see a scenario where etm_free_aux() is called without
previously calling event_etm_stop()? 

> +		}
>  		*ppath = NULL;
>  	}
>  
> +	/* mark perf event as done for trace id allocator */
> +	coresight_trace_id_perf_stop();
> +
>  	free_percpu(event_data->path);
>  	kfree(event_data);
>  }
> @@ -314,6 +325,9 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
>  		sink = user_sink = coresight_get_sink_by_id(id);
>  	}
>  
> +	/* tell the trace ID allocator that a perf event is starting up */
> +	coresight_trace_id_perf_start();
> +
>  	/* check if user wants a coresight configuration selected */
>  	cfg_hash = (u32)((event->attr.config2 & GENMASK_ULL(63, 32)) >> 32);
>  	if (cfg_hash) {
> -- 
> 2.17.1
>