Re: [PATCH 2/3] trace/ftrace: seperate cold paths of tracing functions

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3747 bytes --]

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.

> 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.

> 
> Signed-off-by: Tanish Desai <tanishdesai37@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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]