On Mon, Jun 29, 2026 at 10:00:01AM +0100, Yeoreum Yun wrote:
[...]
@@ -922,25 +950,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa { struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct etm4_enable_arg arg = { };
- unsigned long cfg_hash;
- int ret, preset;
- /* enable any config activated by configfs */
- cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset);
- if (cfg_hash) {
ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset);if (ret) {etm4_release_trace_id(drvdata);return ret;}- }
- raw_spin_lock(&drvdata->spinlock);
- drvdata->trcid = path->trace_id;
- /* Tracer will never be paused in sysfs mode */
- drvdata->paused = false;
- int ret;
/* * Executing etm4_enable_hw on the cpu whose ETM is being enabled @@ -948,20 +958,20 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa */ arg.drvdata = drvdata; arg.path = path;
- raw_spin_lock(&drvdata->spinlock);
- arg.config = drvdata->config;
- raw_spin_unlock(&drvdata->spinlock);
- ret = smp_call_function_single(drvdata->cpu, etm4_enable_sysfs_smp_call, &arg, 1); if (!ret) ret = arg.rc; if (!ret)
drvdata->sticky_enable = true;- if (ret)
dev_dbg(&csdev->dev, "ETM tracing enabled\n");- else etm4_release_trace_id(drvdata);
- raw_spin_unlock(&drvdata->spinlock);
- if (!ret)
dev_dbg(&csdev->dev, "ETM tracing enabled\n");}
This is most valuable change for me, as now we will have much clear scope for what is protected ("drvdata->config").
However, a corner case was mentioned by Sashiko:
| It appears etm4_enable_hw() modifies drvdata->ss_status while executing | via IPI, but sshot_ctrl_store() can modify the same array concurrently | from process context since the lock is no longer held across the | smp_call_function_single() call.
"drvdata->ss_status" is a race condition between SMP call and sysfs knobs. Should we change to spinlock_irqsave/irqrestore when access drvdata->ss_status?
Thanks, Leo