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 everywhere the same PID checking logic was copy pasted. Then the PIDs can be dropped from a few structs as they are now unused.
Signed-off-by: James Clark james.clark@linaro.org --- 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 (8): coresight: tmc-etr: Don't stop Perf cleanup for active sysfs reads coresight: configfs: Don't assume active until cscfg_mgr is set coresight: etm-perf: Flush workqueue before unloading module coresight: tmc-etr: Prevent per-thread events from sharing a sink coresight: tmc-etr: Use session ID for buffer ownership coresight: tmc-etf: Prevent per-thread events from sharing a sink coresight: etb10: Prevent per-thread events from sharing a sink coresight: ultrasoc-smb: Prevent per-thread events from sharing a sink
drivers/hwtracing/coresight/coresight-core.c | 28 +-- drivers/hwtracing/coresight/coresight-etb10.c | 33 ++-- drivers/hwtracing/coresight/coresight-etm-perf.c | 79 ++++++++- drivers/hwtracing/coresight/coresight-etm-perf.h | 15 ++ drivers/hwtracing/coresight/coresight-priv.h | 2 - drivers/hwtracing/coresight/coresight-syscfg.c | 6 +- drivers/hwtracing/coresight/coresight-tmc-core.c | 6 +- drivers/hwtracing/coresight/coresight-tmc-etf.c | 44 ++--- drivers/hwtracing/coresight/coresight-tmc-etr.c | 207 +++++++++++++---------- drivers/hwtracing/coresight/coresight-tmc.h | 30 ++-- drivers/hwtracing/coresight/coresight-trbe.c | 3 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 25 +-- drivers/hwtracing/coresight/ultrasoc-smb.h | 6 +- include/linux/coresight.h | 5 +- 14 files changed, 294 insertions(+), 195 deletions(-) --- base-commit: 98495b5a4d77dd22e106f462b76e1093a55b29a7 change-id: 20260708-james-cs-multiple-per-threads-ed1d25ed1734
Best regards,
The linked fixes commit deliberately allows reads of an old sysfs buffer while in Perf mode because they are separate software buffers. However it didn't modify tmc_disable_etr_sink() to match this relaxation. The result is that when a Perf event ends while the sysfs buffer is being read, clean up will be skipped.
Fix it by ignoring the sysfs_reading flag unless the active session is a sysfs one.
When sysfs and Perf share the same memory in ETR_MODE_RESRV mode, a new Perf session needs to overwrite an old inactive sysfs session by zeroing len. This avoids sysfs from reading stale data because it has a separate set of offsets in its etr_buf struct, even if that's backed by the same memory as the Perf one.
Reported-by: sashiko-bot sashiko-bot@kernel.org Fixes: cad5f8d399bb ("coresight: tmc-etr: Relax collection of trace from sysfs mode") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-tmc-etf.c | 16 ++++---- drivers/hwtracing/coresight/coresight-tmc-etr.c | 50 +++++++++++++++++++++---- drivers/hwtracing/coresight/coresight-tmc.h | 7 +++- 3 files changed, 55 insertions(+), 18 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 8882b1c4cdc0..3836063031d7 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -198,7 +198,7 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev) raw_spin_lock_irqsave(&drvdata->spinlock, flags); }
- if (drvdata->reading) { + if (drvdata->sysfs_reading) { ret = -EBUSY; goto out; } @@ -259,7 +259,7 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, raw_spin_lock_irqsave(&drvdata->spinlock, flags); do { ret = -EINVAL; - if (drvdata->reading) + if (drvdata->sysfs_reading) break; /* * No need to continue if the ETB/ETF is already operated @@ -337,7 +337,7 @@ static int tmc_disable_etf_sink(struct coresight_device *csdev)
raw_spin_lock_irqsave(&drvdata->spinlock, flags);
- if (drvdata->reading) { + if (drvdata->sysfs_reading) { raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); return -EBUSY; } @@ -371,7 +371,7 @@ static int tmc_enable_etf_link(struct coresight_device *csdev, bool first_enable = false;
raw_spin_lock_irqsave(&drvdata->spinlock, flags); - if (drvdata->reading) { + if (drvdata->sysfs_reading) { raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); return -EBUSY; } @@ -401,7 +401,7 @@ static void tmc_disable_etf_link(struct coresight_device *csdev, bool last_disable = false;
raw_spin_lock_irqsave(&drvdata->spinlock, flags); - if (drvdata->reading) { + if (drvdata->sysfs_reading) { raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); return; } @@ -718,7 +718,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
raw_spin_lock_irqsave(&drvdata->spinlock, flags);
- if (drvdata->reading) { + if (drvdata->sysfs_reading) { ret = -EBUSY; goto out; } @@ -746,7 +746,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata) __tmc_etb_disable_hw(drvdata); }
- drvdata->reading = true; + drvdata->sysfs_reading = true; out: raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -797,7 +797,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata) drvdata->buf = NULL; }
- drvdata->reading = false; + drvdata->sysfs_reading = false; raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
/* diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 361a433e6f0c..be0bbe036d02 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1151,7 +1151,7 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, * starts at anywhere in the buffer, depending on the RRP, we adjust the * @len returned to handle buffer wrapping around. * - * We are protected here by drvdata->reading != 0, which ensures the + * We are protected here by drvdata->sysfs_reading != 0, which ensures the * sysfs_buf stays alive. */ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, @@ -1268,7 +1268,7 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev) raw_spin_lock_irqsave(&drvdata->spinlock, flags); }
- if (drvdata->reading || coresight_get_mode(csdev) == CS_MODE_PERF) { + if (drvdata->sysfs_reading || coresight_get_mode(csdev) == CS_MODE_PERF) { ret = -EBUSY; goto out; } @@ -1732,6 +1732,16 @@ tmc_update_etr_buffer(struct coresight_device *csdev, return size; }
+static bool tmc_perf_sysfs_shared(struct tmc_drvdata *drvdata, + struct etr_buf *perf_buf) +{ + /* In ETR_MODE_RESRV mode, sysfs and Perf share the same memory. */ + return perf_buf && + drvdata->sysfs_buf && + drvdata->sysfs_buf->mode == ETR_MODE_RESRV && + perf_buf->mode == ETR_MODE_RESRV; +} + static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, struct coresight_path *path) { @@ -1772,6 +1782,18 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, goto unlock_out; }
+ /* + * Don't use if it's shared and being read by sysfs. Sysfs may only + * start reading (the cleared zero length buffer) after the first + * tmc_enable_etr_sink_perf(), which changes the result of this check, + * so it should only be done once. + */ + if ((drvdata->sysfs_reading && + tmc_perf_sysfs_shared(drvdata, etr_perf->etr_buf))) { + rc = -EBUSY; + goto unlock_out; + } + rc = tmc_etr_enable_hw(drvdata, etr_perf->etr_buf); if (!rc) { /* Associate with monitored process. */ @@ -1779,6 +1801,10 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, coresight_set_mode(csdev, CS_MODE_PERF); drvdata->perf_buf = etr_perf->etr_buf; csdev->refcnt++; + + /* A new Perf session clears an old sysfs one if the buffer is shared */ + if (tmc_perf_sysfs_shared(drvdata, etr_perf->etr_buf)) + drvdata->sysfs_buf->len = 0; }
unlock_out: @@ -1807,7 +1833,13 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
raw_spin_lock_irqsave(&drvdata->spinlock, flags);
- if (drvdata->reading) { + /* + * In SYSFS mode an active read is responsible for disabling and + * enabling HW. Otherwise in Perf mode, an old inactive sysfs session + * may be read which Perf should ignore. + */ + if (drvdata->sysfs_reading && + coresight_get_mode(csdev) == CS_MODE_SYSFS) { raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); return -EBUSY; } @@ -1928,14 +1960,16 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) return -EINVAL;
raw_spin_lock_irqsave(&drvdata->spinlock, flags); - if (drvdata->reading) { + if (drvdata->sysfs_reading) { ret = -EBUSY; goto out; }
/* - * We can safely allow reads even if the ETR is operating in PERF mode, - * since the sysfs session is captured in mode specific data. + * We can safely allow reads even if the ETR is operating in PERF mode + * since sysfs has it's own buffer. For ETR_MODE_RESRV the buffers are + * shared but Perf discards sysfs data before starting a session to + * avoid corruption. * If drvdata::sysfs_data is NULL the trace data has been read already. */ if (!drvdata->sysfs_buf) { @@ -1947,7 +1981,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS) __tmc_etr_disable_hw(drvdata);
- drvdata->reading = true; + drvdata->sysfs_reading = true; out: raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -1982,7 +2016,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) drvdata->sysfs_buf = NULL; }
- drvdata->reading = false; + drvdata->sysfs_reading = false; raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
/* Free allocated memory out side of the spinlock */ diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 319a354ede9f..dc1a57ab8011 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -221,7 +221,10 @@ struct tmc_resrv_buf { * @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 + * @sysfs_reading: Sysfs mode buffer is being read through "/dev/xyz.tmc" entry. + * Note: ETR has a separate software buffer for the two modes so + * the device can still be read while in Perf mode if there is a + * previous inactive sysfs session. * @stop_on_flush: Stop on flush trigger user configuration. * @buf: Snapshot of the trace data for ETF/ETB. * @etr_buf: details of buffer used in TMC-ETR @@ -255,7 +258,7 @@ struct tmc_drvdata { struct miscdevice crashdev; raw_spinlock_t spinlock; pid_t pid; - bool reading; + bool sysfs_reading; bool stop_on_flush; union { char *buf; /* TMC ETB */
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.
Reported-by: sashiko-bot sashiko-bot@kernel.org 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 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c index 2bfdd7b45e49..475e8fefa100 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.c +++ b/drivers/hwtracing/coresight/coresight-syscfg.c @@ -581,7 +581,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_mgr || cscfg_mgr->load_state != CSCFG_NONE) { mutex_unlock(&cscfg_mutex); return -EBUSY; } @@ -662,7 +662,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_mgr || cscfg_mgr->load_state != CSCFG_NONE) { mutex_unlock(&cscfg_mutex); return -EBUSY; } @@ -902,7 +902,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_mgr || cscfg_mgr->load_state == CSCFG_UNLOAD) return -EBUSY;
list_for_each_entry(config_desc, &cscfg_mgr->config_desc_list, item) {
The work queue is never flushed on module unload, so pending work items can run after coresight-config is cleaned up or the .text section is unloaded. The global workqueue also can't be flushed manually, so change it to a local workqueue and flush it on exit.
The init function needs to be rearranged to fix a similar problem. The Perf PMU can be used as soon as registration succeeds, so move it as late as possible so that the only failure that can follow is coresight- config. Failing to register coresight-config means no flush dependency between coresight-config and the Perf PMU will exist, so it doesn't need to be done.
Assisted-by: Codex:GPT-5.6-Sol Reported-by: sashiko-bot sashiko-bot@kernel.org 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 | 28 ++++++++++++++---------- drivers/hwtracing/coresight/coresight-etm-perf.c | 22 +++++++++++++++---- drivers/hwtracing/coresight/coresight-etm-perf.h | 1 + 3 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index f7b1308a759c..5e653b08a83d 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -2094,31 +2094,31 @@ static int __init coresight_init(void) if (ret) return ret;
- ret = etm_perf_init(); - if (ret) - goto exit_bus_unregister; - /* Register function to be called for panic */ ret = atomic_notifier_chain_register(&panic_notifier_list, &coresight_notifier); if (ret) - goto exit_perf; + goto exit_bus_unregister;
- /* initialise the coresight syscfg API */ - ret = cscfg_init(); + ret = coresight_pm_setup(); if (ret) goto exit_notifier;
- ret = coresight_pm_setup(); + ret = etm_perf_init(); + if (ret) + goto exit_pm; + + /* initialise the coresight syscfg API */ + ret = cscfg_init(); if (!ret) return 0;
- cscfg_exit(); + etm_perf_exit(); +exit_pm: + coresight_pm_cleanup(); exit_notifier: atomic_notifier_chain_unregister(&panic_notifier_list, &coresight_notifier); -exit_perf: - etm_perf_exit(); exit_bus_unregister: bus_unregister(&coresight_bustype); return ret; @@ -2127,6 +2127,12 @@ static int __init coresight_init(void) static void __exit coresight_exit(void) { coresight_pm_cleanup(); + /* + * Flush Perf workqueue before the rest of Coresight is torn down + * because work items touch coresight-config and also require the .text + * to remain loaded. + */ + etm_perf_flush_workqueue(); cscfg_exit(); atomic_notifier_chain_unregister(&panic_notifier_list, &coresight_notifier); diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 09b21a711a87..7fb5c3d18bd5 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -27,6 +27,7 @@ #include "coresight-trace-id.h"
static struct pmu etm_pmu; +static struct workqueue_struct *etm_free_wq; static bool etm_perf_up;
/* @@ -292,7 +293,7 @@ static void etm_free_aux(void *data) { struct etm_event_data *event_data = data;
- schedule_work(&event_data->work); + queue_work(etm_free_wq, &event_data->work); }
/* @@ -1034,6 +1035,10 @@ int __init etm_perf_init(void) { int ret;
+ etm_free_wq = alloc_workqueue("coresight_etm_free", WQ_UNBOUND, 0); + if (!etm_free_wq) + return -ENOMEM; + etm_pmu.capabilities = (PERF_PMU_CAP_EXCLUSIVE | PERF_PMU_CAP_ITRACE | PERF_PMU_CAP_AUX_PAUSE); @@ -1054,13 +1059,22 @@ int __init etm_perf_init(void) etm_pmu.module = THIS_MODULE;
ret = perf_pmu_register(&etm_pmu, CORESIGHT_ETM_PMU_NAME, -1); - if (ret == 0) - etm_perf_up = true; + if (ret) { + destroy_workqueue(etm_free_wq); + return ret; + }
- return ret; + etm_perf_up = true; + return 0; }
void etm_perf_exit(void) { perf_pmu_unregister(&etm_pmu); + destroy_workqueue(etm_free_wq); +} + +void etm_perf_flush_workqueue(void) +{ + flush_workqueue(etm_free_wq); } diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 24d929428633..86e259cc1adf 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -116,5 +116,6 @@ int etm_perf_add_symlink_cscfg(struct device *dev, void etm_perf_del_symlink_cscfg(struct cscfg_config_desc *config_desc); int __init etm_perf_init(void); void etm_perf_exit(void); +void etm_perf_flush_workqueue(void);
#endif
Sinks are potentially shared between multiple ETMs, and even with Coresight being an exclusive PMU, multiple PERF_ATTACH_TASK events in parallel are supported. If both of the events are owned by the same PID then the sink becomes shared resulting in trace from the wrong thread appearing in a per-thread buffer and WARNs in the driver being hit:
$ perf test -w named_threads 2 10 & $ perf record -e cs_etm//u --per-thread --pid 984
WARNING: drivers/hwtracing/coresight/coresight-tmc-etr.c:1654 at tmc_update_etr_buffer+0x328/0x348 [coresight_tmc], CPU#1: perf/686 [...] Call trace: tmc_update_etr_buffer+0x328/0x348 [coresight_tmc] (P) etm_event_stop+0x1c8/0x260 [coresight] etm_event_del+0x20/0x38 [coresight] event_sched_out+0xc4/0x1c8 group_sched_out+0x5c/0x170 __pmu_ctx_sched_out+0xe8/0x148 ctx_sched_out+0x12c/0x178 perf_event_exit_task_context+0xb0/0x3a8 perf_event_exit_task+0xa4/0x138 do_exit+0x1cc/0x808 do_group_exit+0x7c/0xb0 get_signal+0x628/0x678 arch_do_signal_or_restart+0x98/0x1b28 exit_to_user_mode_loop+0x90/0x188 el0_svc+0x1cc/0x260 el0t_64_sync_handler+0x78/0x130 el0t_64_sync+0x198/0x1a0
Fix it by returning -EBUSY if an incompatible event tries to use the same sink. Users will only be able to use per-thread mode reliably with multiple threads on a system that has a dedicated sink per ETM. This limitation hasn't changed since this fix, it just makes the behavior more consistent.
The identity and compatibility of a session are determined by the owning process, the target process, and the inheritance settings together.
"Owning process" is a reference to the TGID of the caller of etm_setup_aux(), so any thread in that group can access the events. It has to be pinned at this point rather than checked at runtime via event->owner so that we can handle scenarios like event FDs being passed to child threads and original owners exiting.
The "target process" is determined by taking a reference to event->hw.target in etm_setup_aux(), which is always called on the root event, so is unaffected by inheritance. Perf holds a reference to that task struct as long as the event exists, so it can be safely taken even if the task isn't live by the time mmap is called.
The remaining inheritance settings are used to determine if a PERF_ATTACH_TASK event tracks the same child threads as another event with the same target on a different CPU. If not, then sinks can't be shared to avoid unexpected trace appearing from a child when inherit=false on one but not another.
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-etm-perf.c | 53 ++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-etm-perf.h | 14 +++++++ drivers/hwtracing/coresight/coresight-tmc-etr.c | 20 +++------ drivers/hwtracing/coresight/coresight-tmc.h | 3 ++ include/linux/coresight.h | 1 + 5 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 7fb5c3d18bd5..28cf319a39da 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -251,6 +251,9 @@ static void free_event_data(struct work_struct *work) }
free_percpu(event_data->path); + put_pid(event_data->session_id.owner); + if (event_data->session_id.target) + put_task_struct(event_data->session_id.target); kfree(event_data); }
@@ -452,6 +455,16 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, event_data->cfg_hash = cfg_hash; }
+ /* + * Save an identity that determines if sessions are equivalent enough + * for shared sinks to receive trace from other events. + */ + event_data->session_id.owner = get_task_pid(current, PIDTYPE_TGID); + if (event->attach_state & PERF_ATTACH_TASK) { + event_data->session_id.target = get_task_struct(event->hw.target); + event_data->session_id.inherit = event->attr.inherit; + event_data->session_id.inherit_thread = event->attr.inherit_thread; + } mask = &event_data->mask;
/* @@ -1078,3 +1091,43 @@ void etm_perf_flush_workqueue(void) { flush_workqueue(etm_free_wq); } + +bool etm_perf_compare_session(struct etm_session_id *a, + struct etm_session_id *b) +{ + return a->owner == b->owner && + a->target == b->target && + a->inherit == b->inherit && + a->inherit_thread == b->inherit_thread; +} +EXPORT_SYMBOL_GPL(etm_perf_compare_session); + +/* + * Returns true if a sink that's potentially connected to multiple ETMs can be + * used in parallel with another event. + */ +bool etm_perf_sink_can_share(struct coresight_device *csdev, + struct etm_session_id *owner, + struct perf_output_handle *new) +{ + struct etm_event_data *new_event_data = perf_get_aux(new); + struct etm_session_id *new_id = &new_event_data->session_id; + + /* First user can always use the sink */ + if (csdev->refcnt == 0) + return true; + + /* + * Only allow sharing if both events have the same owner and are + * expected to follow the exact same set of processes. For CPU wide + * events, both new and current 'target's are NULL so sharing between + * all events is allowed. + * + * When 'target' mismatches, trace from two different PERF_ATTACH_TASK + * events is prevented from appearing in a buffer targeting another + * process. Inherit flags change which set of processes are followed, + * even for events with the same 'target', so they must also be checked. + */ + return etm_perf_compare_session(owner, new_id); +} +EXPORT_SYMBOL_GPL(etm_perf_sink_can_share); diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 86e259cc1adf..87b3ba72d07c 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -82,6 +82,13 @@ struct etm_filters { bool ssstatus; };
+struct etm_session_id { + struct pid *owner; + struct task_struct *target; + bool inherit; + bool inherit_thread; +}; + /** * struct etm_event_data - Coresight specifics associated to an event * @work: Handle to free allocated memory outside IRQ context. @@ -90,6 +97,7 @@ struct etm_filters { * @snk_config: The sink configuration. * @cfg_hash: The hash id of any coresight config selected. * @path: An array of path, each slot for one CPU. + * @session_id: Reference to the session that called setup_aux() */ struct etm_event_data { struct work_struct work; @@ -98,6 +106,7 @@ struct etm_event_data { void *snk_config; u32 cfg_hash; struct coresight_path * __percpu *path; + struct etm_session_id session_id; };
int etm_perf_symlink(struct coresight_device *csdev, bool link); @@ -117,5 +126,10 @@ void etm_perf_del_symlink_cscfg(struct cscfg_config_desc *config_desc); int __init etm_perf_init(void); void etm_perf_exit(void); void etm_perf_flush_workqueue(void); +bool etm_perf_compare_session(struct etm_session_id *a, + struct etm_session_id *b); +bool etm_perf_sink_can_share(struct coresight_device *csdev, + struct etm_session_id *owner, + struct perf_output_handle *new);
#endif diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index be0bbe036d02..be5ed04a554f 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1746,10 +1746,10 @@ 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; + struct etm_event_data *event_data = perf_get_aux(handle); struct etr_perf_buffer *etr_perf = etm_perf_sink_config(handle);
raw_spin_lock_irqsave(&drvdata->spinlock, flags); @@ -1764,20 +1764,14 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, 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) { + if (!etm_perf_sink_can_share(csdev, &drvdata->perf_session, handle)) { 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; } @@ -1796,10 +1790,9 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev,
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; + drvdata->perf_session = event_data->session_id; csdev->refcnt++;
/* A new Perf session clears an old sysfs one if the buffer is shared */ @@ -1853,11 +1846,10 @@ 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; + drvdata->perf_session = (struct etm_session_id) {};
raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index dc1a57ab8011..9b153b7910fb 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -13,6 +13,7 @@ #include <linux/mutex.h> #include <linux/refcount.h> #include <linux/crc32.h> +#include "coresight-etm-perf.h"
#define TMC_RSZ 0x004 #define TMC_STS 0x00c @@ -248,6 +249,7 @@ struct tmc_resrv_buf { * (after crash) by default. * @crash_mdata: Reserved memory for storing tmc crash metadata. * Used by ETR/ETF. + * @perf_session: Session identity of the Perf event currently using this sink. */ struct tmc_drvdata { struct clk *atclk; @@ -278,6 +280,7 @@ struct tmc_drvdata { struct etr_buf *perf_buf; struct tmc_resrv_buf resrv_buf; struct tmc_resrv_buf crash_mdata; + struct etm_session_id perf_session; };
struct etr_buf_operations { diff --git a/include/linux/coresight.h b/include/linux/coresight.h index add0579cad88..1c06a9800cfe 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -715,4 +715,5 @@ int coresight_etm_get_trace_id(struct coresight_device *csdev, enum cs_mode mode struct coresight_device *sink); int coresight_get_enable_clocks(struct device *dev, struct clk **pclk, struct clk **atclk); + #endif /* _LINUX_COREISGHT_H */
Comparing numeric PIDs for ownership no longer matches the sharing rules in coresight_sink_can_share(), change it to compare the session identity instead.
Now that we're not using a numeric PID for ownership we can't use the IDR map. Change it to a simple linear realloc'd array, it's unlikely the performance difference is measurable, systems wouldn't have more than a handful of shared sources for one sink, and this is only accessed once at setup.
Assisted-by: Codex:GPT-5.6-Sol 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-etb10.c | 3 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 4 +- drivers/hwtracing/coresight/coresight-tmc-core.c | 4 +- drivers/hwtracing/coresight/coresight-tmc-etf.c | 5 +- drivers/hwtracing/coresight/coresight-tmc-etr.c | 139 ++++++++++++----------- drivers/hwtracing/coresight/coresight-tmc.h | 16 ++- drivers/hwtracing/coresight/coresight-trbe.c | 3 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 3 +- include/linux/coresight.h | 4 +- 9 files changed, 100 insertions(+), 81 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index a827f76b8144..6a4a1edf46ba 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -371,7 +371,8 @@ static int etb_disable(struct coresight_device *csdev) }
static void *etb_alloc_buffer(struct coresight_device *csdev, - struct perf_event *event, void **pages, + struct perf_event *event, + struct etm_session_id *owner, void **pages, int nr_pages, bool overwrite) { int node; diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 28cf319a39da..652beee2edbd 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -517,7 +517,9 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, * sinks. */ event_data->snk_config = - sink_ops(sink)->alloc_buffer(sink, event, pages, + sink_ops(sink)->alloc_buffer(sink, event, + &event_data->session_id, + pages, nr_pages, overwrite); if (!event_data->snk_config) goto err; diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index c89fe996af23..873868718b14 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_bufs_mutex); dev_list = "tmc_etr"; break; case TMC_CONFIG_TYPE_ETF: diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 3836063031d7..d90090b846ab 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -419,8 +419,9 @@ static void tmc_disable_etf_link(struct coresight_device *csdev, }
static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, - struct perf_event *event, void **pages, - int nr_pages, bool overwrite) + struct perf_event *event, + struct etm_session_id *owner, + void **pages, int nr_pages, bool overwrite) { int node; struct cs_buffers *buf; diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index be5ed04a554f..324d94e6d7e1 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> @@ -37,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. @@ -46,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; @@ -1396,16 +1392,16 @@ 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_etr_buf_cpu_wide(struct tmc_drvdata *drvdata, + struct perf_event *event, + struct etm_session_id *owner, + int nr_pages, void **pages, + bool snapshot) { - int ret; - pid_t pid = task_pid_nr(event->owner); - struct etr_buf *etr_buf; + struct etr_buf_mapping *perf_bufs; + struct etr_buf *etr_buf, *existing; + unsigned int i;
-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,69 +1420,75 @@ 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); - if (etr_buf) { - refcount_inc(&etr_buf->refcount); - mutex_unlock(&drvdata->idr_mutex); - return etr_buf; + mutex_lock(&drvdata->perf_bufs_mutex); + for (i = 0; i < drvdata->nr_perf_bufs; i++) { + if (etm_perf_compare_session(&drvdata->perf_bufs[i].owner, owner)) { + etr_buf = drvdata->perf_bufs[i].buf; + refcount_inc(&etr_buf->refcount); + mutex_unlock(&drvdata->perf_bufs_mutex); + return etr_buf; + } }
/* If we made it here no buffer has been allocated, do so now. */ - mutex_unlock(&drvdata->idr_mutex); + mutex_unlock(&drvdata->perf_bufs_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; + mutex_lock(&drvdata->perf_bufs_mutex); + for (i = 0; i < drvdata->nr_perf_bufs; i++) { + if (etm_perf_compare_session(&drvdata->perf_bufs[i].owner, owner)) { + existing = drvdata->perf_bufs[i].buf; + refcount_inc(&existing->refcount); + mutex_unlock(&drvdata->perf_bufs_mutex); + tmc_free_etr_buf(etr_buf); + return existing; + } }
- /* The IDR can't allocate room for a new session, abandon ship. */ - if (ret == -ENOMEM) { + perf_bufs = krealloc_array(drvdata->perf_bufs, + drvdata->nr_perf_bufs + 1, + sizeof(*perf_bufs), GFP_KERNEL); + if (!perf_bufs) { + mutex_unlock(&drvdata->perf_bufs_mutex); tmc_free_etr_buf(etr_buf); - return ERR_PTR(ret); + return ERR_PTR(-ENOMEM); }
+ drvdata->perf_bufs = perf_bufs; + perf_bufs[drvdata->nr_perf_bufs].owner = *owner; + perf_bufs[drvdata->nr_perf_bufs].buf = etr_buf; + drvdata->nr_perf_bufs++; + mutex_unlock(&drvdata->perf_bufs_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) +static struct etr_buf *get_perf_etr_buf(struct tmc_drvdata *drvdata, + struct perf_event *event, + struct etm_session_id *owner, + int nr_pages, void **pages, + bool snapshot) { /* - * In per-thread mode the etr_buf isn't shared, so just go ahead - * with memory allocation. + * Events without a CPU must have inherit=false so always target a + * single process. etm_perf_sink_can_share() prevents other single + * process events from sharing sinks, so take a shortcut and go ahead + * with memory allocation without using shared drvdata->perf_bufs. */ - 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); + return alloc_etr_buf(drvdata, event, nr_pages, pages, snapshot);
- return get_perf_etr_buf_cpu_wide(drvdata, event, nr_pages, + return get_perf_etr_buf_cpu_wide(drvdata, event, owner, nr_pages, pages, snapshot); }
static struct etr_perf_buffer * tmc_etr_setup_perf_buf(struct tmc_drvdata *drvdata, struct perf_event *event, - int nr_pages, void **pages, bool snapshot) + struct etm_session_id *owner, int nr_pages, void **pages, + bool snapshot) { int node; struct etr_buf *etr_buf; @@ -1498,7 +1500,8 @@ tmc_etr_setup_perf_buf(struct tmc_drvdata *drvdata, struct perf_event *event, if (!etr_perf) return ERR_PTR(-ENOMEM);
- etr_buf = get_perf_etr_buf(drvdata, event, nr_pages, pages, snapshot); + etr_buf = get_perf_etr_buf(drvdata, event, owner, nr_pages, pages, + snapshot); if (!IS_ERR(etr_buf)) goto done;
@@ -1508,7 +1511,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 have access to the buffer array in tmc_free_etr_buffer(). */ etr_perf->drvdata = drvdata; etr_perf->etr_buf = etr_buf; @@ -1516,22 +1519,21 @@ tmc_etr_setup_perf_buf(struct tmc_drvdata *drvdata, struct perf_event *event, return etr_perf; }
- static void *tmc_alloc_etr_buffer(struct coresight_device *csdev, - struct perf_event *event, void **pages, + struct perf_event *event, + struct etm_session_id *owner, void **pages, int nr_pages, bool snapshot) { struct etr_perf_buffer *etr_perf; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
- etr_perf = tmc_etr_setup_perf_buf(drvdata, event, + etr_perf = tmc_etr_setup_perf_buf(drvdata, event, owner, nr_pages, pages, snapshot); if (IS_ERR(etr_perf)) { dev_dbg(&csdev->dev, "Unable to allocate ETR buffer\n"); return NULL; }
- etr_perf->pid = task_pid_nr(event->owner); etr_perf->snapshot = snapshot; etr_perf->nr_pages = nr_pages; etr_perf->pages = pages; @@ -1543,28 +1545,33 @@ 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; + unsigned int i;
if (!etr_buf) goto free_etr_perf_buffer;
- mutex_lock(&drvdata->idr_mutex); + mutex_lock(&drvdata->perf_bufs_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_bufs_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); + for (i = 0; i < drvdata->nr_perf_bufs; i++) { + if (drvdata->perf_bufs[i].buf != etr_buf) + continue;
- /* - * 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; + drvdata->nr_perf_bufs--; + drvdata->perf_bufs[i] = + drvdata->perf_bufs[drvdata->nr_perf_bufs]; + if (!drvdata->nr_perf_bufs) { + kfree(drvdata->perf_bufs); + drvdata->perf_bufs = NULL; + } + break; + } + mutex_unlock(&drvdata->perf_bufs_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 9b153b7910fb..0125f2ad78ed 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> @@ -192,6 +191,11 @@ struct etr_buf { void *private; };
+struct etr_buf_mapping { + struct etm_session_id owner; + struct etr_buf *buf; +}; + /** * @paddr : Start address of reserved memory region. * @vaddr : Corresponding CPU virtual address. @@ -239,8 +243,9 @@ 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. + * @perf_bufs: CPU-wide Perf buffers and their owners. + * @nr_perf_bufs: Number of entries in @perf_bufs. + * @perf_bufs_mutex: Access serialisation for @perf_bufs. * @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 @@ -274,8 +279,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_mapping *perf_bufs; + unsigned int nr_perf_bufs; + struct mutex perf_bufs_mutex; struct etr_buf *sysfs_buf; struct etr_buf *perf_buf; struct tmc_resrv_buf resrv_buf; diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index c7cbca45f2de..81841dc95e0c 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -746,7 +746,8 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, }
static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, - struct perf_event *event, void **pages, + struct perf_event *event, + struct etm_session_id *owner, void **pages, int nr_pages, bool snapshot) { struct trbe_buf *buf; diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c index 20a950b9dd4f..f66ec53070d7 100644 --- a/drivers/hwtracing/coresight/ultrasoc-smb.c +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c @@ -302,7 +302,8 @@ static int smb_disable(struct coresight_device *csdev) }
static void *smb_alloc_buffer(struct coresight_device *csdev, - struct perf_event *event, void **pages, + struct perf_event *event, + struct etm_session_id *owner, void **pages, int nr_pages, bool overwrite) { struct cs_buffers *buf; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 1c06a9800cfe..9e29f2cbf557 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -37,6 +37,7 @@ #define CORESIGHT_UNLOCK 0xc5acce55
extern const struct bus_type coresight_bustype; +struct etm_session_id;
enum coresight_dev_type { CORESIGHT_DEV_TYPE_SINK, @@ -371,7 +372,8 @@ struct coresight_ops_sink { struct coresight_path *path); int (*disable)(struct coresight_device *csdev); void *(*alloc_buffer)(struct coresight_device *csdev, - struct perf_event *event, void **pages, + struct perf_event *event, + struct etm_session_id *owner, void **pages, int nr_pages, bool overwrite); void (*free_buffer)(void *config); unsigned long (*update_buffer)(struct coresight_device *csdev,
Like the previous commit to ETR, ETF sinks shouldn't share per-thread events. Fix it by returning -EBUSY if a second per-thread event tries to use the same sink.
Fixes: 880af782c6e8 ("coresight: tmc-etf: Add support for CPU-wide trace scenarios") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-tmc-core.c | 2 -- drivers/hwtracing/coresight/coresight-tmc-etf.c | 23 +++++++---------------- drivers/hwtracing/coresight/coresight-tmc.h | 4 ---- 3 files changed, 7 insertions(+), 22 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 873868718b14..2c2f2f312e8e 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 d90090b846ab..c451f1da3cdd 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -250,11 +250,10 @@ 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); + struct etm_event_data *event_data = perf_get_aux(handle);
raw_spin_lock_irqsave(&drvdata->spinlock, flags); do { @@ -270,10 +269,8 @@ 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) { + if (!etm_perf_sink_can_share(csdev, &drvdata->perf_session, + handle)) { ret = -EBUSY; break; } @@ -282,19 +279,15 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, 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; + drvdata->perf_session = event_data->session_id; coresight_set_mode(csdev, CS_MODE_PERF); csdev->refcnt++; } @@ -351,9 +344,8 @@ 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); + drvdata->perf_session = (struct etm_session_id) {};
raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -433,7 +425,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 0125f2ad78ed..99601ac9179d 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -223,9 +223,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. * @sysfs_reading: Sysfs mode buffer is being read through "/dev/xyz.tmc" entry. * Note: ETR has a separate software buffer for the two modes so * the device can still be read while in Perf mode if there is a @@ -264,7 +261,6 @@ struct tmc_drvdata { struct miscdevice miscdev; struct miscdevice crashdev; raw_spinlock_t spinlock; - pid_t pid; bool sysfs_reading; bool stop_on_flush; union {
Like the previous commit to ETR, ETB10 sinks shouldn't share per-thread events. Fix it by returning -EBUSY if a second per-thread event tries to use the same sink.
Fixes: 75d7dbd38824 ("coresight: etb10: Add support for CPU-wide trace scenarios") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etb10.c | 30 +++++++-------------------- 1 file changed, 8 insertions(+), 22 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 6a4a1edf46ba..ee0f20c30153 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -71,8 +71,7 @@ * @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. + * @perf_session: Session identity of the Perf event currently using this sink. * @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 +83,7 @@ struct etb_drvdata { struct miscdevice miscdev; raw_spinlock_t spinlock; atomic_t reading; - pid_t pid; + struct etm_session_id perf_session; u8 *buf; u32 buffer_depth; u32 trigger_cntr; @@ -168,11 +167,10 @@ 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); + struct etm_event_data *event_data = perf_get_aux(handle);
raw_spin_lock_irqsave(&drvdata->spinlock, flags);
@@ -182,19 +180,13 @@ 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) { + if (!etm_perf_sink_can_share(csdev, &drvdata->perf_session, handle)) { 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 +202,7 @@ 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; + drvdata->perf_session = event_data->session_id; coresight_set_mode(drvdata->csdev, CS_MODE_PERF); csdev->refcnt++; } @@ -361,8 +352,7 @@ 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; + drvdata->perf_session = (struct etm_session_id) {}; coresight_set_mode(csdev, CS_MODE_DISABLED); raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -384,7 +374,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; @@ -755,9 +744,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);
Like the previous commit to ETR, SMB sinks shouldn't share per-thread events. Fix it by returning -EBUSY if a second per-thread event tries to use the same sink.
Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-priv.h | 2 -- drivers/hwtracing/coresight/ultrasoc-smb.c | 22 ++++++---------------- drivers/hwtracing/coresight/ultrasoc-smb.h | 6 +++--- 3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index dddac946659f..492f050eb698 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -94,7 +94,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 @@ -103,7 +102,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 f66ec53070d7..ba4a7a8f6f96 100644 --- a/drivers/hwtracing/coresight/ultrasoc-smb.c +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c @@ -216,22 +216,15 @@ static int smb_enable_perf(struct coresight_device *csdev, { 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; + struct etm_event_data *event_data = perf_get_aux(handle);
- 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) + /* Do not proceed if this device is associated with another session */ + if (!etm_perf_sink_can_share(csdev, &drvdata->perf_session, handle)) return -EBUSY;
- if (drvdata->pid == -1) { + if (!csdev->refcnt) { smb_enable_hw(drvdata); - drvdata->pid = pid; + drvdata->perf_session = event_data->session_id; coresight_set_mode(csdev, CS_MODE_PERF); }
@@ -293,8 +286,7 @@ static int smb_disable(struct coresight_device *csdev)
smb_disable_hw(drvdata);
- /* Dissociate from the target process. */ - drvdata->pid = -1; + drvdata->perf_session = (struct etm_session_id) {}; coresight_set_mode(csdev, CS_MODE_DISABLED); dev_dbg(&csdev->dev, "Ultrasoc SMB disabled\n");
@@ -317,7 +309,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; } @@ -565,7 +556,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..491c2716b835 100644 --- a/drivers/hwtracing/coresight/ultrasoc-smb.h +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h @@ -10,6 +10,7 @@ #include <linux/bitfield.h> #include <linux/miscdevice.h> #include <linux/spinlock.h> +#include "coresight-etm-perf.h"
/* Offset of SMB global registers */ #define SMB_GLB_CFG_REG 0x00 @@ -108,8 +109,7 @@ 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. + * @perf_session: Session identity of the Perf event currently using this sink. */ struct smb_drv_data { void __iomem *base; @@ -118,7 +118,7 @@ struct smb_drv_data { struct miscdevice miscdev; raw_spinlock_t spinlock; bool reading; - pid_t pid; + struct etm_session_id perf_session; };
#endif
-
James Clark