Re: [PATCH 3/6] trace: qpace: Add tracepoints for QPaCE operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] trace: unpaired end tracepoint in qpace_urgent_decompress() on error
- [Low] trace: qpace tracepoints violate subsystem_event naming convention
--

commit 95671997f765567ada636d922558170e766019a7
Author: Georgi Djakov <georgi.djakov@oss.qualcomm.com>

trace: qpace: Add tracepoints for QPaCE operations

Add ftrace tracepoints for the Qualcomm Page Compression Engine
to allow performance analysis and debugging of QPaCE hardware
operations.

> diff --git a/drivers/soc/qcom/qpace.c b/drivers/soc/qcom/qpace.c
> index 69aa14b7c4772..f5b21ffbcbff5 100644
> --- a/drivers/soc/qcom/qpace.c
> +++ b/drivers/soc/qcom/qpace.c

[ ... ]

> @@ -356,6 +362,9 @@ int qpace_urgent_decompress(dma_addr_t input_addr,
>  	if (ret)
>  		goto out;
>  
> +	trace_start_qpace_urgent_decompress((u64)input_addr,
> +					    (u64)output_addr, input_size);
> +
>  	urg_reg_num = get_cpu() % NUM_TRS_ERS_URG_CMD_REGS;

[ ... ]

>  	ret = FIELD_GET(URG_CMD_0_ED_STAT_SIZE, stat_reg);
>  out:
> +	trace_end_qpace_urgent_decompress((u64)input_addr,
> +					  (u64)output_addr,
> +					  input_size, ret);
>  	return ret;
>  }

[Severity: Medium]
If qpace_get() fails at the beginning of qpace_urgent_decompress(), the code
jumps to the 'out' label, bypassing trace_start_qpace_urgent_decompress()
entirely.

Does this result in an unpaired end tracepoint being emitted unconditionally?

> diff --git a/include/trace/events/qpace.h b/include/trace/events/qpace.h
> new file mode 100644
> index 0000000000000..3f0ccdb8f2060
> --- /dev/null
> +++ b/include/trace/events/qpace.h
> @@ -0,0 +1,99 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM qpace
> +
> +#if !defined(_TRACE_QPACE_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_QPACE_H
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(start_qpace_urgent_decompress,

[Severity: Low]
Does this tracepoint name violate the mandatory subsystem_event naming
convention by using an action prefix instead of the subsystem prefix?

Could these be renamed to use the subsystem prefix, such as
qpace_start_urgent_decompress?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930145215.3380977-1-georgi.djakov@oss.qualcomm.com?part=3