[PATCH v2 0/3] tracetool: cleanup "if(true)" check from trace_foo()

[PATCH v2 0/3] tracetool: cleanup "if(true)" check from trace_foo()

[Tanish Desai]

This series of patch aims to removes the leftover if (true) condition
from trace_foo, a remnant from the TCG tracing feature removal.

It replaces it with a proper trace_event_get_state(...)
check where necessary (for log/simple/syslog and ftrace backend).

Additionally, this change centralizes the generation of
trace_event_get_state(...) calls into format/h.py,
eliminating redundant code across individual backends.

This cleanup results in more consistent and less
repetitive backend code.

Tanish Desai (3):
  tracetool: removed the unused vcpu property
  tracetool: introduce generate_unconditional
  tracetool: remove redundant event_get_state checks

 scripts/tracetool/__init__.py         |  6 +++---
 scripts/tracetool/backend/__init__.py |  3 +++
 scripts/tracetool/backend/dtrace.py   |  3 ++-
 scripts/tracetool/backend/ftrace.py   |  3 ---
 scripts/tracetool/backend/log.py      |  9 +--------
 scripts/tracetool/backend/simple.py   | 11 +----------
 scripts/tracetool/backend/syslog.py   | 11 +----------
 scripts/tracetool/backend/ust.py      |  2 +-
 scripts/tracetool/format/h.py         | 13 +++++++++----
 9 files changed, 21 insertions(+), 40 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] tracetool: removed the unused vcpu property

[Tanish Desai]

The vcpu property is no longer used in these backends. Removing it avoids
unnecessary checks and simplifies the code generation for these trace
backends.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
---
 scripts/tracetool/__init__.py       | 6 +++---
 scripts/tracetool/backend/log.py    | 6 +-----
 scripts/tracetool/backend/simple.py | 6 +-----
 scripts/tracetool/backend/syslog.py | 6 +-----
 4 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index bc03238c0f..e86c898a1e 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -216,9 +216,9 @@ class Event(object):
                       r"\s*"
                       r"(?:(?:(?P<fmt_trans>\".+),)?\s*(?P<fmt>\".+))?"
                       r"\s*")
-
-    _VALID_PROPS = set(["disable", "vcpu"])
-
+    
+    _VALID_PROPS = set(["disable"])
+    
     def __init__(self, name, props, fmt, args, lineno, filename, orig=None,
                  event_trans=None, event_exec=None):
         """
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index de27b7e62e..f24acad74c 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -31,11 +31,7 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    if "vcpu" in event.properties:
-        # already checked on the generic format code
-        cond = "true"
-    else:
-        cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
+    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
 
     out('    if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {',
         '        if (message_with_timestamp) {',
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 2688d4b64b..7c84c06b20 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -37,11 +37,7 @@ def generate_h_begin(events, group):
 
 def generate_h(event, group):
     event_id = 'TRACE_' + event.name.upper()
-    if "vcpu" in event.properties:
-        # already checked on the generic format code
-        cond = "true"
-    else:
-        cond = "trace_event_get_state(%s)" % event_id
+    cond = "trace_event_get_state(%s)" % event_id
     out('    if (%(cond)s) {',
         '        _simple_%(api)s(%(args)s);',
         '    }',
diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
index 012970f6cc..6fdc1142fb 100644
--- a/scripts/tracetool/backend/syslog.py
+++ b/scripts/tracetool/backend/syslog.py
@@ -30,11 +30,7 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    if "vcpu" in event.properties:
-        # already checked on the generic format code
-        cond = "true"
-    else:
-        cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
+    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
 
     out('    if (%(cond)s) {',
         '#line %(event_lineno)d "%(event_filename)s"',
-- 
2.34.1

Re: [PATCH v2 1/3] tracetool: removed the unused vcpu property

[Alex Bennée]

Tanish Desai <tanishdesai37@gmail.com> writes:

> The vcpu property is no longer used in these backends. Removing it avoids
> unnecessary checks and simplifies the code generation for these trace
> backends.
>
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>

I think you forgot to collect the r-b tags from v1, see:

  https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review

> ---
>  scripts/tracetool/__init__.py       | 6 +++---
>  scripts/tracetool/backend/log.py    | 6 +-----
>  scripts/tracetool/backend/simple.py | 6 +-----
>  scripts/tracetool/backend/syslog.py | 6 +-----
>  4 files changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index bc03238c0f..e86c898a1e 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -216,9 +216,9 @@ class Event(object):
>                        r"\s*"
>                        r"(?:(?:(?P<fmt_trans>\".+),)?\s*(?P<fmt>\".+))?"
>                        r"\s*")
> -
> -    _VALID_PROPS = set(["disable", "vcpu"])
> -
> +    
> +    _VALID_PROPS = set(["disable"])
> +    
>      def __init__(self, name, props, fmt, args, lineno, filename, orig=None,
>                   event_trans=None, event_exec=None):
>          """
> diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
> index de27b7e62e..f24acad74c 100644
> --- a/scripts/tracetool/backend/log.py
> +++ b/scripts/tracetool/backend/log.py
> @@ -31,11 +31,7 @@ def generate_h(event, group):
>      if len(event.args) > 0:
>          argnames = ", " + argnames
>  
> -    if "vcpu" in event.properties:
> -        # already checked on the generic format code
> -        cond = "true"
> -    else:
> -        cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
> +    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
>  
>      out('    if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {',
>          '        if (message_with_timestamp) {',
> diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
> index 2688d4b64b..7c84c06b20 100644
> --- a/scripts/tracetool/backend/simple.py
> +++ b/scripts/tracetool/backend/simple.py
> @@ -37,11 +37,7 @@ def generate_h_begin(events, group):
>  
>  def generate_h(event, group):
>      event_id = 'TRACE_' + event.name.upper()
> -    if "vcpu" in event.properties:
> -        # already checked on the generic format code
> -        cond = "true"
> -    else:
> -        cond = "trace_event_get_state(%s)" % event_id
> +    cond = "trace_event_get_state(%s)" % event_id
>      out('    if (%(cond)s) {',
>          '        _simple_%(api)s(%(args)s);',
>          '    }',
> diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
> index 012970f6cc..6fdc1142fb 100644
> --- a/scripts/tracetool/backend/syslog.py
> +++ b/scripts/tracetool/backend/syslog.py
> @@ -30,11 +30,7 @@ def generate_h(event, group):
>      if len(event.args) > 0:
>          argnames = ", " + argnames
>  
> -    if "vcpu" in event.properties:
> -        # already checked on the generic format code
> -        cond = "true"
> -    else:
> -        cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
> +    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
>  
>      out('    if (%(cond)s) {',
>          '#line %(event_lineno)d "%(event_filename)s"',

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2 1/3] tracetool: removed the unused vcpu property

[Tanish Desai]

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

Noted will take care for all upcoming patches.
For this patch should I resend it with r-b tags from v1 or this patch it’s
fine for now?


On Fri, 20 Jun 2025 at 9:43 PM, Alex Bennée <alex.bennee@linaro.org> wrote:

> Tanish Desai <tanishdesai37@gmail.com> writes:
>
> > The vcpu property is no longer used in these backends. Removing it avoids
> > unnecessary checks and simplifies the code generation for these trace
> > backends.
> >
> > Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
>
> I think you forgot to collect the r-b tags from v1, see:
>
>
> https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review
>
> > ---
> >  scripts/tracetool/__init__.py       | 6 +++---
> >  scripts/tracetool/backend/log.py    | 6 +-----
> >  scripts/tracetool/backend/simple.py | 6 +-----
> >  scripts/tracetool/backend/syslog.py | 6 +-----
> >  4 files changed, 6 insertions(+), 18 deletions(-)
> >
> > diff --git a/scripts/tracetool/__init__.py
> b/scripts/tracetool/__init__.py
> > index bc03238c0f..e86c898a1e 100644
> > --- a/scripts/tracetool/__init__.py
> > +++ b/scripts/tracetool/__init__.py
> > @@ -216,9 +216,9 @@ class Event(object):
> >                        r"\s*"
> >                        r"(?:(?:(?P<fmt_trans>\".+),)?\s*(?P<fmt>\".+))?"
> >                        r"\s*")
> > -
> > -    _VALID_PROPS = set(["disable", "vcpu"])
> > -
> > +
> > +    _VALID_PROPS = set(["disable"])
> > +
> >      def __init__(self, name, props, fmt, args, lineno, filename,
> orig=None,
> >                   event_trans=None, event_exec=None):
> >          """
> > diff --git a/scripts/tracetool/backend/log.py
> b/scripts/tracetool/backend/log.py
> > index de27b7e62e..f24acad74c 100644
> > --- a/scripts/tracetool/backend/log.py
> > +++ b/scripts/tracetool/backend/log.py
> > @@ -31,11 +31,7 @@ def generate_h(event, group):
> >      if len(event.args) > 0:
> >          argnames = ", " + argnames
> >
> > -    if "vcpu" in event.properties:
> > -        # already checked on the generic format code
> > -        cond = "true"
> > -    else:
> > -        cond = "trace_event_get_state(%s)" % ("TRACE_" +
> event.name.upper())
> > +    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
> >
> >      out('    if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {',
> >          '        if (message_with_timestamp) {',
> > diff --git a/scripts/tracetool/backend/simple.py
> b/scripts/tracetool/backend/simple.py
> > index 2688d4b64b..7c84c06b20 100644
> > --- a/scripts/tracetool/backend/simple.py
> > +++ b/scripts/tracetool/backend/simple.py
> > @@ -37,11 +37,7 @@ def generate_h_begin(events, group):
> >
> >  def generate_h(event, group):
> >      event_id = 'TRACE_' + event.name.upper()
> > -    if "vcpu" in event.properties:
> > -        # already checked on the generic format code
> > -        cond = "true"
> > -    else:
> > -        cond = "trace_event_get_state(%s)" % event_id
> > +    cond = "trace_event_get_state(%s)" % event_id
> >      out('    if (%(cond)s) {',
> >          '        _simple_%(api)s(%(args)s);',
> >          '    }',
> > diff --git a/scripts/tracetool/backend/syslog.py
> b/scripts/tracetool/backend/syslog.py
> > index 012970f6cc..6fdc1142fb 100644
> > --- a/scripts/tracetool/backend/syslog.py
> > +++ b/scripts/tracetool/backend/syslog.py
> > @@ -30,11 +30,7 @@ def generate_h(event, group):
> >      if len(event.args) > 0:
> >          argnames = ", " + argnames
> >
> > -    if "vcpu" in event.properties:
> > -        # already checked on the generic format code
> > -        cond = "true"
> > -    else:
> > -        cond = "trace_event_get_state(%s)" % ("TRACE_" +
> event.name.upper())
> > +    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
> >
> >      out('    if (%(cond)s) {',
> >          '#line %(event_lineno)d "%(event_filename)s"',
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
>

[-- Attachment #2: Type: text/html, Size: 5842 bytes --]

[PATCH v2 2/3] tracetool: introduce generate_unconditional

[Tanish Desai]

This patch separates the generation logic of trace_foo() for the UST
and DTrace backends from other backends.

The motivation is to remove the unnecessary if (true) in the trace_foo()
function, as UST and DTrace do not require a trace_event_get_state check
without introducing a seperate function it is very difficult to generate
code which keeps them out of unified if condition.

With this separation, we can safely move the trace_event_get_state check
into trace_foo for the other backends only, keeping UST/DTrace generation
paths clean.

A new generate_h_unconditional function has been introduced for UST and DTrace.
It behaves similarly to generate_h, but is defined only in UST and
DTrace backends.

This ensures that generate_h is used by the other backends, while UST/DTrace
selectively use generate_h_unconditional.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
---
 scripts/tracetool/backend/__init__.py |  3 +++
 scripts/tracetool/backend/dtrace.py   |  3 ++-
 scripts/tracetool/backend/ust.py      |  2 +-
 scripts/tracetool/format/h.py         | 10 +++++++---
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
index 7bfcc86cc5..c4456a5efd 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -118,6 +118,9 @@ def generate_begin(self, events, group):
     def generate(self, event, group):
         self._run_function("generate_%s", event, group)
 
+    def generate_unconditional(self, event, group):
+        self._run_function("generate_%s_unconditional", event, group)
+    
     def generate_backend_dstate(self, event, group):
         self._run_function("generate_%s_backend_dstate", event, group)
 
diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
index e17edc9b9d..171b7e09ed 100644
--- a/scripts/tracetool/backend/dtrace.py
+++ b/scripts/tracetool/backend/dtrace.py
@@ -61,7 +61,8 @@ def generate_h_begin(events, group):
             '#endif',
             uppername=e.name.upper())
 
-def generate_h(event, group):
+
+def generate_h_unconditional(event, group):
     out('    QEMU_%(uppername)s(%(argnames)s);',
         uppername=event.name.upper(),
         argnames=", ".join(event.args.names()))
diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
index c857516f21..1564b490ec 100644
--- a/scripts/tracetool/backend/ust.py
+++ b/scripts/tracetool/backend/ust.py
@@ -30,7 +30,7 @@ def generate_h_begin(events, group):
         '')
 
 
-def generate_h(event, group):
+def generate_h_unconditional(event, group):
     argnames = ", ".join(event.args.names())
     if len(event.args) > 0:
         argnames = ", " + argnames
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index ea126b07ea..89d54b9aff 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -76,13 +76,17 @@ def generate(events, backend, group):
         out('',
             'static inline void %(api)s(%(args)s)',
             '{',
-            '    if (%(cond)s) {',
+            api=e.api(),
+            args=e.args)
+        
+        if "disable" not in e.properties:
+            backend.generate_unconditional(e, group)
+
+        out('    if (%(cond)s) {',
             '        %(api_nocheck)s(%(names)s);',
             '    }',
             '}',
-            api=e.api(),
             api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
-            args=e.args,
             names=", ".join(e.args.names()),
             cond=cond)
 
-- 
2.34.1

Re: [PATCH v2 2/3] tracetool: introduce generate_unconditional

[Stefan Hajnoczi]

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

On Fri, Jun 20, 2025 at 02:37:19PM +0000, Tanish Desai wrote:
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index ea126b07ea..89d54b9aff 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -76,13 +76,17 @@ def generate(events, backend, group):
>          out('',
>              'static inline void %(api)s(%(args)s)',
>              '{',
> -            '    if (%(cond)s) {',
> +            api=e.api(),
> +            args=e.args)
> +        
> +        if "disable" not in e.properties:
> +            backend.generate_unconditional(e, group)
> +
> +        out('    if (%(cond)s) {',
>              '        %(api_nocheck)s(%(names)s);',
>              '    }',
>              '}',
> -            api=e.api(),
>              api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> -            args=e.args,
>              names=", ".join(e.args.names()),
>              cond=cond)

Two thoughts:

1. nocheck isn't necessary anymore. The body of nocheck could be inlined
   here instead to simplify the generated code.

2. "if (%(cond)s) {" is only useful for backends that implement
   .generate(). For example, if only the dtrace backend is enabled then
   "if (trace_event_get_state(...)) {}" will be emitted unnecessarily.

Maybe backends should have a .condition() interface so that
scripts/tracetool/format/h.py:generate() can first collect a dict[cond]
-> backend. Then it iterates over the map, calling backend.generate()
within "if (%(cond)s) { ... }". That way only the conditions that are
actually needed are generated and multiple backends that have the same
condition will share the same if statement.

Stefan

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

Re: [PATCH v2 2/3] tracetool: introduce generate_unconditional

[Tanish Desai]

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

> 1. nocheck isn't necessary anymore. The body of nocheck could be inlined
>   here instead to simplify the generated code.
Yes I agree.I will remove nocheck and inline the body of nocheck in
trace-foo
> 2. "if (%(cond)s) {" is only useful for backends that implement
>   .generate(). For example, if only the dtrace backend is enabled then
>   "if (trace_event_get_state(...)) {}" will be emitted unnecessarily.
Yes, we should remove unnecessary if (trace_event_get_state(...)){}
blocks.

But It is difficult because backend.generate() calls
_run_function("generate_%s", event, group) which in turn loops on all
backends.Format can't call generate for individual backends.I will need
to make this map in scripts/tracetool/backend/__init__.py:_run_function().(I
think this
will not be a good thing to do).

possible fix would be to create in scripts/tracetool/backend/__init__.py
def is_conditional(self, cond_check):
    self._run_function("generate_%s_conditional", cond_check)
now cond_check will be passed to all backends and backend's will have
def is_h_conditional(cond_check):
    cond_check = cond_check or True

Finally if cond_check==True in h.py I will generate "if
(trace_event_get_state(...)) {"
else not. As the same condition is re-used this solution would work well.
Since h.py only handles reused/shared logic, it's safe to assume consistent
conditions.
If a new backend requires a different condition, it's better handled
directly in backend/*.py.


On Tue, Jun 24, 2025 at 8:07 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:

> On Fri, Jun 20, 2025 at 02:37:19PM +0000, Tanish Desai wrote:
> > diff --git a/scripts/tracetool/format/h.py
> b/scripts/tracetool/format/h.py
> > index ea126b07ea..89d54b9aff 100644
> > --- a/scripts/tracetool/format/h.py
> > +++ b/scripts/tracetool/format/h.py
> > @@ -76,13 +76,17 @@ def generate(events, backend, group):
> >          out('',
> >              'static inline void %(api)s(%(args)s)',
> >              '{',
> > -            '    if (%(cond)s) {',
> > +            api=e.api(),
> > +            args=e.args)
> > +
> > +        if "disable" not in e.properties:
> > +            backend.generate_unconditional(e, group)
> > +
> > +        out('    if (%(cond)s) {',
> >              '        %(api_nocheck)s(%(names)s);',
> >              '    }',
> >              '}',
> > -            api=e.api(),
> >              api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> > -            args=e.args,
> >              names=", ".join(e.args.names()),
> >              cond=cond)
>
> Two thoughts:
>
> 1. nocheck isn't necessary anymore. The body of nocheck could be inlined
>    here instead to simplify the generated code.
>
> 2. "if (%(cond)s) {" is only useful for backends that implement
>    .generate(). For example, if only the dtrace backend is enabled then
>    "if (trace_event_get_state(...)) {}" will be emitted unnecessarily.
>
> Maybe backends should have a .condition() interface so that
> scripts/tracetool/format/h.py:generate() can first collect a dict[cond]
> -> backend. Then it iterates over the map, calling backend.generate()
> within "if (%(cond)s) { ... }". That way only the conditions that are
> actually needed are generated and multiple backends that have the same
> condition will share the same if statement.
>
> Stefan
>

[-- Attachment #2: Type: text/html, Size: 5263 bytes --]

Re: [PATCH v2 2/3] tracetool: introduce generate_unconditional

[Stefan Hajnoczi]

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

On Tue, Jun 24, 2025 at 11:07:34PM +0530, Tanish Desai wrote:
> > 1. nocheck isn't necessary anymore. The body of nocheck could be inlined
> >   here instead to simplify the generated code.
> Yes I agree.I will remove nocheck and inline the body of nocheck in
> trace-foo
> > 2. "if (%(cond)s) {" is only useful for backends that implement
> >   .generate(). For example, if only the dtrace backend is enabled then
> >   "if (trace_event_get_state(...)) {}" will be emitted unnecessarily.
> Yes, we should remove unnecessary if (trace_event_get_state(...)){}
> blocks.
> 
> But It is difficult because backend.generate() calls
> _run_function("generate_%s", event, group) which in turn loops on all
> backends.Format can't call generate for individual backends.I will need
> to make this map in scripts/tracetool/backend/__init__.py:_run_function().(I
> think this
> will not be a good thing to do).
> 
> possible fix would be to create in scripts/tracetool/backend/__init__.py
> def is_conditional(self, cond_check):
>     self._run_function("generate_%s_conditional", cond_check)
> now cond_check will be passed to all backends and backend's will have
> def is_h_conditional(cond_check):
>     cond_check = cond_check or True
> 
> Finally if cond_check==True in h.py I will generate "if
> (trace_event_get_state(...)) {"
> else not. As the same condition is re-used this solution would work well.
> Since h.py only handles reused/shared logic, it's safe to assume consistent
> conditions.
> If a new backend requires a different condition, it's better handled
> directly in backend/*.py.

To me a generic solution that doesn't hard-code trace_event_get_state()
is nicer, but it's okay if you want to introduce the concept of
"conditional" meaning specifically trace_event_get_state() since there
are no other cases where we want to merge two conditionals. I don't
really mind.

> 
> 
> On Tue, Jun 24, 2025 at 8:07 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > On Fri, Jun 20, 2025 at 02:37:19PM +0000, Tanish Desai wrote:
> > > diff --git a/scripts/tracetool/format/h.py
> > b/scripts/tracetool/format/h.py
> > > index ea126b07ea..89d54b9aff 100644
> > > --- a/scripts/tracetool/format/h.py
> > > +++ b/scripts/tracetool/format/h.py
> > > @@ -76,13 +76,17 @@ def generate(events, backend, group):
> > >          out('',
> > >              'static inline void %(api)s(%(args)s)',
> > >              '{',
> > > -            '    if (%(cond)s) {',
> > > +            api=e.api(),
> > > +            args=e.args)
> > > +
> > > +        if "disable" not in e.properties:
> > > +            backend.generate_unconditional(e, group)
> > > +
> > > +        out('    if (%(cond)s) {',
> > >              '        %(api_nocheck)s(%(names)s);',
> > >              '    }',
> > >              '}',
> > > -            api=e.api(),
> > >              api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> > > -            args=e.args,
> > >              names=", ".join(e.args.names()),
> > >              cond=cond)
> >
> > Two thoughts:
> >
> > 1. nocheck isn't necessary anymore. The body of nocheck could be inlined
> >    here instead to simplify the generated code.
> >
> > 2. "if (%(cond)s) {" is only useful for backends that implement
> >    .generate(). For example, if only the dtrace backend is enabled then
> >    "if (trace_event_get_state(...)) {}" will be emitted unnecessarily.
> >
> > Maybe backends should have a .condition() interface so that
> > scripts/tracetool/format/h.py:generate() can first collect a dict[cond]
> > -> backend. Then it iterates over the map, calling backend.generate()
> > within "if (%(cond)s) { ... }". That way only the conditions that are
> > actually needed are generated and multiple backends that have the same
> > condition will share the same if statement.
> >
> > Stefan
> >

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

[PATCH v3] tracetool: generates conitional checks when needed

[Tanish Desai]

Adds generate_conditional, allowing backends to wrap generate()
output in a trace_event_get_state(...) check if needed.

Removes no_check by inlining its logic into trace_foo(...).
Also ensures the generated code is formatted properly.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
---
 scripts/tracetool/backend/__init__.py |  3 +++
 scripts/tracetool/backend/ftrace.py   | 16 +++++++-------
 scripts/tracetool/backend/log.py      | 26 +++++++++++++----------
 scripts/tracetool/backend/simple.py   |  4 ++++
 scripts/tracetool/backend/syslog.py   |  4 ++++
 scripts/tracetool/format/h.py         | 30 ++++++++++-----------------
 6 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
index c4456a5efd..dc0806f8d0 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -118,6 +118,9 @@ def generate_begin(self, events, group):
     def generate(self, event, group):
         self._run_function("generate_%s", event, group)
 
+    def generate_conditional(self, hasCondition):
+        self._run_function("generate_%s_conditional", hasCondition)
+    
     def generate_unconditional(self, event, group):
         self._run_function("generate_%s_unconditional", event, group)
     
diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index 2d6d608add..d579139532 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -30,17 +30,15 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    out('    {',
-        '        char ftrace_buf[MAX_TRACE_STRLEN];',
+    out('        char ftrace_buf[MAX_TRACE_STRLEN];',
         '        int unused __attribute__ ((unused));',
         '        int trlen;',
         '#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,',
+        '                         "%(name)s " %(fmt)s "\\n" %(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
-        '            unused = write(trace_marker_fd, ftrace_buf, trlen);',
-        '    }',
+        '        trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
+        '        unused = write(trace_marker_fd, ftrace_buf, trlen);',
         name=event.name,
         args=event.args,
         event_lineno=event.lineno,
@@ -52,3 +50,7 @@ def generate_h(event, group):
 def generate_h_backend_dstate(event, group):
     out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
         event_id="TRACE_" + event.name.upper())
+
+
+def generate_h_conditional(hasCondition):
+    hasCondition[0] = hasCondition[0] or True
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index 35a3aeee55..119e24af04 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -31,22 +31,22 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    out('    if (qemu_loglevel_mask(LOG_TRACE)) {',
-        '        if (message_with_timestamp) {',
-        '            struct timeval _now;',
-        '            gettimeofday(&_now, NULL);',
+    out('        if (qemu_loglevel_mask(LOG_TRACE)) {',
+        '            if (message_with_timestamp) {',
+        '                struct timeval _now;',
+        '                gettimeofday(&_now, NULL);',
         '#line %(event_lineno)d "%(event_filename)s"',
-        '            qemu_log("%%d@%%zu.%%06zu:%(name)s " %(fmt)s "\\n",',
-        '                     qemu_get_thread_id(),',
-        '                     (size_t)_now.tv_sec, (size_t)_now.tv_usec',
-        '                     %(argnames)s);',
+        '                qemu_log("%%d@%%zu.%%06zu:%(name)s " %(fmt)s "\\n",',
+        '                         qemu_get_thread_id(),',
+        '                         (size_t)_now.tv_sec, (size_t)_now.tv_usec',
+        '                         %(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '        } else {',
+        '            } else {',
         '#line %(event_lineno)d "%(event_filename)s"',
-        '            qemu_log("%(name)s " %(fmt)s "\\n"%(argnames)s);',
+        '                qemu_log("%(name)s " %(fmt)s "\\n"%(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
+        '            }',
         '        }',
-        '    }',
         event_lineno=event.lineno,
         event_filename=os.path.relpath(event.filename),
         name=event.name,
@@ -57,3 +57,7 @@ def generate_h(event, group):
 def generate_h_backend_dstate(event, group):
     out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
         event_id="TRACE_" + event.name.upper())
+
+    
+def generate_h_conditional(hasCondition):
+    hasCondition[0] = hasCondition[0] or True
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index ce8036f5da..316f39727b 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -97,3 +97,7 @@ def generate_c(event, group):
     out('    trace_record_finish(&rec);',
         '}',
         '')
+
+
+def generate_h_conditional(hasCondition):
+    hasCondition[0] = hasCondition[0] or True
diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
index f84cec641c..a224bd1922 100644
--- a/scripts/tracetool/backend/syslog.py
+++ b/scripts/tracetool/backend/syslog.py
@@ -43,3 +43,7 @@ def generate_h(event, group):
 def generate_h_backend_dstate(event, group):
     out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
         event_id="TRACE_" + event.name.upper())
+
+
+def generate_h_conditional(hasCondition):
+    hasCondition[0] = hasCondition[0] or True
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 7bbe6d3148..7a5a32d518 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -59,21 +59,10 @@ def generate(events, backend, group):
 
         out('    false)')
 
-        # tracer without checks
-        out('',
-            'static inline void %(api)s(%(args)s)',
-            '{',
-            api=e.api(e.QEMU_TRACE_NOCHECK),
-            args=e.args)
-
-        if "disable" not in e.properties:
-            backend.generate(e, group)
-
-        out('}')
-
         event_id = 'TRACE_' + e.name.upper()
         cond = "trace_event_get_state(%s)" % event_id
-
+        hasCondition = [False]
+        backend.generate_conditional(hasCondition)
         out('',
             'static inline void %(api)s(%(args)s)',
             '{',
@@ -83,13 +72,16 @@ def generate(events, backend, group):
         if "disable" not in e.properties:
             backend.generate_unconditional(e, group)
 
-        out('    if (%(cond)s) {',
-            '        %(api_nocheck)s(%(names)s);',
-            '    }',
-            '}',
-            api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
-            names=", ".join(e.args.names()),
+        if hasCondition[0]:
+            out('    if (%(cond)s) {',
             cond=cond)
+        
+        if "disable" not in e.properties:
+            backend.generate(e, group)
+
+        if hasCondition[0]:
+            out('    }')
+        out('}')
 
     backend.generate_end(events, group)
 
-- 
2.34.1

Re: [PATCH v3] tracetool: generates conitional checks when needed

[Stefan Hajnoczi]

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

On Wed, Jun 25, 2025 at 12:30:23PM +0000, Tanish Desai wrote:
> Adds generate_conditional, allowing backends to wrap generate()
> output in a trace_event_get_state(...) check if needed.
> 
> Removes no_check by inlining its logic into trace_foo(...).
> Also ensures the generated code is formatted properly.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> ---
>  scripts/tracetool/backend/__init__.py |  3 +++
>  scripts/tracetool/backend/ftrace.py   | 16 +++++++-------
>  scripts/tracetool/backend/log.py      | 26 +++++++++++++----------
>  scripts/tracetool/backend/simple.py   |  4 ++++
>  scripts/tracetool/backend/syslog.py   |  4 ++++
>  scripts/tracetool/format/h.py         | 30 ++++++++++-----------------
>  6 files changed, 46 insertions(+), 37 deletions(-)

This patch does not apply to qemu.git/master. When posting new revisions
of patches, please resend the entire patch series that this belongs to
or send it as a separate patch with the Based-on: <message-id> trailer
to let people (and tools) know which unmerged patch series it depends
on.

> 
> diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
> index c4456a5efd..dc0806f8d0 100644
> --- a/scripts/tracetool/backend/__init__.py
> +++ b/scripts/tracetool/backend/__init__.py
> @@ -118,6 +118,9 @@ def generate_begin(self, events, group):
>      def generate(self, event, group):
>          self._run_function("generate_%s", event, group)
>  
> +    def generate_conditional(self, hasCondition):
> +        self._run_function("generate_%s_conditional", hasCondition)
> +    
>      def generate_unconditional(self, event, group):
>          self._run_function("generate_%s_unconditional", event, group)
>      
> diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
> index 2d6d608add..d579139532 100644
> --- a/scripts/tracetool/backend/ftrace.py
> +++ b/scripts/tracetool/backend/ftrace.py
> @@ -30,17 +30,15 @@ def generate_h(event, group):
>      if len(event.args) > 0:
>          argnames = ", " + argnames
>  
> -    out('    {',
> -        '        char ftrace_buf[MAX_TRACE_STRLEN];',
> +    out('        char ftrace_buf[MAX_TRACE_STRLEN];',
>          '        int unused __attribute__ ((unused));',
>          '        int trlen;',
>          '#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,',
> +        '                         "%(name)s " %(fmt)s "\\n" %(argnames)s);',
>          '#line %(out_next_lineno)d "%(out_filename)s"',
> -        '            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
> -        '            unused = write(trace_marker_fd, ftrace_buf, trlen);',
> -        '    }',
> +        '        trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
> +        '        unused = write(trace_marker_fd, ftrace_buf, trlen);',
>          name=event.name,
>          args=event.args,
>          event_lineno=event.lineno,
> @@ -52,3 +50,7 @@ def generate_h(event, group):
>  def generate_h_backend_dstate(event, group):
>      out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
>          event_id="TRACE_" + event.name.upper())
> +
> +
> +def generate_h_conditional(hasCondition):
> +    hasCondition[0] = hasCondition[0] or True
> diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
> index 35a3aeee55..119e24af04 100644
> --- a/scripts/tracetool/backend/log.py
> +++ b/scripts/tracetool/backend/log.py
> @@ -31,22 +31,22 @@ def generate_h(event, group):
>      if len(event.args) > 0:
>          argnames = ", " + argnames
>  
> -    out('    if (qemu_loglevel_mask(LOG_TRACE)) {',
> -        '        if (message_with_timestamp) {',
> -        '            struct timeval _now;',
> -        '            gettimeofday(&_now, NULL);',
> +    out('        if (qemu_loglevel_mask(LOG_TRACE)) {',
> +        '            if (message_with_timestamp) {',
> +        '                struct timeval _now;',
> +        '                gettimeofday(&_now, NULL);',
>          '#line %(event_lineno)d "%(event_filename)s"',
> -        '            qemu_log("%%d@%%zu.%%06zu:%(name)s " %(fmt)s "\\n",',
> -        '                     qemu_get_thread_id(),',
> -        '                     (size_t)_now.tv_sec, (size_t)_now.tv_usec',
> -        '                     %(argnames)s);',
> +        '                qemu_log("%%d@%%zu.%%06zu:%(name)s " %(fmt)s "\\n",',
> +        '                         qemu_get_thread_id(),',
> +        '                         (size_t)_now.tv_sec, (size_t)_now.tv_usec',
> +        '                         %(argnames)s);',
>          '#line %(out_next_lineno)d "%(out_filename)s"',
> -        '        } else {',
> +        '            } else {',
>          '#line %(event_lineno)d "%(event_filename)s"',
> -        '            qemu_log("%(name)s " %(fmt)s "\\n"%(argnames)s);',
> +        '                qemu_log("%(name)s " %(fmt)s "\\n"%(argnames)s);',
>          '#line %(out_next_lineno)d "%(out_filename)s"',
> +        '            }',
>          '        }',
> -        '    }',
>          event_lineno=event.lineno,
>          event_filename=os.path.relpath(event.filename),
>          name=event.name,
> @@ -57,3 +57,7 @@ def generate_h(event, group):
>  def generate_h_backend_dstate(event, group):
>      out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
>          event_id="TRACE_" + event.name.upper())
> +
> +    
> +def generate_h_conditional(hasCondition):
> +    hasCondition[0] = hasCondition[0] or True
> diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
> index ce8036f5da..316f39727b 100644
> --- a/scripts/tracetool/backend/simple.py
> +++ b/scripts/tracetool/backend/simple.py
> @@ -97,3 +97,7 @@ def generate_c(event, group):
>      out('    trace_record_finish(&rec);',
>          '}',
>          '')
> +
> +
> +def generate_h_conditional(hasCondition):
> +    hasCondition[0] = hasCondition[0] or True
> diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
> index f84cec641c..a224bd1922 100644
> --- a/scripts/tracetool/backend/syslog.py
> +++ b/scripts/tracetool/backend/syslog.py
> @@ -43,3 +43,7 @@ def generate_h(event, group):
>  def generate_h_backend_dstate(event, group):
>      out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
>          event_id="TRACE_" + event.name.upper())
> +
> +
> +def generate_h_conditional(hasCondition):
> +    hasCondition[0] = hasCondition[0] or True
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index 7bbe6d3148..7a5a32d518 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -59,21 +59,10 @@ def generate(events, backend, group):
>  
>          out('    false)')
>  
> -        # tracer without checks
> -        out('',
> -            'static inline void %(api)s(%(args)s)',
> -            '{',
> -            api=e.api(e.QEMU_TRACE_NOCHECK),
> -            args=e.args)
> -
> -        if "disable" not in e.properties:
> -            backend.generate(e, group)
> -
> -        out('}')
> -
>          event_id = 'TRACE_' + e.name.upper()
>          cond = "trace_event_get_state(%s)" % event_id
> -
> +        hasCondition = [False]
> +        backend.generate_conditional(hasCondition)
>          out('',
>              'static inline void %(api)s(%(args)s)',
>              '{',
> @@ -83,13 +72,16 @@ def generate(events, backend, group):
>          if "disable" not in e.properties:
>              backend.generate_unconditional(e, group)
>  
> -        out('    if (%(cond)s) {',
> -            '        %(api_nocheck)s(%(names)s);',
> -            '    }',
> -            '}',
> -            api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> -            names=", ".join(e.args.names()),
> +        if hasCondition[0]:
> +            out('    if (%(cond)s) {',
>              cond=cond)
> +        
> +        if "disable" not in e.properties:
> +            backend.generate(e, group)
> +
> +        if hasCondition[0]:
> +            out('    }')
> +        out('}')

This doesn't handle cases where some backends (like dtrace) do not want
trace_event_get_state() but some backends (like simple) do. You can test
that case with ./configure --enable-trace-backends=dtrace,simple.

Both types of backends need to have their generate function
called separately:

   ...backends without the conditional...
   if (trace_event_get_state(...)) {
       ...backends with the conditional...
   }

The hasCondition list argument can be avoided by returning bool from
generate_h_conditional() instead of modifying the argument. That's a
little cleaner than the pass-by-reference trick where each backend has
to logical-or in their value.

The generate_h_conditional() function could also be replace with a
module variable like the existing PUBLIC variable. That way backends can
simply declare what they want instead of implementing a function:

  CHECK_TRACE_EVENT_GET_STATE = True # or False

And the code in h.py would know whether to call the generate function
inside the conditional or not. (I snuck in another suggestion: changing
the name from "conditional", which is a general term, to
"check_trace_event_get_state" to be more specific about what it does.)

Stefan

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

Re: [PATCH v3] tracetool: generates conitional checks when needed

[Tanish Desai]

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

>
>
> > This patch does not apply to qemu.git/master. When posting new revisions
> > of patches, please resend the entire patch series that this belongs to
> > or send it as a separate patch with the Based-on: <message-id> trailer
> > to let people (and tools) know which unmerged patch series it depends
> > on.
>

Noted


> > The hasCondition list argument can be avoided by returning bool from
> > generate_h_conditional() instead of modifying the argument. That's a
> > little cleaner than the pass-by-reference trick where each backend has
> > to logical-or in their value.
>

I thought of this earlier as well and even tried to implement but
the problem
with this is h.py calls backend.generate() which calls _run_function which
inturn loops it for all backends and runs the respective generate function.
Now if I want generate_h_conditional() to return that means I need to
add a return in _run_function(..) and also need to add a return in
generate_unconditional(..) as well.
Adding a return statement to _run_function(..) didn't make much sense
to me and then I would also need to find a way to merge all the return
value(from individual backend) as well so I skipped this approach.

> The generate_h_conditional() function could also be replace with a
> > module variable like the existing PUBLIC variable. That way backends can
> > simply declare what they want instead of implementing a function:
> >
> >  CHECK_TRACE_EVENT_GET_STATE = True # or False
> >
> > And the code in h.py would know whether to call the generate function
> > inside the conditional or not. (I snuck in another suggestion: changing
> > the name from "conditional", which is a general term, to
> > "check_trace_event_get_state" to be more specific about what it does.)
>

Yes, I can do this.
Wrapper Class has attribute check_trace_event_get_state which will
be *"or"* of all CHECK_TRACE_EVENT_GET_STATE which is derived
from getattr(module, "CHECK_TRACE_EVENT_GET_STATE", False)
and finally in h.py I will have code like this

backend.generate_unconditional(...);
> if backend.check_trace_event_get_state:
>    out("if(check_trace_event_get_state(...)){")
>         backend.generate_conditional(...);
>    out("}")

[-- Attachment #2: Type: text/html, Size: 4358 bytes --]

[PATCH v2 3/3] tracetool: remove redundant event_get_state checks

[Tanish Desai]

Moved trace_event_get_state check from _no_check_trace_foo to trace_foo,
and removed if (true) checks.

The _no_check_trace_foo now only emits backend-specific core
logic, avoiding trace event conditionals entirely.

This brings conditional logic in format/h.py, reducing duplication across
backends and simplifying their code generation. 

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
---
 scripts/tracetool/backend/ftrace.py | 3 ---
 scripts/tracetool/backend/log.py    | 5 +----
 scripts/tracetool/backend/simple.py | 7 +------
 scripts/tracetool/backend/syslog.py | 7 +------
 scripts/tracetool/format/h.py       | 3 ++-
 5 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index baed2ae61c..2d6d608add 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -34,18 +34,15 @@ def generate_h(event, group):
         '        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);',
         '#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(),
         event_lineno=event.lineno,
         event_filename=os.path.relpath(event.filename),
         fmt=event.fmt.rstrip("\n"),
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index f24acad74c..35a3aeee55 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -31,9 +31,7 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
-
-    out('    if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {',
+    out('    if (qemu_loglevel_mask(LOG_TRACE)) {',
         '        if (message_with_timestamp) {',
         '            struct timeval _now;',
         '            gettimeofday(&_now, NULL);',
@@ -49,7 +47,6 @@ def generate_h(event, group):
         '#line %(out_next_lineno)d "%(out_filename)s"',
         '        }',
         '    }',
-        cond=cond,
         event_lineno=event.lineno,
         event_filename=os.path.relpath(event.filename),
         name=event.name,
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 7c84c06b20..ce8036f5da 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -36,13 +36,8 @@ def generate_h_begin(events, group):
 
 
 def generate_h(event, group):
-    event_id = 'TRACE_' + event.name.upper()
-    cond = "trace_event_get_state(%s)" % event_id
-    out('    if (%(cond)s) {',
-        '        _simple_%(api)s(%(args)s);',
-        '    }',
+    out('        _simple_%(api)s(%(args)s);',
         api=event.api(),
-        cond=cond,
         args=", ".join(event.args.names()))
 
 
diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
index 6fdc1142fb..f84cec641c 100644
--- a/scripts/tracetool/backend/syslog.py
+++ b/scripts/tracetool/backend/syslog.py
@@ -30,14 +30,9 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
-
-    out('    if (%(cond)s) {',
-        '#line %(event_lineno)d "%(event_filename)s"',
+    out('#line %(event_lineno)d "%(event_filename)s"',
         '        syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '    }',
-        cond=cond,
         event_lineno=event.lineno,
         event_filename=os.path.relpath(event.filename),
         name=event.name,
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 89d54b9aff..7bbe6d3148 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -71,7 +71,8 @@ def generate(events, backend, group):
 
         out('}')
 
-        cond = "true"
+        event_id = 'TRACE_' + e.name.upper()
+        cond = "trace_event_get_state(%s)" % event_id
 
         out('',
             'static inline void %(api)s(%(args)s)',
-- 
2.34.1

Re: [PATCH v2 3/3] tracetool: remove redundant event_get_state checks

[Stefan Hajnoczi]

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

On Fri, Jun 20, 2025 at 02:37:20PM +0000, Tanish Desai wrote:
> diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
> index baed2ae61c..2d6d608add 100644
> --- a/scripts/tracetool/backend/ftrace.py
> +++ b/scripts/tracetool/backend/ftrace.py
> @@ -34,18 +34,15 @@ def generate_h(event, group):
>          '        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);',

Please unindent lines now that "if (trace_event_get_state(...)) {" is
gone and deeper indentation is not necessary anymore. There are more
instances below in this patch.

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

Re: [PATCH v2 0/3] tracetool: cleanup "if(true)" check from trace_foo()

[Tanish Desai]

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

Sorry forgot to CC Alex and Paolo .


On Fri, 20 Jun 2025 at 8:07 PM, Tanish Desai <tanishdesai37@gmail.com>
wrote:

> This series of patch aims to removes the leftover if (true) condition
> from trace_foo, a remnant from the TCG tracing feature removal.
>
> It replaces it with a proper trace_event_get_state(...)
> check where necessary (for log/simple/syslog and ftrace backend).
>
> Additionally, this change centralizes the generation of
> trace_event_get_state(...) calls into format/h.py,
> eliminating redundant code across individual backends.
>
> This cleanup results in more consistent and less
> repetitive backend code.
>
> Tanish Desai (3):
>   tracetool: removed the unused vcpu property
>   tracetool: introduce generate_unconditional
>   tracetool: remove redundant event_get_state checks
>
>  scripts/tracetool/__init__.py         |  6 +++---
>  scripts/tracetool/backend/__init__.py |  3 +++
>  scripts/tracetool/backend/dtrace.py   |  3 ++-
>  scripts/tracetool/backend/ftrace.py   |  3 ---
>  scripts/tracetool/backend/log.py      |  9 +--------
>  scripts/tracetool/backend/simple.py   | 11 +----------
>  scripts/tracetool/backend/syslog.py   | 11 +----------
>  scripts/tracetool/backend/ust.py      |  2 +-
>  scripts/tracetool/format/h.py         | 13 +++++++++----
>  9 files changed, 21 insertions(+), 40 deletions(-)
>
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 1902 bytes --]