* [PATCH] coresight: fix missing error code when trace ID is invalid
@ 2026-05-08 5:45 Jie Gan
2026-05-08 8:25 ` James Clark
2026-05-08 13:44 ` Leo Yan
0 siblings, 2 replies; 5+ messages in thread
From: Jie Gan @ 2026-05-08 5:45 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
Alexander Shishkin, Tingwei Zhang
Cc: coresight, linux-arm-kernel, linux-kernel, Jie Gan
When coresight_path_assign_trace_id() fails to allocate a valid trace
ID, the code jumps to err_path without setting ret to an error value.
This causes coresight_enable_sysfs() to return 0 (success) to the
caller even though no trace session was started.
Set ret = -EINVAL before the goto so that callers receive a proper
error code.
Fixes: d87d76d823d1 ("Coresight: Allocate trace ID after building the path")
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
drivers/hwtracing/coresight/coresight-sysfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index d2a6ed8bcc74..c9338c783540 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -195,42 +195,44 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
*/
if (subtype == CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)
csdev->refcnt++;
goto out;
}
sink = coresight_find_activated_sysfs_sink(csdev);
if (!sink) {
ret = -EINVAL;
goto out;
}
path = coresight_build_path(csdev, sink);
if (IS_ERR(path)) {
pr_err("building path(s) failed\n");
ret = PTR_ERR(path);
goto out;
}
coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
- if (!IS_VALID_CS_TRACE_ID(path->trace_id))
+ if (!IS_VALID_CS_TRACE_ID(path->trace_id)) {
+ ret = -EINVAL;
goto err_path;
+ }
ret = coresight_enable_path(path, CS_MODE_SYSFS);
if (ret)
goto err_path;
ret = coresight_enable_source_sysfs(csdev, CS_MODE_SYSFS, path);
if (ret)
goto err_source;
switch (subtype) {
case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
/*
* When working from sysFS it is important to keep track
* of the paths that were created so that they can be
* undone in 'coresight_disable()'. Since there can only
* be a single session per tracer (when working from sysFS)
* a per-cpu variable will do just fine.
*/
cpu = source_ops(csdev)->cpu_id(csdev);
per_cpu(tracer_path, cpu) = path;
---
base-commit: 17c7841d09ee7d33557fd075562d9289b6018c90
change-id: 20260508-fix-trace-id-error-dbfdd4d8f2d1
Best regards,
--
Jie Gan <jie.gan@oss.qualcomm.com>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] coresight: fix missing error code when trace ID is invalid
2026-05-08 5:45 [PATCH] coresight: fix missing error code when trace ID is invalid Jie Gan
@ 2026-05-08 8:25 ` James Clark
2026-05-08 13:44 ` Leo Yan
1 sibling, 0 replies; 5+ messages in thread
From: James Clark @ 2026-05-08 8:25 UTC (permalink / raw)
To: Jie Gan
Cc: coresight, linux-arm-kernel, linux-kernel, Suzuki K Poulose,
Mike Leach, Leo Yan, Alexander Shishkin, Tingwei Zhang
On 08/05/2026 6:45 am, Jie Gan wrote:
> When coresight_path_assign_trace_id() fails to allocate a valid trace
> ID, the code jumps to err_path without setting ret to an error value.
> This causes coresight_enable_sysfs() to return 0 (success) to the
> caller even though no trace session was started.
>
> Set ret = -EINVAL before the goto so that callers receive a proper
> error code.
>
> Fixes: d87d76d823d1 ("Coresight: Allocate trace ID after building the path")
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
> drivers/hwtracing/coresight/coresight-sysfs.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index d2a6ed8bcc74..c9338c783540 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -195,42 +195,44 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
> */
> if (subtype == CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)
> csdev->refcnt++;
> goto out;
> }
>
> sink = coresight_find_activated_sysfs_sink(csdev);
> if (!sink) {
> ret = -EINVAL;
> goto out;
> }
>
> path = coresight_build_path(csdev, sink);
> if (IS_ERR(path)) {
> pr_err("building path(s) failed\n");
> ret = PTR_ERR(path);
> goto out;
> }
>
> coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
> - if (!IS_VALID_CS_TRACE_ID(path->trace_id))
> + if (!IS_VALID_CS_TRACE_ID(path->trace_id)) {
> + ret = -EINVAL;
> goto err_path;
> + }
>
> ret = coresight_enable_path(path, CS_MODE_SYSFS);
> if (ret)
> goto err_path;
>
> ret = coresight_enable_source_sysfs(csdev, CS_MODE_SYSFS, path);
> if (ret)
> goto err_source;
>
> switch (subtype) {
> case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
> /*
> * When working from sysFS it is important to keep track
> * of the paths that were created so that they can be
> * undone in 'coresight_disable()'. Since there can only
> * be a single session per tracer (when working from sysFS)
> * a per-cpu variable will do just fine.
> */
> cpu = source_ops(csdev)->cpu_id(csdev);
> per_cpu(tracer_path, cpu) = path;
>
> ---
> base-commit: 17c7841d09ee7d33557fd075562d9289b6018c90
> change-id: 20260508-fix-trace-id-error-dbfdd4d8f2d1
>
> Best regards,
Reviewed-by: James Clark <james.clark@linaro.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] coresight: fix missing error code when trace ID is invalid
2026-05-08 5:45 [PATCH] coresight: fix missing error code when trace ID is invalid Jie Gan
2026-05-08 8:25 ` James Clark
@ 2026-05-08 13:44 ` Leo Yan
2026-05-08 13:53 ` Jie Gan
2026-05-08 14:00 ` Suzuki K Poulose
1 sibling, 2 replies; 5+ messages in thread
From: Leo Yan @ 2026-05-08 13:44 UTC (permalink / raw)
To: Jie Gan
Cc: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
Tingwei Zhang, coresight, linux-arm-kernel, linux-kernel
On Fri, May 08, 2026 at 01:45:35PM +0800, Jie Gan wrote:
[...]
> coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
> - if (!IS_VALID_CS_TRACE_ID(path->trace_id))
> + if (!IS_VALID_CS_TRACE_ID(path->trace_id)) {
> + ret = -EINVAL;
> goto err_path;
> + }
On the top of this patch, could we do a further improvement?
Move IS_VALID_CS_TRACE_ID() into coresight_path_assign_trace_id() and
return 0 for success and < 0 for failures. As result, callers only
need to check the returned value.
For this patch:
Reviewed-by: Leo Yan <leo.yan@arm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] coresight: fix missing error code when trace ID is invalid
2026-05-08 13:44 ` Leo Yan
@ 2026-05-08 13:53 ` Jie Gan
2026-05-08 14:00 ` Suzuki K Poulose
1 sibling, 0 replies; 5+ messages in thread
From: Jie Gan @ 2026-05-08 13:53 UTC (permalink / raw)
To: Leo Yan
Cc: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
Tingwei Zhang, coresight, linux-arm-kernel, linux-kernel
Hi Leo,
On 5/8/2026 9:44 PM, Leo Yan wrote:
> On Fri, May 08, 2026 at 01:45:35PM +0800, Jie Gan wrote:
>
> [...]
>
>> coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
>> - if (!IS_VALID_CS_TRACE_ID(path->trace_id))
>> + if (!IS_VALID_CS_TRACE_ID(path->trace_id)) {
>> + ret = -EINVAL;
>> goto err_path;
>> + }
>
> On the top of this patch, could we do a further improvement?
>
> Move IS_VALID_CS_TRACE_ID() into coresight_path_assign_trace_id() and
> return 0 for success and < 0 for failures. As result, callers only
> need to check the returned value.
>
That's a good suggestion. I will check these logic once. I will send a
new patch to do the refactor if worth.
Thanks,
Jie
> For this patch:
>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] coresight: fix missing error code when trace ID is invalid
2026-05-08 13:44 ` Leo Yan
2026-05-08 13:53 ` Jie Gan
@ 2026-05-08 14:00 ` Suzuki K Poulose
1 sibling, 0 replies; 5+ messages in thread
From: Suzuki K Poulose @ 2026-05-08 14:00 UTC (permalink / raw)
To: Leo Yan, Jie Gan
Cc: Mike Leach, James Clark, Alexander Shishkin, Tingwei Zhang,
coresight, linux-arm-kernel, linux-kernel
On 08/05/2026 14:44, Leo Yan wrote:
> On Fri, May 08, 2026 at 01:45:35PM +0800, Jie Gan wrote:
>
> [...]
>
>> coresight_path_assign_trace_id(path, CS_MODE_SYSFS);
>> - if (!IS_VALID_CS_TRACE_ID(path->trace_id))
>> + if (!IS_VALID_CS_TRACE_ID(path->trace_id)) {
>> + ret = -EINVAL;
>> goto err_path;
>> + }
>
> On the top of this patch, could we do a further improvement?
>
> Move IS_VALID_CS_TRACE_ID() into coresight_path_assign_trace_id() and
> return 0 for success and < 0 for failures. As result, callers only
> need to check the returned value.
Yes please ^
Cheers
Suzuki
>
> For this patch:
>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-08 14:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 5:45 [PATCH] coresight: fix missing error code when trace ID is invalid Jie Gan
2026-05-08 8:25 ` James Clark
2026-05-08 13:44 ` Leo Yan
2026-05-08 13:53 ` Jie Gan
2026-05-08 14:00 ` Suzuki K Poulose
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox