Re: [PATCH v5 07/12] coresight: etm4x: fix inconsistencies with sysfs configuration

[Yeoreum Yun]

Hi Jie,
>
>
> On 4/16/2026 12:55 AM, Yeoreum Yun wrote:
> > The current ETM4x configuration via sysfs can lead to
> > several inconsistencies:
> >
> >    - If the configuration is modified via sysfs while a perf session is
> >      active, the running configuration may differ before a sched-out and
> >      after a subsequent sched-in.
> >
> >    - If a perf session and a sysfs session enable tracing concurrently,
> >      the configuration from configfs may become corrupted.
> >
> >    - There is a risk of corrupting drvdata->config if a perf session enables
> >      tracing while cscfg_csdev_disable_active_config() is being handled in
> >      etm4_disable_sysfs().
> >
> > To resolve these issues, separate the configuration into:
> >
> >    - active_config: the configuration applied to the current session
> >    - config: the configuration set via sysfs
> >
> > Additionally:
> >
> >    - Apply the configuration from configfs after taking the appropriate mode.
> >
> >    - Since active_config and related fields are accessed only by the local CPU
> >      in etm4_enable/disable_sysfs_smp_call() (similar to perf enable/disable),
> >      remove the lock/unlock from the sysfs enable/disable path and
> >      startup/dying_cpu except when to access config fields.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@xxxxxxx>
> > ---
>
> <...>
>
> > @@ -618,23 +624,45 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> >   static void etm4_enable_sysfs_smp_call(void *info)
> >   {
> >   	struct etm4_enable_arg *arg = info;
> > +	struct etmv4_drvdata *drvdata;
> >   	struct coresight_device *csdev;
> >   	if (WARN_ON(!arg))
> >   		return;
> > -	csdev = arg->drvdata->csdev;
> > +	drvdata = arg->drvdata;
> > +	csdev = drvdata->csdev;
> >   	if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
> >   		/* Someone is already using the tracer */
> >   		arg->rc = -EBUSY;
> >   		return;
> >   	}
> > -	arg->rc = etm4_enable_hw(arg->drvdata);
> > +	drvdata->active_config = arg->config;
> > -	/* The tracer didn't start */
> > +	if (arg->cfg_hash) {
> > +		arg->rc = cscfg_csdev_enable_active_config(csdev,
> > +							   arg->cfg_hash,
> > +							   arg->preset);
> > +		if (arg->rc)
> > +			goto err;
> > +	}
> > +
> > +	drvdata->trcid = arg->trace_id;
> > +
> > +	/* Tracer will never be paused in sysfs mode */
> > +	drvdata->paused = false;
> > +
> > +	arg->rc = etm4_enable_hw(drvdata);
> >   	if (arg->rc)
> > -		coresight_set_mode(csdev, CS_MODE_DISABLED);
>
> needs disable the active config in error path:
> cscfg_csdev_disable_active_config(drvdata->csdev);

You're right. I missed it. Thanks!

[...]

--
Sincerely,
Yeoreum Yun