Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [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