linux-kernel - Re: [PATCH 14/17] coresight: Use per-sink trace ID maps for Perf sessions

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: <3923dc07-c037-452a-9e77-d407703876cd@arm.com>
Date: Tue, 7 May 2024 11:52:34 +0100
From: Suzuki K Poulose <suzuki.poulose@....com>
To: James Clark <james.clark@....com>, linux-perf-users@...r.kernel.org,
 gankulkarni@...amperecomputing.com, scclevenger@...amperecomputing.com,
 coresight@...ts.linaro.org, mike.leach@...aro.org
Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 Adrian Hunter <adrian.hunter@...el.com>, John Garry
 <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
 Leo Yan <leo.yan@...ux.dev>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH 14/17] coresight: Use per-sink trace ID maps for Perf
 sessions

On 29/04/2024 16:22, James Clark wrote:
> This will allow sessions with more than CORESIGHT_TRACE_IDS_MAX ETMs
> as long as there are fewer than that many ETMs connected to each sink.
> 
> Each sink owns its own trace ID map, and any Perf session connecting to
> that sink will allocate from it, even if the sink is currently in use by
> other users. This is similar to the existing behavior where the dynamic
> trace IDs are constant as long as there is any concurrent Perf session
> active. It's not completely optimal because slightly more IDs will be
> used than necessary, but the optimal solution involves tracking the PIDs
> of each session and allocating ID maps based on the session owner. This
> is difficult to do with the combination of per-thread and per-cpu modes
> and some scheduling issues. The complexity of this isn't likely to worth
> it because even with multiple users they'd just see a difference in the
> ordering of ID allocations rather than hitting any limits (unless the
> hardware does have too many ETMs connected to one sink).
> 
> Signed-off-by: James Clark <james.clark@....com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c     | 10 ++++++++++
>   drivers/hwtracing/coresight/coresight-etm-perf.c | 15 ++++++++-------
>   include/linux/coresight.h                        |  1 +
>   3 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 9fc6f6b863e0..d1adff467670 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -902,6 +902,7 @@ static void coresight_device_release(struct device *dev)
>   	struct coresight_device *csdev = to_coresight_device(dev);
>   
>   	fwnode_handle_put(csdev->dev.fwnode);
> +	free_percpu(csdev->perf_id_map.cpu_map);
>   	kfree(csdev);
>   }
>   
> @@ -1159,6 +1160,14 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>   	csdev->dev.fwnode = fwnode_handle_get(dev_fwnode(desc->dev));
>   	dev_set_name(&csdev->dev, "%s", desc->name);
>   
> +	if (csdev->type == CORESIGHT_DEV_TYPE_SINK ||
> +	    csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) {
> +		csdev->perf_id_map.cpu_map = alloc_percpu(atomic_t);
> +		if (!csdev->perf_id_map.cpu_map) {
> +			ret = -ENOMEM;
> +			goto err_out;
> +		}
> +	}
>   	/*
>   	 * Make sure the device registration and the connection fixup
>   	 * are synchronised, so that we don't see uninitialised devices
> @@ -1216,6 +1225,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>   err_out:
>   	/* Cleanup the connection information */
>   	coresight_release_platform_data(NULL, desc->dev, desc->pdata);
> +	kfree(csdev);
>   	return ERR_PTR(ret);
>   }
>   EXPORT_SYMBOL_GPL(coresight_register);
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 177cecae38d9..86ca1a9d09a7 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -229,10 +229,13 @@ 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))) {
> +			struct coresight_device *sink = coresight_get_sink(*ppath);
> +
> +			coresight_trace_id_put_cpu_id(cpu, &sink->perf_id_map);
>   			coresight_release_path(*ppath);
> +		}
>   		*ppath = NULL;
> -		coresight_trace_id_put_cpu_id(cpu, coresight_trace_id_map_default());
>   	}
>   
>   	/* mark perf event as done for trace id allocator */
> @@ -401,8 +404,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
>   		}
>   
>   		/* ensure we can allocate a trace ID for this CPU */
> -		trace_id = coresight_trace_id_get_cpu_id(cpu,
> -							 coresight_trace_id_map_default());
> +		trace_id = coresight_trace_id_get_cpu_id(cpu, &sink->perf_id_map);

We could either store the perf_id_map or the traceid itself in the 
event_data isn't it ? Rather than passing the idmap to enable_source ?

Suzuki