[PATCH v8 08/13] coresight: etm4x: fix inconsistencies with sysfs configuration
Leo Yan
leo.yan at arm.com
Mon Jul 6 10:39:40 PDT 2026
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");
> return ret;
> }
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
More information about the linux-arm-kernel
mailing list