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.1On 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
> 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
>
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
> >
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.1On 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
>
>
> > 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("}")
© 2016 - 2025 Red Hat, Inc.