Re: [PATCH v5 06/12] coresight: etm4x: fix leaked trace id

[Jie Gan]

On 4/17/2026 12:55 AM, Leo Yan wrote:
> On Wed, Apr 15, 2026 at 05:55:22PM +0100, Yeoreum Yun wrote:
> > If etm4_enable_sysfs() fails in cscfg_csdev_enable_active_config(),
> > the trace ID may be leaked because it is not released.
> >
> > To address this, call etm4_release_trace_id() when etm4_enable_sysfs()
> > fails in cscfg_csdev_enable_active_config().
> >
> > Reviewed-by: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@xxxxxxx>
> > ---
> >   drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > index f55338a4989d..b199aebbdb60 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > @@ -920,8 +920,10 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa
> >   	cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset);
> >   	if (cfg_hash) {
> >   		ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset);
> > -		if (ret)
> > +		if (ret) {
> > +			etm4_release_trace_id(drvdata);
> >   			return ret;
> > +		}
>
> LGTM:
>
> Reviewed-by: Leo Yan <leo.yan@xxxxxxx>
>
> Just recording a bit thoughts.  As Suzuki mentioned, it would be better
> to allocate trace IDs within a session.  We might consider maintaining
> the trace ID map in the sink driver data, since the sink driver is
> unique within a session so it is a central place to allocate trace ID.
>
> We should use paired way for allocation and release. For example:
>
>    coresight_enable_sysfs()
>    {
>        ...
>        coresight_path_assign_trace_id(path);
>
>    failed:
>        coresight_path_unassign_trace_id(path);
>    }
>
>    coresight_disable_sysfs()
>    {
>        coresight_path_unassign_trace_id(path);
>    }
>
> But this requires broader refactoring.  E.g., the STM driver currently
> allocates system trace IDs statically during probe, we might need to
> consolidate for all modules to use dynamic allocation.

Agree. That's making sense. Currently, the trace ID of some devices is 
allocated during probe, and never to be released. It's kind of waste of 
our trace ID resource if the device never to be enabled. But we still 
need support static trace ID allocation in parallel for the dummy 
sources and we should not break this logic in future refactor.

Thanks,
Jie

> Thanks,
> Leo