| 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
| ||
|
Message-ID: <CAJ9a7VicoyFAh1m+0wcEdKan_whfXP6cb7GBZvBXCukfyugjgg@mail.gmail.com>
Date: Fri, 3 May 2024 10:43:56 +0100
From: Mike Leach <mike.leach@...aro.org>
To: James Clark <james.clark@....com>
Cc: linux-perf-users@...r.kernel.org, gankulkarni@...amperecomputing.com,
scclevenger@...amperecomputing.com, coresight@...ts.linaro.org,
suzuki.poulose@....com,
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
Hi James
On Mon, 29 Apr 2024 at 16:25, James Clark <james.clark@....com> 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);
> if (!IS_VALID_CS_TRACE_ID(trace_id)) {
> cpumask_clear_cpu(cpu, mask);
> coresight_release_path(path);
> @@ -497,7 +499,7 @@ static void etm_event_start(struct perf_event *event, int flags)
>
> /* Finally enable the tracer */
> if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF,
> - coresight_trace_id_map_default()))
> + &sink->perf_id_map))
> goto fail_disable_path;
>
> /*
> @@ -509,8 +511,7 @@ static void etm_event_start(struct perf_event *event, int flags)
> hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK,
> CS_AUX_HW_ID_CURR_VERSION);
> hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK,
> - coresight_trace_id_read_cpu_id(cpu,
> - coresight_trace_id_map_default()));
> + coresight_trace_id_read_cpu_id(cpu, &sink->perf_id_map));
> perf_report_aux_output_id(event, hw_id);
> }
>
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 3a678e5425dc..8c4c1860c76b 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -290,6 +290,7 @@ struct coresight_device {
> bool sysfs_sink_activated;
> struct dev_ext_attribute *ea;
> struct coresight_device *def_sink;
> + struct coresight_trace_id_map perf_id_map;
perhaps this should be sink_id_map? At some point sysfs may use is and
naming is sink... is more consistent with other sink attributes in the
structure.
> /* sysfs links between components */
> int nr_links;
> bool has_conns_grp;
> --
> 2.34.1
>
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists