On 6/3/25 00:24, Stefan Hajnoczi wrote:
On Sun, Jun 01, 2025 at 06:12:30PM +0000, Tanish Desai wrote:Moved rarely used (cold) code from the header file to the C file to avoid unnecessary inlining and reduce binary size.How much of a binary size reduction do you measure? Most trace events are only called once, so the difference in code size is likely to be small.
Indeed I don't think there would be much reduction. I expect a bigger benefit in terms of improved register allocation when tracepoints are disabled, keeping unused tracepoint code out of the TLB, and the like.
That is, the difference in code size would be in the functions that include tracepoints, not in QEMU overall. That's a bit difficult to measure because you would have to isolate tracepoint code in the "before" .o files, but we can try.
This improves code organization and follows good practices for managing cold paths.It's easier to understand the code generator and the generated code when each trace event is implemented as a single function in the header file. Splitting the trace event up adds complexity. I don't think this is a step in the right direction.
I am not sure I agree on that; something like
static inline void trace_smmu_config_cache_inv(uint32_t sid)
{
if (trace_event_get_state(TRACE_SMMU_CONFIG_CACHE_INV)) {
_simple__trace_smmu_config_cache_inv(sid);
_log__trace_smmu_config_cache_inv(sid);
}
QEMU_SMMU_CONFIG_CACHE_INV(sid);
tracepoint(qemu, smmu_config_cache_inv(sid));
}
and one function per backend seems the most readable way to format the
code in the headers. I understand that most of the time you'll have
only one backend enabled, but still the above seems pretty good and
clarifies the difference between efficient backends like dtrace and UST
and the others.
This series doesn't go all the way to something like the above, but it does go in that direction.
Now, in all honesty the main reason to do this was to allow reusing the C code generator when it's Rust code that is using tracepoints; but I do believe that these changes make sense on their own, and I didn't want to make these a blocker for Rust enablement as well (Tanish has already looked into generating Rust code for the simple backend, for example).
Paolo
Signed-off-by: Tanish Desai <tanishdesa...@gmail.com> --- scripts/tracetool/backend/ftrace.py | 44 +++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py index baed2ae61c..c9717d7b42 100644 --- a/scripts/tracetool/backend/ftrace.py +++ b/scripts/tracetool/backend/ftrace.py @@ -23,6 +23,10 @@ def generate_h_begin(events, group): out('#include "trace/ftrace.h"', '') + for event in events: + out('void _ftrace_%(api)s(%(args)s);', + api=event.api(), + args=event.args)def generate_h(event, group):@@ -30,26 +34,42 @@ def generate_h(event, group): if len(event.args) > 0: argnames = ", " + argnames- out(' {',+ out(' if (trace_event_get_state(%(event_id)s)) {', + ' _ftrace_%(api)s(%(args)s);', + ' }', + name=event.name, + args=", ".join(event.args.names()), + event_id="TRACE_" + event.name.upper(), + event_lineno=event.lineno, + event_filename=os.path.relpath(event.filename), + fmt=event.fmt.rstrip("\n"), + argnames=argnames, + api=event.api() + ) + + +def generate_c(event, group): + argnames = ", ".join(event.args.names()) + if len(event.args) > 0: + argnames = ", " + argnames + out('void _ftrace_%(api)s(%(args)s){', ' char ftrace_buf[MAX_TRACE_STRLEN];', ' int unused __attribute__ ((unused));', ' int trlen;', - ' if (trace_event_get_state(%(event_id)s)) {', '#line %(event_lineno)d "%(event_filename)s"', - ' trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,', - ' "%(name)s " %(fmt)s "\\n" %(argnames)s);', + ' trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,', '#line %(out_next_lineno)d "%(out_filename)s"', - ' trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);', - ' unused = write(trace_marker_fd, ftrace_buf, trlen);', - ' }', - ' }', - name=event.name, - args=event.args, - event_id="TRACE_" + event.name.upper(), + ' "%(name)s " %(fmt)s "\\n" %(argnames)s);', + ' trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);', + ' unused = write(trace_marker_fd, ftrace_buf, trlen);', + '}', event_lineno=event.lineno, event_filename=os.path.relpath(event.filename), + name=event.name, fmt=event.fmt.rstrip("\n"), - argnames=argnames) + argnames=argnames, + api=event.api(), + args=event.args)def generate_h_backend_dstate(event, group):-- 2.34.1
