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 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 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 (4): coresight: tmc-etr: Prevent per-thread events from sharing a sink 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 | 31 ++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-etb10.c | 30 ++++++----------------- drivers/hwtracing/coresight/coresight-priv.h | 2 -- drivers/hwtracing/coresight/coresight-tmc-core.c | 2 -- drivers/hwtracing/coresight/coresight-tmc-etf.c | 23 ++++++------------ drivers/hwtracing/coresight/coresight-tmc-etr.c | 20 +++++---------- drivers/hwtracing/coresight/coresight-tmc.h | 2 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 22 +++++------------ drivers/hwtracing/coresight/ultrasoc-smb.h | 5 ++-- include/linux/coresight.h | 3 +++ 10 files changed, 64 insertions(+), 76 deletions(-) --- base-commit: 98495b5a4d77dd22e106f462b76e1093a55b29a7 change-id: 20260708-james-cs-multiple-per-threads-ed1d25ed1734
Best regards,
Sinks are potentially shared between multiple ETMs, and even with Coresight being an exclusive PMU, multiple per-thread 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 a second per-thread 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.
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 | 31 +++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc-etr.c | 20 +++++----------- drivers/hwtracing/coresight/coresight-tmc.h | 1 + include/linux/coresight.h | 3 +++ 4 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index f7b1308a759c..9410af0de49c 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -2246,6 +2246,37 @@ int coresight_get_enable_clocks(struct device *dev, struct clk **pclk, } EXPORT_SYMBOL_GPL(coresight_get_enable_clocks);
+/* + * Returns true if a sink that's potentially connected to multiple ETMs can be + * used in parallel with another event. + */ +bool coresight_sink_can_share(struct perf_event *existing, + struct perf_event *new) +{ + /* + * Inheritance in per-CPU mode means an event might not have an owner, + * but the parent will. + */ + struct perf_event *existing_root = existing->parent ?: existing; + struct perf_event *new_root = new->parent ?: new; + + /* + * In per-thread mode only one event can use a sink, otherwise a sink + * shared by multiple ETMs could have trace from two different threads + * in it from two different events. + */ + if (existing->cpu == -1) + return existing == new; + + /* + * In per-CPU mode allow the sink to be shared across events, userspace + * expects data from multiple CPUs in a shared sink. The only + * restriction is the events must have the same owner. + */ + return existing_root->owner == new_root->owner; +} +EXPORT_SYMBOL_GPL(coresight_sink_can_share); + MODULE_LICENSE("GPL v2"); MODULE_AUTHOR("Pratik Patel pratikp@codeaurora.org"); MODULE_AUTHOR("Mathieu Poirier mathieu.poirier@linaro.org"); diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 361a433e6f0c..ea05fa3e7902 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1736,7 +1736,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; @@ -1754,30 +1753,24 @@ 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 (drvdata->event && + !coresight_sink_can_share(drvdata->event, handle->event)) { 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; + drvdata->event = handle->event; csdev->refcnt++; }
@@ -1821,11 +1814,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->event = 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 319a354ede9f..220fe45fca83 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -275,6 +275,7 @@ struct tmc_drvdata { struct etr_buf *perf_buf; struct tmc_resrv_buf resrv_buf; struct tmc_resrv_buf crash_mdata; + struct perf_event *event; };
struct etr_buf_operations { diff --git a/include/linux/coresight.h b/include/linux/coresight.h index add0579cad88..916ea657a194 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -715,4 +715,7 @@ 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); +bool coresight_sink_can_share(struct perf_event *existing, + struct perf_event *new); + #endif /* _LINUX_COREISGHT_H */
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 | 1 - 3 files changed, 7 insertions(+), 19 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index c89fe996af23..77bdf8892617 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -801,8 +801,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..4ed83f07eec8 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,10 +268,9 @@ 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 (drvdata->event && + !coresight_sink_can_share(drvdata->event, + handle->event)) { 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->event = handle->event; 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->event = NULL;
raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -432,7 +424,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 220fe45fca83..595d6a3b0767 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -254,7 +254,6 @@ struct tmc_drvdata { struct miscdevice miscdev; struct miscdevice crashdev; raw_spinlock_t spinlock; - pid_t pid; bool 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 a827f76b8144..6b5caad330d9 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. + * @event: Perf event using this sink if in Perf mode. * @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 perf_event *event; u8 *buf; u32 buffer_depth; u32 trigger_cntr; @@ -168,11 +167,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 +179,14 @@ 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 (drvdata->event && + !coresight_sink_can_share(drvdata->event, handle->event)) { 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->event = handle->event; 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->event = NULL; coresight_set_mode(csdev, CS_MODE_DISABLED); raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -383,7 +373,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 +743,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 | 5 ++--- 3 files changed, 8 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 20a950b9dd4f..12d2f855de25 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;
- 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 (drvdata->event && + !coresight_sink_can_share(drvdata->event, handle->event)) return -EBUSY;
- if (drvdata->pid == -1) { + if (!drvdata->event) { smb_enable_hw(drvdata); - drvdata->pid = pid; + drvdata->event = handle->event; 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->event = NULL; coresight_set_mode(csdev, CS_MODE_DISABLED); dev_dbg(&csdev->dev, "Ultrasoc SMB disabled\n");
@@ -316,7 +308,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; } @@ -564,7 +555,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..5129afa1dfc8 100644 --- a/drivers/hwtracing/coresight/ultrasoc-smb.h +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h @@ -108,8 +108,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. + * @event: Perf event using this sink if in Perf mode. */ struct smb_drv_data { void __iomem *base; @@ -118,7 +117,7 @@ struct smb_drv_data { struct miscdevice miscdev; raw_spinlock_t spinlock; bool reading; - pid_t pid; + struct perf_event *event; };
#endif
-
James Clark