Concurrent per-thread events results in a WARN on N1SDP which leads to the realization that per-thread events shouldn't have been sharing sinks in the first place.
This slips through because different per-thread events will have the same PID if owned by the same process, and we only check the PID and nothing else. That results in unexpected WARNs because it looks like we assumed it couldn't happen (although exclusive PMU rules allow it). But even if it was supported it would result in trace from the wrong thread in another event's per-thread buffer, so we should disallow it.
Fix it by determining if sharing can happen at setup time. Then remove everywhere the same PID checking logic was copy pasted in each sink.
The first three patches fix configuration initialization, AUX cleanup and topology locking. The remaining patches enforce the sharing rules and simplify the sink drivers.
Signed-off-by: James Clark james.clark@linaro.org --- Changes in v4: - Count CPU sources in the registered topology and retain each sink's sharing classification until its last AUX session ends. - Serialize AUX topology access and provide locked and locking path release functions. - Reject unexpected path setup errors while retaining silent skips for missing ETMs and unreachable sinks. - Prevent all problematic sharing cases at aux setup rather than at runtime. (Leo) - Drop "coresight: tmc-etr: Don't stop Perf cleanup for active sysfs reads". It was a pre-existing bug and is even more unrelated now with the new model. - Remove workqueue rather than fix the lifecycle, its no longer needed. (Leo) - Add a getter for the config manager, cscfg_get_state(). (Leo) - Link to v3: https://lore.kernel.org/r/20260728-james-cs-multiple-per-threads-v3-0-6aee75...
Changes in v3: - Storing and accessing event owners at runtime causes problems due to various scenarios of: events (and sibling events) exiting, children inheriting event FDs, PID reuse, CPU affine events that also have a target process set but different inherit settings. Fix it by creating a session ID in etm_setup_aux() and holding the references in it for the duration of the whole session. (Leo) - Make the ETR buffer allocator consistent with sink sharing rules by not doing numeric PID comparisons there either. - Fix up some Sashiko reports that it sees after interacting with cscfg and taking extra references to tasks and PIDs. - Link to v2: https://lore.kernel.org/r/20260709-james-cs-multiple-per-threads-v2-0-10ac7a...
Changes in v2: - Fix inherited events by following event->parent - Link to v1: https://lore.kernel.org/r/20260709-james-cs-multiple-per-threads-v1-0-d384e6...
--- James Clark (10): coresight: configfs: Don't assume active until cscfg_mgr is set coresight: etm-perf: Remove workqueue for freeing AUX data coresight: etm-perf: Serialize AUX topology access coresight: etm-perf: Reject event if building path fails coresight: etm-perf: Prevent sink sharing at setup time coresight: tmc-etr: Reflect new sharing rules in allocator coresight: tmc-etr: Remove sharing guards from ETR coresight: tmc-etf: Remove sharing guards from ETF coresight: etb10: Remove sharing guards from ETB10 coresight: ultrasoc-smb: Remove sharing guards from SMB
drivers/hwtracing/coresight/coresight-core.c | 104 +++++++++++++++-- drivers/hwtracing/coresight/coresight-etb10.c | 28 +---- drivers/hwtracing/coresight/coresight-etm-perf.c | 119 ++++++++++--------- drivers/hwtracing/coresight/coresight-etm-perf.h | 2 - drivers/hwtracing/coresight/coresight-priv.h | 4 +- drivers/hwtracing/coresight/coresight-syscfg.c | 11 +- drivers/hwtracing/coresight/coresight-sysfs.c | 4 +- drivers/hwtracing/coresight/coresight-tmc-core.c | 6 +- drivers/hwtracing/coresight/coresight-tmc-etf.c | 22 +--- drivers/hwtracing/coresight/coresight-tmc-etr.c | 142 +++++++---------------- drivers/hwtracing/coresight/coresight-tmc.h | 16 +-- drivers/hwtracing/coresight/coresight-trace-id.c | 97 ++++++++++++++-- drivers/hwtracing/coresight/coresight-trace-id.h | 26 +++-- drivers/hwtracing/coresight/ultrasoc-smb.c | 20 +--- drivers/hwtracing/coresight/ultrasoc-smb.h | 3 - include/linux/coresight.h | 26 ++++- 16 files changed, 346 insertions(+), 284 deletions(-) --- base-commit: 343cfcbb3ecfdb80aa576a428cfa3b65ad783842 change-id: 20260708-james-cs-multiple-per-threads-ed1d25ed1734
Best regards, -- James Clark james.clark@linaro.org
Perf's etm_setup_aux() calls cscfg_activate_config() and dereferences cscfg_mgr, which can be done after coresight_init() has initialized the Perf PMU but before cscfg_init() has finished while the driver is still loading.
Fix it by only assuming cscfg is initialized if cscfg_mgr is set.
Fixes: a0114b4740dd ("coresight: etm-perf: Update to activate selected configuration") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-syscfg.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c index 2bfdd7b45e49..7e3c9844bc00 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.c +++ b/drivers/hwtracing/coresight/coresight-syscfg.c @@ -26,6 +26,11 @@ static DEFINE_MUTEX(cscfg_mutex); /* only one of these */ static struct cscfg_manager *cscfg_mgr;
+static enum cscfg_load_ops cscfg_get_state(void) +{ + return cscfg_mgr ? cscfg_mgr->load_state : CSCFG_UNLOAD; +} + /* load features and configuations into the lists */
/* get name feature instance from a coresight device list of features */ @@ -581,7 +586,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **config_descs, int err = 0;
mutex_lock(&cscfg_mutex); - if (cscfg_mgr->load_state != CSCFG_NONE) { + if (cscfg_get_state() != CSCFG_NONE) { mutex_unlock(&cscfg_mutex); return -EBUSY; } @@ -662,7 +667,7 @@ int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info) struct cscfg_load_owner_info *load_list_item = NULL;
mutex_lock(&cscfg_mutex); - if (cscfg_mgr->load_state != CSCFG_NONE) { + if (cscfg_get_state() != CSCFG_NONE) { mutex_unlock(&cscfg_mutex); return -EBUSY; } @@ -902,7 +907,7 @@ static int _cscfg_activate_config(unsigned long cfg_hash) struct cscfg_config_desc *config_desc; int err = -EINVAL;
- if (cscfg_mgr->load_state == CSCFG_UNLOAD) + if (cscfg_get_state() == CSCFG_UNLOAD) return -EBUSY;
list_for_each_entry(config_desc, &cscfg_mgr->config_desc_list, item) {
AUX cleanup is deferred to a workqueue which is never flushed on module unload. Pending work items can therefore run after coresight-config is cleaned up or the module text is unloaded.
Since commit 95ff4ca26c49 ("perf/core: Free AUX pages in unmap path"), Perf core stops AUX transactions and frees the PMU AUX data from the unmap path in non-atomic context. Release the resources directly from the free_aux() callback and remove the workqueue.
Fixes: 0bcbf2e30ff2 ("coresight: etm-perf: new PMU driver for ETM tracers") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++------------ drivers/hwtracing/coresight/coresight-etm-perf.h | 2 -- 2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 09b21a711a87..2323a68584d4 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -18,7 +18,6 @@ #include <linux/slab.h> #include <linux/stringhash.h> #include <linux/types.h> -#include <linux/workqueue.h>
#include "coresight-config.h" #include "coresight-etm-perf.h" @@ -211,13 +210,12 @@ static void free_sink_buffer(struct etm_event_data *event_data) sink_ops(sink)->free_buffer(event_data->snk_config); }
-static void free_event_data(struct work_struct *work) +static void etm_free_aux(void *data) { int cpu; cpumask_t *mask; - struct etm_event_data *event_data; + struct etm_event_data *event_data = data;
- event_data = container_of(work, struct etm_event_data, work); mask = &event_data->mask;
/* Free the sink buffers, if there are any */ @@ -288,13 +286,6 @@ static void *alloc_event_data(int cpu) return event_data; }
-static void etm_free_aux(void *data) -{ - struct etm_event_data *event_data = data; - - schedule_work(&event_data->work); -} - /* * Check if two given sinks are compatible with each other, * so that they can use the same sink buffers, when an event @@ -436,7 +427,6 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, event_data = alloc_event_data(cpu); if (!event_data) return NULL; - INIT_WORK(&event_data->work, free_event_data);
/* First get the selected sink from user space. */ sink_hash = ATTR_CFG_GET_FLD(&event->attr, sinkid); diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 24d929428633..5e8d985522bd 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -84,7 +84,6 @@ struct etm_filters {
/** * struct etm_event_data - Coresight specifics associated to an event - * @work: Handle to free allocated memory outside IRQ context. * @mask: Hold the CPU(s) this event was set for. * @aux_hwid_done: Whether a CPU has emitted the TraceID packet or not. * @snk_config: The sink configuration. @@ -92,7 +91,6 @@ struct etm_filters { * @path: An array of path, each slot for one CPU. */ struct etm_event_data { - struct work_struct work; cpumask_t mask; cpumask_t aux_hwid_done; void *snk_config;
Perf AUX setup walks the CoreSight connections to select a sink, build paths and acquire references to the devices and their helpers. Unlike sysfs tracing, it does not hold coresight_mutex. Driver registration and removal can therefore change or free connections while setup walks them. Adding a CTI helper can also reallocate an existing device's output connection array even when the device is already referenced by a path.
Hold coresight_mutex from sink selection through buffer allocation. The allocation must also be protected because ETR buffers can look up a connected CATU. Release the mutex before calling etm_free_aux() on error.
AUX cleanup walks helper connections again when releasing paths. Add a locking coresight_release_path() wrapper and use coresight_release_path_locked() for sysfs and setup error paths which already hold the mutex. Leave sink buffer freeing and configuration deactivation outside the mutex. AUX cleanup runs in sleepable context following the preceding workqueue removal.
Add lockdep assertions to path construction and release to document the existing sysfs locking contract and enforce it for perf as well. This protects sleepable AUX topology walks; connection reads in atomic path enable/disable callbacks need a separate solution.
Fixes: 0bcbf2e30ff2 ("coresight: etm-perf: new PMU driver for ETM tracers") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-core.c | 20 ++++++++++++++++---- drivers/hwtracing/coresight/coresight-etm-perf.c | 11 +++++++---- drivers/hwtracing/coresight/coresight-priv.h | 1 + drivers/hwtracing/coresight/coresight-sysfs.c | 4 ++-- 4 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index d2f9c1fb77d9..c05d657cde73 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -30,8 +30,8 @@ #include "coresight-trace-id.h"
/* - * Mutex used to lock all sysfs enable and disable actions and loading and - * unloading devices by the Coresight core. + * Mutex used to serialize device registration and removal, sysfs enable and + * disable actions, and perf AUX setup and path release. */ DEFINE_MUTEX(coresight_mutex); static DEFINE_PER_CPU(struct coresight_device *, csdev_sink); @@ -910,6 +910,8 @@ static void coresight_drop_device(struct coresight_device *csdev) { int i;
+ lockdep_assert_held(&coresight_mutex); + coresight_put_ref(csdev); for (i = 0; i < csdev->pdata->nr_outconns; i++) { struct coresight_device *child; @@ -1046,6 +1048,8 @@ struct coresight_path *coresight_build_path(struct coresight_device *source, struct coresight_path *path; int rc;
+ lockdep_assert_held(&coresight_mutex); + if (!sink) return ERR_PTR(-EINVAL);
@@ -1065,17 +1069,19 @@ struct coresight_path *coresight_build_path(struct coresight_device *source, }
/** - * coresight_release_path - release a previously built path. + * coresight_release_path_locked - release a path with coresight_mutex held. * @path: the path to release. * * Go through all the elements of a path and 1) removed it from the list and * 2) free the memory allocated for each node. */ -void coresight_release_path(struct coresight_path *path) +void coresight_release_path_locked(struct coresight_path *path) { struct coresight_device *csdev; struct coresight_node *nd, *next;
+ lockdep_assert_held(&coresight_mutex); + list_for_each_entry_safe(nd, next, &path->path_list, link) { csdev = nd->csdev;
@@ -1087,6 +1093,12 @@ void coresight_release_path(struct coresight_path *path) kfree(path); }
+void coresight_release_path(struct coresight_path *path) +{ + guard(mutex)(&coresight_mutex); + coresight_release_path_locked(path); +} + /* return true if the device is a suitable type for a default sink */ static bool coresight_is_def_sink_type(struct coresight_device *csdev) { diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 2323a68584d4..faec4c7bb83d 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -402,7 +402,7 @@ etm_event_build_path(struct perf_event *event, int cpu, /* ensure we can allocate a trace ID for this CPU */ ret = coresight_path_assign_trace_id(path, CS_MODE_PERF); if (ret) { - coresight_release_path(path); + coresight_release_path_locked(path); path = NULL; goto out; } @@ -428,6 +428,9 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, if (!event_data) return NULL;
+ /* Protect sink selection, path construction and buffer allocation. */ + mutex_lock(&coresight_mutex); + /* First get the selected sink from user space. */ sink_hash = ATTR_CFG_GET_FLD(&event->attr, sinkid); if (sink_hash) @@ -498,13 +501,13 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, if (!event_data->snk_config) goto err;
-out: + mutex_unlock(&coresight_mutex); return event_data;
err: + mutex_unlock(&coresight_mutex); etm_free_aux(event_data); - event_data = NULL; - goto out; + return NULL; }
static int etm_event_resume(struct coresight_path *path) diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index dddac946659f..edec5a7d2be3 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -143,6 +143,7 @@ coresight_find_default_sink(struct coresight_device *csdev); struct coresight_path *coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink); void coresight_release_path(struct coresight_path *path); +void coresight_release_path_locked(struct coresight_path *path); int coresight_add_sysfs_link(struct coresight_sysfs_link *info); void coresight_remove_sysfs_link(struct coresight_sysfs_link *info); int coresight_create_conns_sysfs_group(struct coresight_device *csdev); diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c index 75000fe64fda..5dd4e9b0426e 100644 --- a/drivers/hwtracing/coresight/coresight-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-sysfs.c @@ -230,7 +230,7 @@ int coresight_enable_sysfs(struct coresight_device *csdev) coresight_disable_path(path);
err_path: - coresight_release_path(path); + coresight_release_path_locked(path); goto out; } EXPORT_SYMBOL_GPL(coresight_enable_sysfs); @@ -256,7 +256,7 @@ void coresight_disable_sysfs(struct coresight_device *csdev) goto out;
coresight_disable_path(path); - coresight_release_path(path); + coresight_release_path_locked(path);
out: mutex_unlock(&coresight_mutex);
etm_event_build_path() silently skips error cases to support opening cpu=-1 events on all CPUs when a system might not have reachable sinks or ETMs for every CPU. In the next commit we want to reject the event if invalid sharing on any sink is detected, so propagate errors.
This means that things like failed memory allocations when building the path and invalid aux pause options now reject the event which is probably an improvement.
Keep the silent skips for things that make sense like missing ETMs and sinks.
Previously, _coresight_build_path() used -ENODEV both when no route to the sink existed and when coresight_grab_device() could not acquire a device or helper on a found route. Return -ENOENT exclusively for the unreachable case, and retain -ENODEV for device acquisition failures, such as a missing driver or failure to take a module reference.
Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-core.c | 22 ++++++--- drivers/hwtracing/coresight/coresight-etm-perf.c | 61 +++++++++++++----------- 2 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index c05d657cde73..a635c51c25ac 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -980,6 +980,9 @@ int coresight_path_assign_trace_id(struct coresight_path *path, * From there the sink is added to the list along with all the devices that led * to that point - the end result is a list from source to sink. In that list * the source is the first device and the sink the last one. + * + * Return: 0 on success, -ENOENT if the sink is unreachable, or another error + * if path construction fails. */ static int _coresight_build_path(struct coresight_device *csdev, struct coresight_device *source, @@ -996,10 +999,10 @@ static int _coresight_build_path(struct coresight_device *csdev,
if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) && sink == per_cpu(csdev_sink, csdev->cpu)) { - if (_coresight_build_path(sink, source, sink, path) == 0) { - found = true; - goto out; - } + ret = _coresight_build_path(sink, source, sink, path); + if (ret) + return ret; + goto out; }
/* Not a sink - recursively explore each port found on this element */ @@ -1011,15 +1014,20 @@ static int _coresight_build_path(struct coresight_device *csdev, if (coresight_blocks_source(source, csdev->pdata->out_conns[i])) continue;
- if (child_dev && - _coresight_build_path(child_dev, source, sink, path) == 0) { + if (!child_dev) + continue; + + ret = _coresight_build_path(child_dev, source, sink, path); + if (!ret) { found = true; break; } + if (ret != -ENOENT) + return ret; }
if (!found) - return -ENODEV; + return -ENOENT;
out: /* diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index faec4c7bb83d..a9adf1cca7a8 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -334,32 +334,30 @@ static struct coresight_path *etm_event_get_ctxt_path(struct etm_ctxt *ctxt) return path; }
-static struct coresight_path * -etm_event_build_path(struct perf_event *event, int cpu, - struct coresight_device *user_sink, - struct coresight_device *match_sink) +static int etm_event_build_path(struct perf_event *event, int cpu, + struct coresight_device *user_sink, + struct coresight_device *match_sink, + struct coresight_path **path) { - struct coresight_path *path = NULL; struct coresight_device *source, *sink; - int ret; + int ret = 0;
+ *path = NULL; source = coresight_get_percpu_source_ref(cpu);
- /* - * If there is no ETM associated with this CPU or ever we try to trace - * on this CPU, we handle it accordingly. - */ + /* Silently skip CPUs without a loaded ETM. */ if (!source) - return NULL; + return 0;
/* * If AUX pause feature is enabled but the ETM driver does not - * support the operations, skip for this source. + * support the operations, reject the whole event. */ if (event->attr.aux_start_paused && (!source_ops(source)->pause_perf || !source_ops(source)->resume_perf)) { dev_err_once(&source->dev, "AUX pause is not supported.\n"); + ret = -EINVAL; goto out; }
@@ -380,30 +378,34 @@ etm_event_build_path(struct perf_event *event, int cpu, * trace on a CPU if the sink is not compatible. */
- /* Find the default sink for this ETM */ + /* Silently skip ETMs with no reachable sink loaded */ sink = coresight_find_default_sink(source); if (!sink) goto out;
- /* Check if this sink compatible with the last sink */ + /* Silently skip sinks incompatible with the last sink. */ if (match_sink && !sinks_compatible(match_sink, sink)) goto out; }
/* - * Building a path doesn't enable it, it simply builds a - * list of devices from source to sink that can be - * referenced later when the path is actually needed. + * Build path, but don't enable. Skip CPUs that cannot reach the sink. + * Reject the whole event on other path construction errors. */ - path = coresight_build_path(source, sink); - if (IS_ERR(path)) + *path = coresight_build_path(source, sink); + if (IS_ERR(*path)) { + ret = PTR_ERR(*path); + if (ret == -ENOENT) + ret = 0; + *path = NULL; goto out; + }
- /* ensure we can allocate a trace ID for this CPU */ - ret = coresight_path_assign_trace_id(path, CS_MODE_PERF); + /* Reject whole event if we can't allocate a trace ID for this CPU */ + ret = coresight_path_assign_trace_id(*path, CS_MODE_PERF); if (ret) { - coresight_release_path_locked(path); - path = NULL; + coresight_release_path_locked(*path); + *path = NULL; goto out; }
@@ -411,7 +413,7 @@ etm_event_build_path(struct perf_event *event, int cpu,
out: coresight_put_percpu_source_ref(source); - return IS_ERR_OR_NULL(path) ? NULL : path; + return ret; }
static void *etm_setup_aux(struct perf_event *event, void **pages, @@ -448,15 +450,16 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
/* * Setup the path for each CPU in a trace session. We try to build - * trace path for each CPU in the mask. If we don't find an ETM - * for the CPU or fail to build a path, we clear the CPU from the - * mask and continue with the rest. If ever we try to trace on those - * CPUs, we can handle it and fail the session. + * trace path for each CPU in the mask. Skip CPUs without an ETM, without + * a compatible default sink, or unable to reach the selected sink by + * clearing them from the mask. Other errors reject the whole event. */ for_each_cpu(cpu, mask) { struct coresight_path *path;
- path = etm_event_build_path(event, cpu, user_sink, sink); + if (etm_event_build_path(event, cpu, user_sink, sink, &path)) + goto err; + if (!path) { /* * Failed to create a path for the CPU, clear it from
To simplify lower level code in individual sinks and make tracing more deterministic, check for potential sharing at setup time. Sharing rules are currently enforced after scheduling in the enable() callback, and the user who was not allowed to trace silently drops trace. This isn't ideal because there is no feedback and behaviour depends on scheduling and the actions of other users. It seems to be have designed that way because of a misunderstanding that multiple concurrent per-thread events would be prevented by the perf core exclusive PMU rules, but they are not.
This removes the use case that different users could successfully trace together as long as they collaborate to ensure that their events aren't scheduled concurrently, but in practice, all working use cases when sinks are shared would be single user. It also removes the use case that one user could open multiple per-thread events, but this was already broken and also not supported in Perf userspace decode. With 1:1 sink to ETRs, there is no change.
Sink sharing rules and ownership lifetimes are identical to the trace ID map, so add owning PID into the ID map and re-use the same lock. coresight_trace_id_perf_start() can then be made the place to gate sharing and return -EBUSY when sharing rules prevent starting the session.
Now that coresight_trace_id_perf_start() can fail, we might as well do it before attempting to allocate traceIDs. This has the benefit of making ID allocations deterministic during concurrent session activation because another session that fails to claim the sink doesn't claim trace IDs only to release them again and perturb the successful user.
Use the TGID of the mmap() caller to determine ownership. event->owner may be NULL if the thread that did perf_event_open() has exited, and using the thread group means that we now support sharing in multithreaded use cases.
Walk the registered topology and save the single-source classification when admitting the first session. Keep it until the last session ends, and reject new sessions if the topology has gained sources while existing sessions rely on private buffers.
Fixes: 880af782c6e8 ("coresight: tmc-etf: Add support for CPU-wide trace scenarios") Fixes: 8d03cfd16a72 ("coresight: tmc-etr: Add support for CPU-wide trace scenarios") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-core.c | 62 +++++++++++++++ drivers/hwtracing/coresight/coresight-etm-perf.c | 41 ++++++---- drivers/hwtracing/coresight/coresight-priv.h | 1 + drivers/hwtracing/coresight/coresight-trace-id.c | 97 +++++++++++++++++++++--- drivers/hwtracing/coresight/coresight-trace-id.h | 26 +++++-- include/linux/coresight.h | 26 +++++-- 6 files changed, 217 insertions(+), 36 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index a635c51c25ac..fe64c509bf67 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -50,6 +50,68 @@ struct coresight_node { struct list_head link; };
+static int coresight_add_upstream_node(struct list_head *nodes, + struct coresight_device *csdev) +{ + struct coresight_node *node; + + list_for_each_entry(node, nodes, link) + if (node->csdev == csdev) + return 0; + + node = kmalloc_obj(*node); + if (!node) + return -ENOMEM; + + node->csdev = csdev; + list_add_tail(&node->link, nodes); + return 0; +} + +/* Count registered CPU sources, ignoring source filters. */ +int coresight_count_cpu_sources(struct coresight_device *csdev) +{ + struct coresight_node *node, *tmp; + LIST_HEAD(nodes); + int i, ret, count = 0; + + lockdep_assert_held(&coresight_mutex); + + /* Per-CPU sinks have an implicit connection to their source. */ + if (coresight_is_percpu_sink(csdev)) + return 1; + + ret = coresight_add_upstream_node(&nodes, csdev); + if (ret) + return ret; + + /* Retain visited devices to handle converging paths and cycles. */ + list_for_each_entry(node, &nodes, link) { + csdev = node->csdev; + if (coresight_is_percpu_source(csdev)) { + count++; + continue; + } + + for (i = 0; i < csdev->pdata->nr_inconns; i++) { + struct coresight_connection *conn = csdev->pdata->in_conns[i]; + + if (!conn) + continue; + ret = coresight_add_upstream_node(&nodes, conn->src_dev); + if (ret) + goto out; + } + } + +out: + list_for_each_entry_safe(node, tmp, &nodes, link) { + list_del(&node->link); + kfree(node); + } + return ret ? ret : count; +} + /* * When losing synchronisation a new barrier packet needs to be inserted at the * beginning of the data collected in a buffer. That way the decoder knows that diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index a9adf1cca7a8..5261061e4dd4 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -240,7 +240,7 @@ static void etm_free_aux(void *data) * a refcount is used so that the last event to call * coresight_trace_id_perf_stop() frees all IDs. */ - coresight_trace_id_perf_stop(&sink->perf_sink_id_map); + coresight_trace_id_perf_stop(sink);
coresight_release_path(*ppath); } @@ -337,6 +337,7 @@ static struct coresight_path *etm_event_get_ctxt_path(struct etm_ctxt *ctxt) static int etm_event_build_path(struct perf_event *event, int cpu, struct coresight_device *user_sink, struct coresight_device *match_sink, + struct etm_event_data *event_data, struct coresight_path **path) { struct coresight_device *source, *sink; @@ -358,7 +359,7 @@ static int etm_event_build_path(struct perf_event *event, int cpu, !source_ops(source)->resume_perf)) { dev_err_once(&source->dev, "AUX pause is not supported.\n"); ret = -EINVAL; - goto out; + goto out_src; }
/* If sink has been specified by user, directly use it */ @@ -381,11 +382,11 @@ static int etm_event_build_path(struct perf_event *event, int cpu, /* Silently skip ETMs with no reachable sink loaded */ sink = coresight_find_default_sink(source); if (!sink) - goto out; + goto out_src;
/* Silently skip sinks incompatible with the last sink. */ if (match_sink && !sinks_compatible(match_sink, sink)) - goto out; + goto out_src; }
/* @@ -398,20 +399,27 @@ static int etm_event_build_path(struct perf_event *event, int cpu, if (ret == -ENOENT) ret = 0; *path = NULL; - goto out; + goto out_src; }
+ ret = coresight_trace_id_perf_start(sink, event, event_data); + if (ret) + goto out_path; + /* Reject whole event if we can't allocate a trace ID for this CPU */ ret = coresight_path_assign_trace_id(*path, CS_MODE_PERF); - if (ret) { - coresight_release_path_locked(*path); - *path = NULL; - goto out; - } + if (ret) + goto out_trace_id_started;
- coresight_trace_id_perf_start(&sink->perf_sink_id_map); + coresight_put_percpu_source_ref(source); + return 0;
-out: +out_trace_id_started: + coresight_trace_id_perf_stop(sink); +out_path: + coresight_release_path_locked(*path); + *path = NULL; +out_src: coresight_put_percpu_source_ref(source); return ret; } @@ -457,7 +465,14 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, for_each_cpu(cpu, mask) { struct coresight_path *path;
- if (etm_event_build_path(event, cpu, user_sink, sink, &path)) + /* + * Use event_data as the unique identifier for this event. Event + * itself is not stable and there are some horrible edge cases + * where it's freed and re-used while another event redirecting + * to the same aux buffer keeps the sink allocation alive. + */ + if (etm_event_build_path(event, cpu, user_sink, sink, + event_data, &path)) goto err;
if (!path) { diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index edec5a7d2be3..51313fe2cd0e 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -14,6 +14,7 @@
extern struct mutex coresight_mutex; extern const struct device_type coresight_dev_type[]; +int coresight_count_cpu_sources(struct coresight_device *csdev);
/* * Coresight management registers (0xf00-0xfcc) diff --git a/drivers/hwtracing/coresight/coresight-trace-id.c b/drivers/hwtracing/coresight/coresight-trace-id.c index 7ed337d54d3e..debc30d0de8f 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.c +++ b/drivers/hwtracing/coresight/coresight-trace-id.c @@ -7,9 +7,11 @@ #include <linux/coresight-pmu.h> #include <linux/cpumask.h> #include <linux/kernel.h> +#include <linux/sched/signal.h> #include <linux/spinlock.h> #include <linux/types.h>
+#include "coresight-priv.h" #include "coresight-trace-id.h"
enum trace_id_flags { @@ -128,14 +130,11 @@ static void coresight_trace_id_free(int id, struct coresight_trace_id_map *id_ma */ static void coresight_trace_id_release_all(struct coresight_trace_id_map *id_map) { - unsigned long flags; int cpu;
- raw_spin_lock_irqsave(&id_map->lock, flags); bitmap_zero(id_map->used_ids, CORESIGHT_TRACE_IDS_MAX); for_each_possible_cpu(cpu) atomic_set(per_cpu_ptr(id_map->cpu_map, cpu), 0); - raw_spin_unlock_irqrestore(&id_map->lock, flags); DUMP_ID_MAP(id_map); }
@@ -284,17 +283,95 @@ void coresight_trace_id_put_system_id(int id) } EXPORT_SYMBOL_GPL(coresight_trace_id_put_system_id);
-void coresight_trace_id_perf_start(struct coresight_trace_id_map *id_map) +int coresight_trace_id_perf_start(struct coresight_device *sink, + struct perf_event *event, + struct etm_event_data *event_data) { - atomic_inc(&id_map->perf_cs_etm_session_active); - PERF_SESSION(atomic_read(&id_map->perf_cs_etm_session_active)); + struct coresight_perf_session_id *session = &sink->perf_sink_id_map.session; + struct coresight_trace_id_map *id_map = &sink->perf_sink_id_map; + bool migratable = event->cpu == -1; + int nr_sources; + + /* The caller holds coresight_mutex through path building and admission. */ + nr_sources = coresight_count_cpu_sources(sink); + if (nr_sources < 0) + return nr_sources; + + guard(raw_spinlock)(&id_map->lock); + + if (!session->refcount) { + session->single_source = nr_sources == 1; + } else if (session->single_source && nr_sources != 1) { + /* Existing sessions relied on this sink having only one source. */ + return -EBUSY; + } + + /* + * In addition to the Perf core exclusive PMU rules, prevent disjointed + * sessions when sinks may shared. The following restrictions simplify + * ETR buffer management so we don't have to support + * per-thread/migratable bounce buffers moving to other sinks while + * still in use by other threads on the current sink. Not sharing + * between migratable and non-migratable events also means that we can + * assume a buffer is either always attached to a sink, or always + * attached to an event. + */ + if (!session->single_source) { + /* + * Use 'current' instead of event->owner because that's NULL if + * a worker thread calls perf_event_open() and then exits before + * mmap(). Also allow sink sharing between anyone in the thread + * group to support multithreaded auxtrace mmap() calls. + */ + struct pid *this_pid = task_pid_type(current, PIDTYPE_TGID); + + if (session->owner) { + /* + * Prevent mixing event types per sink, regardless of + * owner. + */ + if (!!session->migratable_event != migratable) + return -EBUSY; + + /* + * Limit migratable events to a single event per sink, + * regardless of owner. + */ + if (migratable && + session->migratable_event != event_data) + return -EBUSY; + + /* Share non migratable events between the same owner */ + if (session->owner != this_pid) + return -EBUSY; + } else { + if (migratable) + session->migratable_event = event_data; + session->owner = get_pid(this_pid); + } + } + + session->refcount++; + PERF_SESSION(session->refcount); + return 0; } EXPORT_SYMBOL_GPL(coresight_trace_id_perf_start);
-void coresight_trace_id_perf_stop(struct coresight_trace_id_map *id_map) +void coresight_trace_id_perf_stop(struct coresight_device *sink) { - if (!atomic_dec_return(&id_map->perf_cs_etm_session_active)) - coresight_trace_id_release_all(id_map); - PERF_SESSION(atomic_read(&id_map->perf_cs_etm_session_active)); + struct coresight_perf_session_id *session = &sink->perf_sink_id_map.session; + struct coresight_trace_id_map *id_map = &sink->perf_sink_id_map; + struct pid *old_pid = NULL; + + scoped_guard(raw_spinlock, &id_map->lock) { + if (--session->refcount <= 0) { + coresight_trace_id_release_all(id_map); + old_pid = session->owner; + id_map->session = (struct coresight_perf_session_id) {}; + } + PERF_SESSION(session->refcount); + } + + put_pid(old_pid); } EXPORT_SYMBOL_GPL(coresight_trace_id_perf_stop); diff --git a/drivers/hwtracing/coresight/coresight-trace-id.h b/drivers/hwtracing/coresight/coresight-trace-id.h index db68e1ec56b6..cd592b0e9076 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.h +++ b/drivers/hwtracing/coresight/coresight-trace-id.h @@ -139,21 +139,31 @@ void coresight_trace_id_put_system_id(int id); /** * Notify the Trace ID allocator that a perf session is starting. * - * Increase the perf session reference count - called by perf when setting up a - * trace event. + * Assign 'current' as the owner of the trace ID map and increment the session + * refcount. * - * Perf sessions never free trace IDs to ensure that the ID associated with a - * CPU cannot change during their and other's concurrent sessions. Instead, - * this refcount is used so that the last event to finish always frees all IDs. + * The caller must hold coresight_mutex while checking the registered topology + * and admitting the session. The first session saves whether the sink has only + * one CPU source, and subsequent sessions retain that classification. + * + * Return 0 on success, -EBUSY if sharing rules don't permit this task to share + * the map, or -ENOMEM if walking the topology fails. + * + * Note: Perf sessions never free trace IDs to ensure that the ID associated + * with a CPU cannot change during their and other's concurrent sessions. + * Instead, this refcount is used so that the last event to finish always frees + * all IDs. */ -void coresight_trace_id_perf_start(struct coresight_trace_id_map *id_map); +int coresight_trace_id_perf_start(struct coresight_device *sink, + struct perf_event *event, + struct etm_event_data *event_data);
/** * Notify the ID allocator that a perf session is stopping. * * Decrease the perf session reference count. If this causes the count to go to - * zero, then all Trace IDs will be released. + * zero, then all Trace IDs and owner PID will be released. */ -void coresight_trace_id_perf_stop(struct coresight_trace_id_map *id_map); +void coresight_trace_id_perf_stop(struct coresight_device *sink);
#endif /* _CORESIGHT_TRACE_ID_H */ diff --git a/include/linux/coresight.h b/include/linux/coresight.h index edf2a2fc697c..e718580ea547 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -235,18 +235,34 @@ struct coresight_sysfs_link { /* architecturally we have 128 IDs some of which are reserved */ #define CORESIGHT_TRACE_IDS_MAX 128
+/** + * Identification of the Perf owner of a trace ID map / sink + * + * @refcount: Number of Perf sessions using this ID map. + * @single_source: Sessions were admitted with only one upstream CPU source. + * Kept unchanged until the last session releases the map. + * @owner: PID that owns the active sessions. + * @migratable_event: Unique pointer to identify the owning event if it's + * migratable (event->cpu == -1). + */ +struct coresight_perf_session_id { + int refcount; + bool single_source; + struct pid *owner; + struct etm_event_data *migratable_event; +}; + /** * Trace ID map. * - * @used_ids: Bitmap to register available (bit = 0) and in use (bit = 1) IDs. - * Initialised so that the reserved IDs are permanently marked as - * in use. - * @perf_cs_etm_session_active: Number of Perf sessions using this ID map. + * @used_ids: Bitmap to register available (bit = 0) and in use (bit = 1) IDs. + * Initialised so that the reserved IDs are permanently marked as + * in use. */ struct coresight_trace_id_map { DECLARE_BITMAP(used_ids, CORESIGHT_TRACE_IDS_MAX); atomic_t __percpu *cpu_map; - atomic_t perf_cs_etm_session_active; + struct coresight_perf_session_id session; raw_spinlock_t lock; };
The IDR map is now unused because only one owner can use a shared sink at a time. Replace it with a single sink local buffer, 'shared_perf_buf'.
And now that we don't have the IDR map to store buffers for different sessions, allocate a private buffer for 1:1 per-cpu sessions as well as per-thread sessions. We didn't eliminate any allocations by sharing buffers in 1:1 scenarios anyway, and we allow multiple per-cpu PERF_ATTACH_TASK events. Obviously multiple users can't use a single shared_perf_buf buffer, so use a per-event one.
Use the single-source classification saved during AUX admission so that buffer allocation follows the same sharing mode as existing sessions.
Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-tmc-core.c | 4 +- drivers/hwtracing/coresight/coresight-tmc-etr.c | 103 ++++++++--------------- drivers/hwtracing/coresight/coresight-tmc.h | 10 +-- 3 files changed, 43 insertions(+), 74 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index bc5a133ada3e..864da1eea52c 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -9,7 +9,6 @@ #include <linux/init.h> #include <linux/types.h> #include <linux/device.h> -#include <linux/idr.h> #include <linux/io.h> #include <linux/iommu.h> #include <linux/err.h> @@ -832,8 +831,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) ret = tmc_etr_setup_caps(dev, devid, &desc.access); if (ret) goto out; - idr_init(&drvdata->idr); - mutex_init(&drvdata->idr_mutex); + mutex_init(&drvdata->perf_buf_mutex); dev_list = "tmc_etr"; break; case TMC_CONFIG_TYPE_ETF: diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 361a433e6f0c..b4b186c605c6 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -8,7 +8,6 @@ #include <linux/coresight.h> #include <linux/dma-mapping.h> #include <linux/iommu.h> -#include <linux/idr.h> #include <linux/mutex.h> #include <linux/refcount.h> #include <linux/slab.h> @@ -1396,16 +1395,13 @@ alloc_etr_buf(struct tmc_drvdata *drvdata, struct perf_event *event, return ERR_PTR(-ENOMEM); }
-static struct etr_buf * -get_perf_etr_buf_cpu_wide(struct tmc_drvdata *drvdata, - struct perf_event *event, int nr_pages, - void **pages, bool snapshot) +static struct etr_buf *get_perf_shared_etr_buf(struct tmc_drvdata *drvdata, + struct perf_event *event, + int nr_pages, void **pages, + bool snapshot) { - int ret; - pid_t pid = task_pid_nr(event->owner); struct etr_buf *etr_buf;
-retry: /* * An etr_perf_buffer is associated with an event and holds a reference * to the AUX ring buffer that was created for that event. In CPU-wide @@ -1424,64 +1420,45 @@ get_perf_etr_buf_cpu_wide(struct tmc_drvdata *drvdata, * allocated for this session. If so it is shared with this event, * otherwise it is created. */ - mutex_lock(&drvdata->idr_mutex); - etr_buf = idr_find(&drvdata->idr, pid); + mutex_lock(&drvdata->perf_buf_mutex); + etr_buf = drvdata->shared_perf_buf; if (etr_buf) { refcount_inc(&etr_buf->refcount); - mutex_unlock(&drvdata->idr_mutex); - return etr_buf; + goto out; }
/* If we made it here no buffer has been allocated, do so now. */ - mutex_unlock(&drvdata->idr_mutex); - etr_buf = alloc_etr_buf(drvdata, event, nr_pages, pages, snapshot); - if (IS_ERR(etr_buf)) - return etr_buf; - - /* Now that we have a buffer, add it to the IDR. */ - mutex_lock(&drvdata->idr_mutex); - ret = idr_alloc(&drvdata->idr, etr_buf, pid, pid + 1, GFP_KERNEL); - mutex_unlock(&drvdata->idr_mutex); - - /* Another event with this session ID has allocated this buffer. */ - if (ret == -ENOSPC) { - tmc_free_etr_buf(etr_buf); - goto retry; - } - - /* The IDR can't allocate room for a new session, abandon ship. */ - if (ret == -ENOMEM) { - tmc_free_etr_buf(etr_buf); - return ERR_PTR(ret); - } - - + if (!IS_ERR(etr_buf)) + drvdata->shared_perf_buf = etr_buf; +out: + mutex_unlock(&drvdata->perf_buf_mutex); return etr_buf; }
-static struct etr_buf * -get_perf_etr_buf_per_thread(struct tmc_drvdata *drvdata, - struct perf_event *event, int nr_pages, - void **pages, bool snapshot) -{ - /* - * In per-thread mode the etr_buf isn't shared, so just go ahead - * with memory allocation. - */ - return alloc_etr_buf(drvdata, event, nr_pages, pages, snapshot); -} - static struct etr_buf * get_perf_etr_buf(struct tmc_drvdata *drvdata, struct perf_event *event, int nr_pages, void **pages, bool snapshot) { - if (event->cpu == -1) - return get_perf_etr_buf_per_thread(drvdata, event, nr_pages, - pages, snapshot); + struct coresight_perf_session_id *session = &drvdata->csdev->perf_sink_id_map.session;
- return get_perf_etr_buf_cpu_wide(drvdata, event, nr_pages, - pages, snapshot); + /* + * Migratable events and 1:1 sinks use private buffers. Don't create a + * drvdata->shared_perf_buf because these type of events support + * multiple concurrent users. The path's session reference keeps the + * saved single-source classification stable throughout this allocation. + */ + if (event->cpu == -1 || session->single_source) + return alloc_etr_buf(drvdata, event, nr_pages, pages, snapshot); + + /* + * Sharing rules defined in coresight_trace_id_perf_start() mean that + * only one user can own the sink now, so we can allocate a sink local + * bounce buffer. If that user had requested events with different aux + * sizes, we'll use the size of the first event to create the buffer. + */ + return get_perf_shared_etr_buf(drvdata, event, nr_pages, pages, + snapshot); }
static struct etr_perf_buffer * @@ -1508,7 +1485,7 @@ tmc_etr_setup_perf_buf(struct tmc_drvdata *drvdata, struct perf_event *event, done: /* * Keep a reference to the ETR this buffer has been allocated for - * in order to have access to the IDR in tmc_free_etr_buffer(). + * in order to clear the shared buffer in tmc_free_etr_buffer(). */ etr_perf->drvdata = drvdata; etr_perf->etr_buf = etr_buf; @@ -1543,28 +1520,22 @@ static void tmc_free_etr_buffer(void *config) { struct etr_perf_buffer *etr_perf = config; struct tmc_drvdata *drvdata = etr_perf->drvdata; - struct etr_buf *buf, *etr_buf = etr_perf->etr_buf; + struct etr_buf *etr_buf = etr_perf->etr_buf;
if (!etr_buf) goto free_etr_perf_buffer;
- mutex_lock(&drvdata->idr_mutex); + mutex_lock(&drvdata->perf_buf_mutex); /* If we are not the last one to use the buffer, don't touch it. */ if (!refcount_dec_and_test(&etr_buf->refcount)) { - mutex_unlock(&drvdata->idr_mutex); + mutex_unlock(&drvdata->perf_buf_mutex); goto free_etr_perf_buffer; }
- /* We are the last one, remove from the IDR and free the buffer. */ - buf = idr_remove(&drvdata->idr, etr_perf->pid); - mutex_unlock(&drvdata->idr_mutex); - - /* - * Something went very wrong if the buffer associated with this ID - * is not the same in the IDR. Leak to avoid use after free. - */ - if (buf && WARN_ON(buf != etr_buf)) - goto free_etr_perf_buffer; + /* Clear the CPU-wide cache before freeing its last reference. */ + if (drvdata->shared_perf_buf == etr_buf) + drvdata->shared_perf_buf = NULL; + mutex_unlock(&drvdata->perf_buf_mutex);
tmc_free_etr_buf(etr_perf->etr_buf);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 319a354ede9f..95e5623ea881 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -8,7 +8,6 @@ #define _CORESIGHT_TMC_H
#include <linux/dma-mapping.h> -#include <linux/idr.h> #include <linux/miscdevice.h> #include <linux/mutex.h> #include <linux/refcount.h> @@ -235,8 +234,8 @@ struct tmc_resrv_buf { * @etr_caps: Bitmask of capabilities of the TMC ETR, inferred from the * device configuration register (DEVID) * @etr_mode: User preferred mode of the ETR device, default auto mode. - * @idr: Holds etr_bufs allocated for this ETR. - * @idr_mutex: Access serialisation for idr. + * @shared_perf_buf: CPU-wide perf buffer shared by this ETR's owner. + * @perf_buf_mutex: Protects shared_perf_buf and its reference count. * @sysfs_buf: SYSFS buffer for ETR. * @perf_buf: PERF buffer for ETR. * @resrv_buf: Used by ETR as hardware trace buffer and for trace data @@ -269,8 +268,9 @@ struct tmc_drvdata { u32 trigger_cntr; u32 etr_caps; enum etr_mode etr_mode; - struct idr idr; - struct mutex idr_mutex; + struct etr_buf *shared_perf_buf; + /* Protects shared_perf_buf and its reference count. */ + struct mutex perf_buf_mutex; struct etr_buf *sysfs_buf; struct etr_buf *perf_buf; struct tmc_resrv_buf resrv_buf;
Potential sink sharing is now identified during AUX setup. Now sinks only need to refcount and use the buffer provided by the event, which is guaranteed to be valid and shareable.
Mismatched buffers seem very unlikely now, so remove the WARN_ON() to reduce how much state is stored in the sink.
Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 39 ++----------------------- drivers/hwtracing/coresight/coresight-tmc.h | 2 -- 2 files changed, 2 insertions(+), 39 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index b4b186c605c6..a2768f5bbfbc 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -36,8 +36,6 @@ struct etr_buf_hw { * etr_perf_buffer - Perf buffer used for ETR * @drvdata - The ETR drvdaga this buffer has been allocated for. * @etr_buf - Actual buffer used by the ETR - * @pid - The PID of the session owner that etr_perf_buffer - * belongs to. * @snaphost - Perf session mode * @nr_pages - Number of pages in the ring buffer. * @pages - Array of Pages in the ring buffer. @@ -45,7 +43,6 @@ struct etr_buf_hw { struct etr_perf_buffer { struct tmc_drvdata *drvdata; struct etr_buf *etr_buf; - pid_t pid; bool snapshot; int nr_pages; void **pages; @@ -1508,7 +1505,6 @@ static void *tmc_alloc_etr_buffer(struct coresight_device *csdev, return NULL; }
- etr_perf->pid = task_pid_nr(event->owner); etr_perf->snapshot = snapshot; etr_perf->nr_pages = nr_pages; etr_perf->pages = pages; @@ -1622,12 +1618,6 @@ tmc_update_etr_buffer(struct coresight_device *csdev, goto out; }
- if (WARN_ON(drvdata->perf_buf != etr_buf)) { - lost = true; - raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); - goto out; - } - CS_UNLOCK(drvdata->base);
tmc_flush_and_stop(drvdata); @@ -1707,7 +1697,6 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, struct coresight_path *path) { int rc = 0; - pid_t pid; unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct perf_output_handle *handle = path->handle; @@ -1720,35 +1709,15 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, goto unlock_out; }
- if (WARN_ON(!etr_perf || !etr_perf->etr_buf)) { - rc = -EINVAL; - goto unlock_out; - } - - /* Get a handle on the pid of the session owner */ - pid = etr_perf->pid; - - /* Do not proceed if this device is associated with another session */ - if (drvdata->pid != -1 && drvdata->pid != pid) { - rc = -EBUSY; - goto unlock_out; - } - - /* - * No HW configuration is needed if the sink is already in - * use for this session. - */ - if (drvdata->pid == pid) { + /* No HW configuration is needed if the sink is already in use. */ + if (csdev->refcnt) { csdev->refcnt++; goto unlock_out; }
rc = tmc_etr_enable_hw(drvdata, etr_perf->etr_buf); if (!rc) { - /* Associate with monitored process. */ - drvdata->pid = pid; coresight_set_mode(csdev, CS_MODE_PERF); - drvdata->perf_buf = etr_perf->etr_buf; csdev->refcnt++; }
@@ -1792,11 +1761,7 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev) /* Complain if we (somehow) got out of sync */ WARN_ON_ONCE(coresight_get_mode(csdev) == CS_MODE_DISABLED); tmc_etr_disable_hw(drvdata); - /* Dissociate from monitored process. */ - drvdata->pid = -1; coresight_set_mode(csdev, CS_MODE_DISABLED); - /* Reset perf specific data */ - drvdata->perf_buf = NULL;
raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 95e5623ea881..d63fa60ecce8 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -237,7 +237,6 @@ struct tmc_resrv_buf { * @shared_perf_buf: CPU-wide perf buffer shared by this ETR's owner. * @perf_buf_mutex: Protects shared_perf_buf and its reference count. * @sysfs_buf: SYSFS buffer for ETR. - * @perf_buf: PERF buffer for ETR. * @resrv_buf: Used by ETR as hardware trace buffer and for trace data * retention (after crash) only when ETR_MODE_RESRV buffer * mode is enabled. Used by ETF for trace data retention @@ -272,7 +271,6 @@ struct tmc_drvdata { /* Protects shared_perf_buf and its reference count. */ struct mutex perf_buf_mutex; struct etr_buf *sysfs_buf; - struct etr_buf *perf_buf; struct tmc_resrv_buf resrv_buf; struct tmc_resrv_buf crash_mdata; };
Potential sink sharing is now identified during AUX setup. Trust the core sharing rules when enabling an event and use the sink reference count to avoid reconfiguring hardware that is already active.
Remove the redundant PID tracking from ETF and the TMC driver state.
Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-tmc-core.c | 2 -- drivers/hwtracing/coresight/coresight-tmc-etf.c | 22 ++-------------------- drivers/hwtracing/coresight/coresight-tmc.h | 4 ---- 3 files changed, 2 insertions(+), 26 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 864da1eea52c..e3c94dca1152 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -800,8 +800,6 @@ static int __tmc_probe(struct device *dev, struct resource *res) devid = readl_relaxed(drvdata->base + CORESIGHT_DEVID); drvdata->config_type = BMVAL(devid, 6, 7); drvdata->memwidth = tmc_get_memwidth(devid); - /* This device is not associated with a session */ - drvdata->pid = -1; drvdata->etr_mode = ETR_MODE_AUTO;
if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) { diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 8882b1c4cdc0..e2204a47c1f6 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -250,11 +250,9 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, struct coresight_path *path) { int ret = 0; - pid_t pid; unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct perf_output_handle *handle = path->handle; - struct cs_buffers *buf = etm_perf_sink_config(handle);
raw_spin_lock_irqsave(&drvdata->spinlock, flags); do { @@ -270,31 +268,18 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, break; }
- /* Get a handle on the pid of the process to monitor */ - pid = buf->pid; - - if (drvdata->pid != -1 && drvdata->pid != pid) { - ret = -EBUSY; - break; - } - ret = tmc_set_etf_buffer(csdev, handle); if (ret) break;
- /* - * No HW configuration is needed if the sink is already in - * use for this session. - */ - if (drvdata->pid == pid) { + /* No HW configuration is needed if the sink is already in use. */ + if (csdev->refcnt) { csdev->refcnt++; break; }
ret = tmc_etb_enable_hw(drvdata); if (!ret) { - /* Associate with monitored process. */ - drvdata->pid = pid; coresight_set_mode(csdev, CS_MODE_PERF); csdev->refcnt++; } @@ -351,8 +336,6 @@ static int tmc_disable_etf_sink(struct coresight_device *csdev) /* Complain if we (somehow) got out of sync */ WARN_ON_ONCE(coresight_get_mode(csdev) == CS_MODE_DISABLED); tmc_etb_disable_hw(drvdata); - /* Dissociate from monitored process. */ - drvdata->pid = -1; coresight_set_mode(csdev, CS_MODE_DISABLED);
raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); @@ -432,7 +415,6 @@ static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, if (!buf) return NULL;
- buf->pid = task_pid_nr(event->owner); buf->snapshot = overwrite; buf->nr_pages = nr_pages; buf->data_pages = pages; diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index d63fa60ecce8..36b54ebf30ba 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -217,9 +217,6 @@ struct tmc_resrv_buf { * @crashdev: specifics to handle "/dev/crash_tmc_xyz" entry for reading * crash tracedata. * @spinlock: only one at a time pls. - * @pid: Process ID of the process that owns the session that is using - * this component. For example this would be the pid of the Perf - * process. * @reading: buffer's in the reading through "/dev/xyz.tmc" entry * @stop_on_flush: Stop on flush trigger user configuration. * @buf: Snapshot of the trace data for ETF/ETB. @@ -252,7 +249,6 @@ struct tmc_drvdata { struct miscdevice miscdev; struct miscdevice crashdev; raw_spinlock_t spinlock; - pid_t pid; bool reading; bool stop_on_flush; union {
Potential sink sharing is now identified during AUX setup. Trust the core sharing rules when enabling an event and use the sink reference count to avoid reconfiguring hardware that is already active.
Remove the redundant PID tracking from ETB10 and its buffer allocation.
Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etb10.c | 28 ++------------------------- 1 file changed, 2 insertions(+), 26 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index a827f76b8144..75c5a71d8d68 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -71,8 +71,6 @@ * @miscdev: specifics to handle "/dev/xyz.etb" entry. * @spinlock: only one at a time pls. * @reading: synchronise user space access to etb buffer. - * @pid: Process ID of the process being monitored by the session - * that is using this component. * @buf: area of memory where ETB buffer content gets sent. * @buffer_depth: size of @buf. * @trigger_cntr: amount of words to store after a trigger. @@ -84,7 +82,6 @@ struct etb_drvdata { struct miscdevice miscdev; raw_spinlock_t spinlock; atomic_t reading; - pid_t pid; u8 *buf; u32 buffer_depth; u32 trigger_cntr; @@ -168,11 +165,9 @@ static int etb_enable_sysfs(struct coresight_device *csdev) static int etb_enable_perf(struct coresight_device *csdev, struct coresight_path *path) { int ret = 0; - pid_t pid; unsigned long flags; struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct perf_output_handle *handle = path->handle; - struct cs_buffers *buf = etm_perf_sink_config(handle);
raw_spin_lock_irqsave(&drvdata->spinlock, flags);
@@ -182,19 +177,8 @@ static int etb_enable_perf(struct coresight_device *csdev, struct coresight_path goto out; }
- /* Get a handle on the pid of the process to monitor */ - pid = buf->pid; - - if (drvdata->pid != -1 && drvdata->pid != pid) { - ret = -EBUSY; - goto out; - } - - /* - * No HW configuration is needed if the sink is already in - * use for this session. - */ - if (drvdata->pid == pid) { + /* No HW configuration is needed if the sink is already in use. */ + if (csdev->refcnt) { csdev->refcnt++; goto out; } @@ -210,8 +194,6 @@ static int etb_enable_perf(struct coresight_device *csdev, struct coresight_path
ret = etb_enable_hw(drvdata); if (!ret) { - /* Associate with monitored process. */ - drvdata->pid = pid; coresight_set_mode(drvdata->csdev, CS_MODE_PERF); csdev->refcnt++; } @@ -361,8 +343,6 @@ static int etb_disable(struct coresight_device *csdev) /* Complain if we (somehow) got out of sync */ WARN_ON_ONCE(coresight_get_mode(csdev) == CS_MODE_DISABLED); etb_disable_hw(drvdata); - /* Dissociate from monitored process. */ - drvdata->pid = -1; coresight_set_mode(csdev, CS_MODE_DISABLED); raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -383,7 +363,6 @@ static void *etb_alloc_buffer(struct coresight_device *csdev, if (!buf) return NULL;
- buf->pid = task_pid_nr(event->owner); buf->snapshot = overwrite; buf->nr_pages = nr_pages; buf->data_pages = pages; @@ -754,9 +733,6 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) if (!drvdata->buf) return -ENOMEM;
- /* This device is not associated with a session */ - drvdata->pid = -1; - pdata = coresight_get_platform_data(dev); if (IS_ERR(pdata)) return PTR_ERR(pdata);
Potential sink sharing is now identified during AUX setup. Trust the core sharing rules when enabling an event and use the sink reference count to avoid reconfiguring hardware that is already active.
Remove the redundant PID tracking from SMB and the now-unused PID field from the common cs_buffers structure.
Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-priv.h | 2 -- drivers/hwtracing/coresight/ultrasoc-smb.c | 20 +------------------- drivers/hwtracing/coresight/ultrasoc-smb.h | 3 --- 3 files changed, 1 insertion(+), 24 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 51313fe2cd0e..9eabfdfa32e1 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -95,7 +95,6 @@ enum etm_addr_type { * struct cs_buffer - keep track of a recording session' specifics * @cur: index of the current buffer * @nr_pages: max number of pages granted to us - * @pid: PID this cs_buffer belongs to * @offset: offset within the current buffer * @data_size: how much we collected in this run * @snapshot: is this run in snapshot mode @@ -104,7 +103,6 @@ enum etm_addr_type { struct cs_buffers { unsigned int cur; unsigned int nr_pages; - pid_t pid; unsigned long offset; local_t data_size; bool snapshot; diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c index b107d4606fcd..f2efbb615cda 100644 --- a/drivers/hwtracing/coresight/ultrasoc-smb.c +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c @@ -214,23 +214,9 @@ static int smb_enable_perf(struct coresight_device *csdev, struct coresight_path *path) { struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent); - struct perf_output_handle *handle = path->handle; - struct cs_buffers *buf = etm_perf_sink_config(handle); - pid_t pid;
- if (!buf) - return -EINVAL; - - /* Get a handle on the pid of the target process */ - pid = buf->pid; - - /* Device is already in used by other session */ - if (drvdata->pid != -1 && drvdata->pid != pid) - return -EBUSY; - - if (drvdata->pid == -1) { + if (!csdev->refcnt) { smb_enable_hw(drvdata); - drvdata->pid = pid; coresight_set_mode(csdev, CS_MODE_PERF); }
@@ -292,8 +278,6 @@ static int smb_disable(struct coresight_device *csdev)
smb_disable_hw(drvdata);
- /* Dissociate from the target process. */ - drvdata->pid = -1; coresight_set_mode(csdev, CS_MODE_DISABLED); dev_dbg(&csdev->dev, "Ultrasoc SMB disabled\n");
@@ -315,7 +299,6 @@ static void *smb_alloc_buffer(struct coresight_device *csdev, buf->snapshot = overwrite; buf->nr_pages = nr_pages; buf->data_pages = pages; - buf->pid = task_pid_nr(event->owner);
return buf; } @@ -563,7 +546,6 @@ static int smb_probe(struct platform_device *pdev) smb_reset_buffer(drvdata); platform_set_drvdata(pdev, drvdata); raw_spin_lock_init(&drvdata->spinlock); - drvdata->pid = -1;
ret = smb_register_sink(pdev, drvdata); if (ret) { diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h index 323f0ccb6878..561e42dc0870 100644 --- a/drivers/hwtracing/coresight/ultrasoc-smb.h +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h @@ -108,8 +108,6 @@ struct smb_data_buffer { * @miscdev: Specifics to handle "/dev/xyz.smb" entry. * @spinlock: Control data access to one at a time. * @reading: Synchronise user space access to SMB buffer. - * @pid: Process ID of the process being monitored by the - * session that is using this component. */ struct smb_drv_data { void __iomem *base; @@ -118,7 +116,6 @@ struct smb_drv_data { struct miscdevice miscdev; raw_spinlock_t spinlock; bool reading; - pid_t pid; };
#endif
-
James Clark