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 <[email protected]> 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/[email protected]?part=3
