[Qemu-devel] [PATCH 1/6] trace: [tracetool] Do not rebuild event list in backend code

[Qemu-devel] [PATCH 1/6] trace: [tracetool] Do not rebuild event list in backend code

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 6874f66..80e5684 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -171,15 +171,14 @@ def simple_c(events):
     print
     print 'TraceEvent trace_list[] = {'
     print
-    eventlist = list(events)
-    for event in eventlist:
+    for event in events:
         print '{.tp_name = "%(name)s", .state=0},' % {
     'name': event.name
 }
         print
     print '};'
     print
-    for event in eventlist:
+    for event in events:
         argc = event.argc
         print '''void trace_%(name)s(%(args)s)
 {
@@ -311,8 +310,7 @@ def ust_c(events):
 #undef inline
 #undef wmb
 #include "trace.h"'''
-    eventlist = list(events)
-    for event in eventlist:
+    for event in events:
         argnames = event.argnames
         if event.argc > 0:
             argnames = ', ' + event.argnames
@@ -344,7 +342,7 @@ static void ust_%(name)s_probe(%(args)s)
     print '''
 static void __attribute__((constructor)) trace_init(void)
 {'''
-    for event in eventlist:
+    for event in events:
         print '    register_trace_ust_%(name)s(ust_%(name)s_probe);' % {
     'name': event.name
 }
@@ -510,14 +508,16 @@ class Event(object):
 
 # Generator that yields Event objects given a trace-events file object
 def read_events(fobj):
+    res = []
     event_num = 0
     for line in fobj:
         if not line.strip():
             continue
         if line.lstrip().startswith('#'):
 	    continue
-	yield Event(event_num, line)
+	res.append(Event(event_num, line))
 	event_num += 1
+    return res
 
 backend = ""
 output = ""

[Qemu-devel] [PATCH 2/6] trace: [tracetool] Simplify event line parsing

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py |   46 ++++++++++++++--------------------------------
 1 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 80e5684..7a877dc 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -38,19 +38,9 @@ Options:
 '''
     sys.exit(1)
 
-def get_name(line, sep='('):
-    head, sep, tail = line.partition(sep)
-    return head
-
-def get_args(line, sep1='(', sep2=')'):
-    head, sep1, tail = line.partition(sep1)
-    args, sep2, fmt_str = tail.partition(sep2)
-    return args
-
-def get_argnames(line, sep=','):
+def get_argnames(args):
     nfields = 0
     str = []
-    args = get_args(line)
     for field in args.split():
       nfields = nfields + 1
       # Drop pointer star
@@ -71,21 +61,7 @@ def get_argnames(line, sep=','):
     else:
       return ''
 
-def get_argc(line):
-    argc = 0
-    argnames = get_argnames(line)
-    if argnames:
-      for name in argnames.split(','):
-        argc = argc + 1
-    return argc
-
-def get_fmt(line, sep=')'):
-    event, sep, fmt = line.partition(sep)
-    return fmt
-
-def calc_sizeofargs(line):
-    args = get_args(line)
-    argc = get_argc(line)
+def calc_sizeofargs(args, argc):
     strtype = ('const char*', 'char*', 'const char *', 'char *')
     str = []
     newstr = ""
@@ -495,16 +471,22 @@ trace_gen = {
 }
 
 # A trace event
+import re
+cre = re.compile("(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*\")?")
+
 class Event(object):
     def __init__(self, num, line):
         self.num = num
-        self.args = get_args(line)
+        m = cre.match(line)
+        assert m is not None
+        groups = m.groupdict('')
+        self.args = groups["args"]
         self.arglist = self.args.split(',')
-        self.name = get_name(line)
-        self.argc = get_argc(line)
-        self.argnames = get_argnames(line)
-        self.sizestr = calc_sizeofargs(line)
-        self.fmt = get_fmt(line)
+        self.name = groups["name"]
+        self.argc = len(self.arglist)
+        self.argnames = get_argnames(self.args)
+        self.sizestr = calc_sizeofargs(self.args, self.argc)
+        self.fmt = groups["fmt"]
 
 # Generator that yields Event objects given a trace-events file object
 def read_events(fobj):

[Qemu-devel] [PATCH 3/6] trace: [ŧracetool] Do not precompute the event number

[Lluís Vilanova]

This would otherwise break event numbering when actually using the "disable"
property.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 7a877dc..b7401a3 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -128,7 +128,7 @@ def simple_h(events):
     'args': event.args
 }
     print
-    print '#define NR_TRACE_EVENTS %d' % (event.num + 1)
+    print '#define NR_TRACE_EVENTS %d' % len(events)
     print 'extern TraceEvent trace_list[NR_TRACE_EVENTS];'
 
     return
@@ -154,7 +154,7 @@ def simple_c(events):
         print
     print '};'
     print
-    for event in events:
+    for num, event in enumerate(events):
         argc = event.argc
         print '''void trace_%(name)s(%(args)s)
 {
@@ -169,12 +169,12 @@ def simple_c(events):
 ''' % {
     'name': event.name,
     'args': event.args,
-    'event_id': event.num,
+    'event_id': num,
 }
         print '''
     tbuf_idx = trace_alloc_record(%(event_id)s, %(sizestr)s);
     rec_off = (tbuf_idx + ST_V2_REC_HDR_LEN) %% TRACE_BUF_LEN; /* seek record header */
-''' % {'event_id': event.num, 'sizestr': event.sizestr,}
+''' % {'event_id': num, 'sizestr': event.sizestr,}
 
         if argc > 0:
             str = event.arglist
@@ -220,7 +220,7 @@ def stderr_h(events):
 #include "trace/stderr.h"
 
 extern TraceEvent trace_list[];'''
-    for event in events:
+    for num, event in enumerate(events):
         argnames = event.argnames
         if event.argc > 0:
             argnames = ', ' + event.argnames
@@ -235,12 +235,12 @@ static inline void trace_%(name)s(%(args)s)
 }''' % {
     'name': event.name,
     'args': event.args,
-    'event_num': event.num,
+    'event_num': num,
     'fmt': event.fmt.rstrip('\n'),
     'argnames': argnames
 }
     print
-    print '#define NR_TRACE_EVENTS %d' % (event.num + 1)
+    print '#define NR_TRACE_EVENTS %d' % len(events)
 
 def stderr_c(events):
     print '''#include "trace.h"
@@ -475,8 +475,7 @@ import re
 cre = re.compile("(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*\")?")
 
 class Event(object):
-    def __init__(self, num, line):
-        self.num = num
+    def __init__(self, line):
         m = cre.match(line)
         assert m is not None
         groups = m.groupdict('')
@@ -491,14 +490,12 @@ class Event(object):
 # Generator that yields Event objects given a trace-events file object
 def read_events(fobj):
     res = []
-    event_num = 0
     for line in fobj:
         if not line.strip():
             continue
         if line.lstrip().startswith('#'):
 	    continue
-	res.append(Event(event_num, line))
-	event_num += 1
+	res.append(Event(line))
     return res
 
 backend = ""

[Qemu-devel] [PATCH 4/6] trace: [tracetool] Add support for event properties

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index b7401a3..e3e665d 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -472,7 +472,7 @@ trace_gen = {
 
 # A trace event
 import re
-cre = re.compile("(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*\")?")
+cre = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*\")?")
 
 class Event(object):
     def __init__(self, line):
@@ -486,6 +486,7 @@ class Event(object):
         self.argnames = get_argnames(self.args)
         self.sizestr = calc_sizeofargs(self.args, self.argc)
         self.fmt = groups["fmt"]
+        self.properties = groups["props"].split()
 
 # Generator that yields Event objects given a trace-events file object
 def read_events(fobj):

[Qemu-devel] [PATCH 5/6] trace: [tracetool] Process the "disable" event property

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py |   13 +++++++++++--
 trace-events         |    2 +-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index e3e665d..5432f61 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -365,6 +365,9 @@ def dtrace_d(events):
     print '};'
     return
 
+def dtrace_nop_d(events):
+    pass
+
 def dtrace_stp(events):
     for event in events:
         # Define prototype for probe arguments
@@ -387,6 +390,9 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
     print
     return
 
+def dtrace_nop_stp(events):
+    pass
+
 def trace_stap_begin():
     global probeprefix
     if backend != "dtrace":
@@ -429,6 +435,8 @@ converters = {
     'nop': {
         'h': nop_h,
         'c': nop_c,
+        'd': dtrace_nop_d,
+        'stap': dtrace_nop_stp,
     },
 
     'stderr': {
@@ -555,10 +563,11 @@ def main():
         sys.exit(0)
 
     events = read_events(sys.stdin)
+
     trace_gen[output]['begin']()
-    converters[backend][output](events)
+    converters[backend][output]([ e for e in events if 'disable' not in e.properties ])
+    converters['nop'][output]([ e for e in events if 'disable' in e.properties ])
     trace_gen[output]['end']()
-    return
 
 if __name__ == "__main__":
     main()
diff --git a/trace-events b/trace-events
index 514849a..ac511ae 100644
--- a/trace-events
+++ b/trace-events
@@ -636,4 +636,4 @@ dma_bdrv_io(void *dbs, void *bs, int64_t sector_num, bool to_dev) "dbs=%p bs=%p
 dma_aio_cancel(void *dbs) "dbs=%p"
 dma_complete(void *dbs, int ret, void *cb) "dbs=%p ret=%d cb=%p"
 dma_bdrv_cb(void *dbs, int ret) "dbs=%p ret=%d"
-dma_map_wait(void *dbs) "dbs=%p"
+disable dma_map_wait(void *dbs) "dbs=%p"

[Qemu-devel] [PATCH 6/6] trace: [tracetool] Rewrite event argument parsing

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py |  186 ++++++++++++++++++++++++--------------------------
 1 files changed, 90 insertions(+), 96 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 5432f61..a9020de 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -38,49 +38,6 @@ Options:
 '''
     sys.exit(1)
 
-def get_argnames(args):
-    nfields = 0
-    str = []
-    for field in args.split():
-      nfields = nfields + 1
-      # Drop pointer star
-      type, ptr, tail = field.partition('*')
-      if type != field:
-        field = tail
-
-      name, sep, tail = field.partition(',')
-
-      if name == field:
-        continue
-      str.append(name)
-      str.append(", ")
-
-    if nfields > 1:
-      str.append(name)
-      return ''.join(str)
-    else:
-      return ''
-
-def calc_sizeofargs(args, argc):
-    strtype = ('const char*', 'char*', 'const char *', 'char *')
-    str = []
-    newstr = ""
-    if argc > 0:
-      str = args.split(',')
-      for elem in str:
-        if elem.lstrip().startswith(strtype): #strings
-          type, sep, var = elem.rpartition('*')
-          newstr = newstr+"4 + strlen("+var.lstrip()+") + "
-        #elif '*' in elem:
-        #  newstr = newstr + "4 + " # pointer vars
-        else:
-          #type, sep, var = elem.rpartition(' ')
-          #newstr = newstr+"sizeof("+type.lstrip()+") + "
-          newstr = newstr + '8 + '
-    newstr = newstr + '0' # for last +
-    return newstr
-
-
 def trace_h_begin():
     print '''#ifndef TRACE_H
 #define TRACE_H
@@ -133,13 +90,6 @@ def simple_h(events):
 
     return
 
-def is_string(arg):
-    strtype = ('const char*', 'char*', 'const char *', 'char *')
-    if arg.lstrip().startswith(strtype):
-        return True
-    else:
-        return False
-
 def simple_c(events):
     rec_off = 0
     print '#include "trace.h"'
@@ -154,8 +104,16 @@ def simple_c(events):
         print
     print '};'
     print
+
     for num, event in enumerate(events):
-        argc = event.argc
+        sizes = []
+        for type_, name in event.args:
+            if type_is_string(type_):
+                sizes.append("4 + strlen(%s)" % name)
+            else:
+                sizes.append("8 + sizeof(%s)" % type_)
+        sizestr = " + ".join(sizes)
+
         print '''void trace_%(name)s(%(args)s)
 {
     unsigned int tbuf_idx, rec_off;
@@ -166,52 +124,52 @@ def simple_c(events):
     if (!trace_list[%(event_id)s].state) {
         return;
     }
+
+    tbuf_idx = trace_alloc_record(%(event_id)s, %(sizestr)s);
+    rec_off = (tbuf_idx + ST_V2_REC_HDR_LEN) %% TRACE_BUF_LEN; /* seek record header */
 ''' % {
     'name': event.name,
     'args': event.args,
     'event_id': num,
+    'sizestr' : sizestr,
 }
-        print '''
-    tbuf_idx = trace_alloc_record(%(event_id)s, %(sizestr)s);
-    rec_off = (tbuf_idx + ST_V2_REC_HDR_LEN) %% TRACE_BUF_LEN; /* seek record header */
-''' % {'event_id': num, 'sizestr': event.sizestr,}
 
-        if argc > 0:
-            str = event.arglist
-            for elem in str:
-                if is_string(elem): # if string
-                    type, sep, var = elem.rpartition('*')
+        if len(event.args) > 0:
+            for type_, name in event.args:
+                # string
+                if type_is_string(type_):
                     print '''
-    slen = strlen(%(var)s);
+    slen = strlen(%(name)s);
     write_to_buffer(rec_off, (uint8_t*)&slen, sizeof(slen));
     rec_off += sizeof(slen);''' % {
-    'var': var.lstrip()
+    'name': name
 }
                     print '''
-    write_to_buffer(rec_off, (uint8_t*)%(var)s, slen);
+    write_to_buffer(rec_off, (uint8_t*)%(name)s, slen);
     rec_off += slen;''' % {
-    'var': var.lstrip()
+    'name': name
 }
-                elif '*' in elem: # pointer var (not string)
-                    type, sep, var = elem.rpartition('*')
+                # pointer var (not string)
+                elif type_.endswith('*'):
                     print '''
-    pvar64 = (uint64_t)(uint64_t*)%(var)s;
+    pvar64 = (uint64_t)(uint64_t*)%(name)s;
     write_to_buffer(rec_off, (uint8_t*)&pvar64, sizeof(uint64_t));
     rec_off += sizeof(uint64_t);''' % {
-    'var': var.lstrip()
+    'name': name
 }
-                else: # primitive data type
-                    type, sep, var = elem.rpartition(' ')
+                # primitive data type
+                else:
                     print '''
-    var64 = (uint64_t)%(var)s;
+    var64 = (uint64_t)%(name)s;
     write_to_buffer(rec_off, (uint8_t*)&var64, sizeof(uint64_t));
     rec_off += sizeof(uint64_t);''' % {
-    'var': var.lstrip()
+    'name': name
 }
         print '''
-    trace_mark_record_complete(tbuf_idx);'''
-        print '}'
-        print
+    trace_mark_record_complete(tbuf_idx);
+}
+
+'''
 
     return
 
@@ -220,12 +178,11 @@ def stderr_h(events):
 #include "trace/stderr.h"
 
 extern TraceEvent trace_list[];'''
+
     for num, event in enumerate(events):
-        argnames = event.argnames
-        if event.argc > 0:
-            argnames = ', ' + event.argnames
-        else:
-            argnames = ''
+        argnames = ", ".join(event.args.names())
+        if len(event.args) > 0:
+            argnames = ", "+argnames
         print '''
 static inline void trace_%(name)s(%(args)s)
 {
@@ -262,13 +219,13 @@ def ust_h(events):
 #undef wmb'''
 
     for event in events:
-        if event.argc > 0:
+        if len(event.args) > 0:
             print '''
 DECLARE_TRACE(ust_%(name)s, TP_PROTO(%(args)s), TP_ARGS(%(argnames)s));
 #define trace_%(name)s trace_ust_%(name)s''' % {
     'name': event.name,
     'args': event.args,
-    'argnames': event.argnames
+    'argnames': ", ".join(event.args.names())
 }
         else:
             print '''
@@ -287,9 +244,9 @@ def ust_c(events):
 #undef wmb
 #include "trace.h"'''
     for event in events:
-        argnames = event.argnames
-        if event.argc > 0:
-            argnames = ', ' + event.argnames
+        argnames = ", ".join(event.args.names())
+        if len(event.args) > 0:
+            argnames = ', ' + argnames
             print '''
 DEFINE_TRACE(ust_%(name)s);
 
@@ -339,7 +296,7 @@ def dtrace_h(events):
     'name': event.name,
     'args': event.args,
     'uppername': event.name.upper(),
-    'argnames': event.argnames,
+    'argnames': ", ".join(event.args.names()),
 }
 
 def dtrace_c(events):
@@ -379,12 +336,12 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
     'binary': binary
 }
         i = 1
-        if event.argc > 0:
-            for arg in event.argnames.split(','):
+        if len(event.args) > 0:
+            for name in event.args.names():
                 # 'limit' is a reserved keyword
-                if arg == 'limit':
-                    arg = '_limit'
-                print '  %s = $arg%d;' % (arg.lstrip(), i)
+                if name == 'limit':
+                    name = '_limit'
+                print '  %s = $arg%d;' % (name.lstrip(), i)
                 i += 1
         print '}'
     print
@@ -478,6 +435,47 @@ trace_gen = {
     },
 }
 
+# Event arguments
+def type_is_string(type_):
+    strtype = ('const char*', 'char*', 'const char *', 'char *')
+    return type_.startswith(strtype)
+
+class Arguments:
+    def __init__ (self, arg_str):
+        self._args = []
+        for arg in arg_str.split(","):
+            arg = arg.strip()
+            parts = arg.split()
+            head, sep, tail = parts[-1].rpartition("*")
+            parts = parts[:-1]
+            if tail == "void":
+                assert len(parts) == 0 and sep == ""
+                continue
+            arg_type = " ".join(parts + [ " ".join([head, sep]).strip() ]).strip()
+            self._args.append((arg_type, tail))
+
+    def __iter__(self):
+        return iter(self._args)
+
+    def __len__(self):
+        return len(self._args)
+
+    def __str__(self):
+        if len(self._args) == 0:
+            return "void"
+        else:
+            return ", ".join([ " ".join([t, n]) for t,n in self._args ])
+
+    def names(self):
+        return [ name for _, name in self._args ]
+
+    def types(self):
+        return [ type_ for type_, _ in self._args ]
+
+    def size_str(self):
+        res = ""
+        return res
+
 # A trace event
 import re
 cre = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*\")?")
@@ -487,14 +485,10 @@ class Event(object):
         m = cre.match(line)
         assert m is not None
         groups = m.groupdict('')
-        self.args = groups["args"]
-        self.arglist = self.args.split(',')
         self.name = groups["name"]
-        self.argc = len(self.arglist)
-        self.argnames = get_argnames(self.args)
-        self.sizestr = calc_sizeofargs(self.args, self.argc)
         self.fmt = groups["fmt"]
         self.properties = groups["props"].split()
+        self.args = Arguments(groups["args"])
 
 # Generator that yields Event objects given a trace-events file object
 def read_events(fobj):

[Qemu-devel] [PATH 7/6] trace: [tracetool] Make format-specific code optional and with access to event information

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py |   35 ++++++++++-------------------------
 1 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index a9020de..ddf4c31 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -38,26 +38,19 @@ Options:
 '''
     sys.exit(1)
 
-def trace_h_begin():
+def trace_h_begin(events):
     print '''#ifndef TRACE_H
 #define TRACE_H
 
 /* This file is autogenerated by tracetool, do not edit. */
 
 #include "qemu-common.h"'''
-    return
 
-def trace_h_end():
+def trace_h_end(events):
     print '#endif /* TRACE_H */'
-    return
 
-def trace_c_begin():
+def trace_c_begin(events):
     print '/* This file is autogenerated by tracetool, do not edit. */'
-    return
-
-def trace_c_end():
-    # nop, required for trace_gen
-    return
 
 def nop_h(events):
     print
@@ -350,7 +343,7 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
 def dtrace_nop_stp(events):
     pass
 
-def trace_stap_begin():
+def trace_stap_begin(events):
     global probeprefix
     if backend != "dtrace":
         print 'SystemTAP tapset generator not applicable to %s backend' % backend
@@ -367,20 +360,13 @@ def trace_stap_begin():
     if probeprefix == "":
         probeprefix = 'qemu.' + targettype + '.' + targetarch
     print '/* This file is autogenerated by tracetool, do not edit. */'
-    return
 
-def trace_stap_end():
-    return #nop, reqd for trace_gen
-
-def trace_d_begin():
+def trace_d_begin(events):
     if backend != 'dtrace':
         print 'DTrace probe generator not applicable to %s backend' % backend
         sys.exit(1)
     print '/* This file is autogenerated by tracetool, do not edit. */'
 
-def trace_d_end():
-    return #nop, reqd for trace_gen
-
 
 # Registry of backends and their converter functions
 converters = {
@@ -416,22 +402,19 @@ converters = {
 }
 
 # Trace file header and footer code generators
-trace_gen = {
+formats = {
     'h': {
         'begin': trace_h_begin,
         'end': trace_h_end,
     },
     'c': {
         'begin': trace_c_begin,
-        'end': trace_c_end,
     },
     'd': {
         'begin': trace_d_begin,
-        'end': trace_d_end,
     },
     'stap': {
         'begin': trace_stap_begin,
-        'end': trace_stap_end,
     },
 }
 
@@ -558,10 +541,12 @@ def main():
 
     events = read_events(sys.stdin)
 
-    trace_gen[output]['begin']()
+    if 'begin' in formats[output]:
+        formats[output]['begin'](events)
     converters[backend][output]([ e for e in events if 'disable' not in e.properties ])
     converters['nop'][output]([ e for e in events if 'disable' in e.properties ])
-    trace_gen[output]['end']()
+    if 'end' in formats[output]:
+        formats[output]['end'](events)
 
 if __name__ == "__main__":
     main()

Re: [Qemu-devel] [PATCH 1/6] trace: [tracetool] Do not rebuild event list in backend code

[Harsh Bora]

On 01/11/2012 11:35 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
> ---
>   scripts/tracetool.py |   14 +++++++-------
>   1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index 6874f66..80e5684 100755
> --- a/scripts/tracetool.py
> +++ b/scripts/tracetool.py

[.. snip ..]

> @@ -510,14 +508,16 @@ class Event(object):
>
>   # Generator that yields Event objects given a trace-events file object
>   def read_events(fobj):
> +    res = []
>       event_num = 0
>       for line in fobj:
>           if not line.strip():
>               continue
>           if line.lstrip().startswith('#'):
>   	    continue
> -	yield Event(event_num, line)
> +	res.append(Event(event_num, line))
>   	event_num += 1
> +    return res
>

Hi Lluis,
This looks really nice. I can include your fixes while addressing other 
review comments. Shall I fold your patches with mine or do you want to 
keep them separate?

regards,
Harsh



>   backend = ""
>   output = ""
>
>

Re: [Qemu-devel] [PATCH 3/6] trace: [ŧracetool] Do not precompute the event number

[Harsh Bora]

On 01/11/2012 11:35 PM, Lluís Vilanova wrote:
> This would otherwise break event numbering when actually using the "disable"
> property.
>

IIUC, event numbering does include disabled events too, are you 
proposing not to include disabled events in event numbering ? That will 
affect interpreting event numbers while reading trace logs also.

- Harsh


> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
> ---
>   scripts/tracetool.py |   21 +++++++++------------
>   1 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index 7a877dc..b7401a3 100755
> --- a/scripts/tracetool.py
> +++ b/scripts/tracetool.py
> @@ -128,7 +128,7 @@ def simple_h(events):
>       'args': event.args
>   }
>       print
> -    print '#define NR_TRACE_EVENTS %d' % (event.num + 1)
> +    print '#define NR_TRACE_EVENTS %d' % len(events)
>       print 'extern TraceEvent trace_list[NR_TRACE_EVENTS];'
>
>       return
> @@ -154,7 +154,7 @@ def simple_c(events):
>           print
>       print '};'
>       print
> -    for event in events:
> +    for num, event in enumerate(events):
>           argc = event.argc
>           print '''void trace_%(name)s(%(args)s)
>   {
> @@ -169,12 +169,12 @@ def simple_c(events):
>   ''' % {
>       'name': event.name,
>       'args': event.args,
> -    'event_id': event.num,
> +    'event_id': num,
>   }
>           print '''
>       tbuf_idx = trace_alloc_record(%(event_id)s, %(sizestr)s);
>       rec_off = (tbuf_idx + ST_V2_REC_HDR_LEN) %% TRACE_BUF_LEN; /* seek record header */
> -''' % {'event_id': event.num, 'sizestr': event.sizestr,}
> +''' % {'event_id': num, 'sizestr': event.sizestr,}
>
>           if argc>  0:
>               str = event.arglist
> @@ -220,7 +220,7 @@ def stderr_h(events):
>   #include "trace/stderr.h"
>
>   extern TraceEvent trace_list[];'''
> -    for event in events:
> +    for num, event in enumerate(events):
>           argnames = event.argnames
>           if event.argc>  0:
>               argnames = ', ' + event.argnames
> @@ -235,12 +235,12 @@ static inline void trace_%(name)s(%(args)s)
>   }''' % {
>       'name': event.name,
>       'args': event.args,
> -    'event_num': event.num,
> +    'event_num': num,
>       'fmt': event.fmt.rstrip('\n'),
>       'argnames': argnames
>   }
>       print
> -    print '#define NR_TRACE_EVENTS %d' % (event.num + 1)
> +    print '#define NR_TRACE_EVENTS %d' % len(events)
>
>   def stderr_c(events):
>       print '''#include "trace.h"
> @@ -475,8 +475,7 @@ import re
>   cre = re.compile("(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*\")?")
>
>   class Event(object):
> -    def __init__(self, num, line):
> -        self.num = num
> +    def __init__(self, line):
>           m = cre.match(line)
>           assert m is not None
>           groups = m.groupdict('')
> @@ -491,14 +490,12 @@ class Event(object):
>   # Generator that yields Event objects given a trace-events file object
>   def read_events(fobj):
>       res = []
> -    event_num = 0
>       for line in fobj:
>           if not line.strip():
>               continue
>           if line.lstrip().startswith('#'):
>   	    continue
> -	res.append(Event(event_num, line))
> -	event_num += 1
> +	res.append(Event(line))
>       return res
>
>   backend = ""
>
>

Re: [Qemu-devel] [PATCH 3/6] trace: [ŧracetool] Do not precompute the event number

[Stefan Hajnoczi]

On Wed, Jan 18, 2012 at 9:48 AM, Harsh Bora <harsh@linux.vnet.ibm.com> wrote:
> On 01/11/2012 11:35 PM, Lluís Vilanova wrote:
>>
>> This would otherwise break event numbering when actually using the
>> "disable"
>> property.
>>
>
> IIUC, event numbering does include disabled events too, are you proposing
> not to include disabled events in event numbering ? That will affect
> interpreting event numbers while reading trace logs also.

I agree with Lluís.  Here's how it worked in scripts/tracetool:
disabled events were processed with "nop" backend and therefore did
not increment the event counter in the "simple" backend.

Stefan

Re: [Qemu-devel] [PATCH 1/6] trace: [tracetool] Do not rebuild event list in backend code

[Lluís Vilanova]

Harsh Bora writes:

> On 01/11/2012 11:35 PM, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
>> ---
>> scripts/tracetool.py |   14 +++++++-------
>> 1 files changed, 7 insertions(+), 7 deletions(-)
>> 
>> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
>> index 6874f66..80e5684 100755
>> --- a/scripts/tracetool.py
>> +++ b/scripts/tracetool.py

> [.. snip ..]

>> @@ -510,14 +508,16 @@ class Event(object):
>> 
>> # Generator that yields Event objects given a trace-events file object
>> def read_events(fobj):
>> +    res = []
>> event_num = 0
>> for line in fobj:
>> if not line.strip():
>> continue
>> if line.lstrip().startswith('#'):
>> continue
>> -	yield Event(event_num, line)
>> +	res.append(Event(event_num, line))
>> event_num += 1
>> +    return res
>> 

> Hi Lluis,
> This looks really nice. I can include your fixes while addressing other review
> comments. Shall I fold your patches with mine or do you want to keep them
> separate?

Whatever works best for you.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH 1/6] trace: [tracetool] Do not rebuild event list in backend code

[Lluís Vilanova]

Lluís Vilanova writes:

> Harsh Bora writes:

>> Hi Lluis,
>> This looks really nice. I can include your fixes while addressing other review
>> comments. Shall I fold your patches with mine or do you want to keep them
>> separate?

> Whatever works best for you.

BTW, I did some more changes, but didn't care to separate them into more
patches, so maybe I'll send you a few more on the next version.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth