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

Tanish Desai posted 3 patches 5 months, 2 weeks ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, Mads Ynddal <mads@ynddal.dk>
[PATCH 2/3] trace/ftrace: seperate cold paths of tracing functions
Posted by Tanish Desai 5 months, 2 weeks ago
Moved rarely used (cold) code from the header file to the C file to avoid
unnecessary inlining and reduce binary size. This improves code organization
and follows good practices for managing cold paths.

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
Re: [PATCH 2/3] trace/ftrace: seperate cold paths of tracing functions
Posted by Stefan Hajnoczi 5 months, 2 weeks ago
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
>