Re: [Qemu-devel] [PATCH 4/4] trace: [tcg] Do not generate TCG code to trace dinamically-disabled events

[Lluís Vilanova]

Daniel P Berrange writes:

> On Wed, Sep 14, 2016 at 11:23:38PM +0200, Lluís Vilanova wrote:
>> If an event is dynamically disabled, the TCG code that calls the
>> execution-time tracer is not generated.
>> 
>> Removes the overheads of execution-time tracers for dynamically disabled
>> events. As a bonus, also avoids checking the event state when the
>> execution-time tracer is called from TCG-generated code (since otherwise
>> TCG would simply not call it).
>> 
>> Signed-off-by: Lluís Vilanova <address@hidden>
>> ---
>> scripts/tracetool/backend/dtrace.py      |    2 +-
>> scripts/tracetool/backend/ftrace.py      |   20 ++++++++++----------
>> scripts/tracetool/backend/log.py         |   16 ++++++++--------
>> scripts/tracetool/backend/simple.py      |    2 +-
>> scripts/tracetool/backend/syslog.py      |    6 +++---
>> scripts/tracetool/backend/ust.py         |    2 +-
>> scripts/tracetool/format/h.py            |   23 +++++++++++++++++------
>> scripts/tracetool/format/tcg_h.py        |   20 +++++++++++++++++---
>> scripts/tracetool/format/tcg_helper_c.py |    3 ++-
>> 9 files changed, 60 insertions(+), 34 deletions(-)
>> 
>> diff --git a/scripts/tracetool/backend/dtrace.py 
>> b/scripts/tracetool/backend/dtrace.py
>> index ab9ecfa..20242f2 100644
>> --- a/scripts/tracetool/backend/dtrace.py
>> +++ b/scripts/tracetool/backend/dtrace.py
>> @@ -41,6 +41,6 @@ def generate_h_begin(events):
>> 
>> 
>> def generate_h(event):
>> -    out('        QEMU_%(uppername)s(%(argnames)s);',
>> +    out('    QEMU_%(uppername)s(%(argnames)s);',
>> uppername=event.name.upper(),
>> argnames=", ".join(event.args.names()))
>> diff --git a/scripts/tracetool/backend/ftrace.py 
>> b/scripts/tracetool/backend/ftrace.py
>> index 80dcf30..d798c71 100644
>> --- a/scripts/tracetool/backend/ftrace.py
>> +++ b/scripts/tracetool/backend/ftrace.py
>> @@ -30,17 +30,17 @@ def generate_h(event):
>> if len(event.args) > 0:
>> argnames = ", " + argnames
>> 
>> -    out('        {',
>> -        '            char ftrace_buf[MAX_TRACE_STRLEN];',
>> -        '            int unused __attribute__ ((unused));',
>> -        '            int trlen;',
>> -        '            if (trace_event_get_state(%(event_id)s)) {',
>> -        '                trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
>> -        '                                 "%(name)s " %(fmt)s "\\n" 
>> %(argnames)s);',
>> -        '                trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
>> -        '                unused = write(trace_marker_fd, ftrace_buf, 
>> trlen);',
>> -        '            }',
>> +    out('    {',
>> +        '        char ftrace_buf[MAX_TRACE_STRLEN];',
>> +        '        int unused __attribute__ ((unused));',
>> +        '        int trlen;',
>> +        '        if (trace_event_get_state(%(event_id)s)) {',
>> +        '            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
>> +        '                             "%(name)s " %(fmt)s "\\n" 
>> %(argnames)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(),
>> diff --git a/scripts/tracetool/backend/log.py 
>> b/scripts/tracetool/backend/log.py
>> index b3ff064..6818147 100644
>> --- a/scripts/tracetool/backend/log.py
>> +++ b/scripts/tracetool/backend/log.py
>> @@ -36,14 +36,14 @@ def generate_h(event):
>> else:
>> cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
>> 
>> -    out('        if (%(cond)s) {',
>> -        '            struct timeval _now;',
>> -        '            gettimeofday(&_now, NULL);',
>> -        '            qemu_log_mask(LOG_TRACE, "address@hidden:%(name)s " 
>> %(fmt)s "\\n",',
>> -        '                          getpid(),',
>> -        '                          (size_t)_now.tv_sec, 
>> (size_t)_now.tv_usec',
>> -        '                          %(argnames)s);',
>> -        '        }',
>> +    out('    if (%(cond)s) {',
>> +        '        struct timeval _now;',
>> +        '        gettimeofday(&_now, NULL);',
>> +        '        qemu_log_mask(LOG_TRACE, "address@hidden:%(name)s " 
>> %(fmt)s "\\n",',
>> +        '                      getpid(),',
>> +        '                      (size_t)_now.tv_sec, (size_t)_now.tv_usec',
>> +        '                      %(argnames)s);',
>> +        '    }',
>> cond=cond,
>> name=event.name,
>> fmt=event.fmt.rstrip("\n"),
>> diff --git a/scripts/tracetool/backend/simple.py 
>> b/scripts/tracetool/backend/simple.py
>> index 1bccada..4acf23f 100644
>> --- a/scripts/tracetool/backend/simple.py
>> +++ b/scripts/tracetool/backend/simple.py
>> @@ -36,7 +36,7 @@ def generate_h_begin(events):
>> 
>> 
>> def generate_h(event):
>> -    out('        _simple_%(api)s(%(args)s);',
>> +    out('    _simple_%(api)s(%(args)s);',
>> api=event.api(),
>> args=", ".join(event.args.names()))
>> 
>> diff --git a/scripts/tracetool/backend/syslog.py 
>> b/scripts/tracetool/backend/syslog.py
>> index 89019bc..b355121 100644
>> --- a/scripts/tracetool/backend/syslog.py
>> +++ b/scripts/tracetool/backend/syslog.py
>> @@ -36,9 +36,9 @@ def generate_h(event):
>> else:
>> cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
>> 
>> -    out('        if (%(cond)s) {',
>> -        '            syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
>> -        '        }',
>> +    out('    if (%(cond)s) {',
>> +        '        syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
>> +        '    }',
>> cond=cond,
>> name=event.name,
>> fmt=event.fmt.rstrip("\n"),
>> diff --git a/scripts/tracetool/backend/ust.py 
>> b/scripts/tracetool/backend/ust.py
>> index ed4c227..88d13e2 100644
>> --- a/scripts/tracetool/backend/ust.py
>> +++ b/scripts/tracetool/backend/ust.py
>> @@ -30,6 +30,6 @@ def generate_h(event):
>> if len(event.args) > 0:
>> argnames = ", " + argnames
>> 
>> -    out('        tracepoint(qemu, %(name)s%(tp_args)s);',
>> +    out('    tracepoint(qemu, %(name)s%(tp_args)s);',
>> name=event.name,
>> tp_args=argnames)


> All the stylistic whitespace changes should be done as a separate
> patch from the the functional changes.

Ok!