Re: [Qemu-devel] [lttng-dev] [RFC PATCH v2 0/4] simpletrace : support var num of args and strings.

[Harsh Bora <harsh@linux.vnet.ibm.com>]

On 01/10/2012 02:36 PM, Harsh Bora wrote:
> On 01/10/2012 12:47 PM, Mathieu Desnoyers wrote:
>> * Harsh Bora (harsh@linux.vnet.ibm.com) wrote:
>>> On 01/10/2012 05:44 AM, Mathieu Desnoyers wrote:
>>>> * Harsh Bora (harsh@linux.vnet.ibm.com) wrote:
>>>>> On 01/09/2012 09:31 PM, Mathieu Desnoyers wrote:
>>>>>> * Harsh Prateek Bora (harsh@linux.vnet.ibm.com) wrote:
>>>>>>> Existing simple trace can log upto 6 args per trace event and
>>>>>>> does not
>>>>>>> support strings in trace record format. Introducing new trace
>>>>>>> format as
>>>>>>> discussed earlier on list to support variable number/size of
>>>>>>> arguments.
>>>>>>> (Ref:
>>>>>>> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg03426.html)
>>>>>>>
>>>>>>> Basic testing of this patch is successful. Stress testing not yet
>>>>>>> done.
>>>>>>>
>>>>>>> Apply patches, then run:
>>>>>>>
>>>>>>> make distclean
>>>>>>> ./configure with --enable-trace-backend=simple
>>>>>>> make
>>>>>>> sudo make install
>>>>>>>
>>>>>>> Sample tracelog showing strings support:
>>>>>>> [harsh@harshbora v9fs]$ scripts/simpletrace.py trace-events
>>>>>>> trace-23261
>>>>>>> v9fs_version 0.000 tag=65535 id=100 msize=8192 version=9P2000.L
>>>>>>> v9fs_version_return 17.530 tag=65535 id=100 msize=8192
>>>>>>> version=9P2000.L
>>>>>>> v9fs_attach 180.121 tag=1 id=104 fid=0 afid=18446744073709551615
>>>>>>> uname=nobody aname=
>>>>>>>
>>>>>>>
>>>>>>> Note: LTTng ust backend is broken in upstream qemu, therefore
>>>>>>> tracetool.py
>>>>>>> doesnt support ust backend as of now. IIUC, ust's trace event
>>>>>>> APIs are under
>>>>>>> development and not yet stable.
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> FYI, the LTTng-UST TRACEPOINT_EVENT API is very much stable as of
>>>>>> now.
>>>>>> Even though we are still in LTTng-UST 2.0 prereleases, the fact
>>>>>> that we
>>>>>> started the round of discussions on this API last summer makes us
>>>>>> confident that from this point on we should not have to change it.
>>>>>>
>>>>>> Moreover, I would like to know if the old UST 0.x (0.16 is the
>>>>>> latest)
>>>>>> is broken wrt qemu, or if this is just for LTTng-2.0 UST support ?
>>>>>> UST 0.x instrumentation is not supposed to have broken wrt qemu.
>>>>>>
>>>>>
>>>>> Hi,
>>>>> Thanks for an early response. I had tried building with ust 0.16
>>>>> and it
>>>>> gives compilation errors, specially for trace events with 'void'
>>>>> argument:
>>>>>
>>>>> CC osdep.o
>>>>> In file included from osdep.c:49:
>>>>> trace.h: In function ‘__trace_ust_slavio_misc_update_irq_raise’:
>>>>> trace.h:277: error: ‘void’ must be the only parameter
>>>>> trace.h:277: error: expected expression before ‘)’ token
>>>>> trace.h:277: error: too many arguments to function ‘(void (*)(void
>>>>> *))__tp_it_func’
>>>>> trace.h: At top level:
>>>>> trace.h:277: error: ‘void’ must be the only parameter
>>>>> trace.h:277: error: ‘void’ must be the only parameter
>>>>> In file included from osdep.c:49:
>>>>> trace.h: In function ‘__trace_ust_slavio_misc_update_irq_lower’:
>>>>> trace.h:280: error: ‘void’ must be the only parameter
>>>>> trace.h:280: error: expected expression before ‘)’ token
>>>>> trace.h:280: error: too many arguments to function ‘(void (*)(void
>>>>> *))__tp_it_func’
>>>>>
>>>>>
>>>>> I am not sure which interface is supposed to be used for void
>>>>> arguments
>>>>> in ust 0.16.
>>>>
>>>> Looking at scripts/tracetool:
>>>>
>>>> linetoh_ust()
>>>> {
>>>> local name args argnames
>>>> name=$(get_name "$1")
>>>> args=$(get_args "$1")
>>>> argnames=$(get_argnames "$1", ",")
>>>>
>>>> cat<<EOF
>>>> DECLARE_TRACE(ust_$name, TP_PROTO($args), TP_ARGS($argnames));
>>>> #define trace_$name trace_ust_$name
>>>> EOF
>>>> }
>>>>
>>>> for those tracepoints with argument "void", DECLARE_TRACE_NOARGS should
>>>> be used for UST 0.16. Similar for:
>>>>
>>>> DEFINE_TRACE(ust_$name); -> DEFINE_TRACE_NOARGS(ust_$name);
>>>>
>>>
>>> I had tried with _NOARGS variants initially by manually changing the
>>> auto-generated code like this:
>>>
>>> In trace.h:
>>>
>>> DECLARE_TRACE_NOARGS(ust_slavio_misc_update_irq_raise);
>>> #define trace_slavio_misc_update_irq_raise
>>> trace_ust_slavio_misc_update_irq_raise
>>>
>>> In trace.c:
>>>
>>> DEFINE_TRACE_NOARGS(ust_slavio_misc_update_irq_raise);
>>>
>>> static void ust_slavio_misc_update_irq_raise_probe()
>>> {
>>> trace_mark(ust, slavio_misc_update_irq_raise);
>>> }
>>>
>>>
>>> However, it still gave error like this:
>>>
>>> [harsh@harshbora v9fs]$ make
>>> CC osdep.o
>>> cc1: warnings being treated as errors
>>> In file included from osdep.c:49:
>>> trace.h:277: error: data definition has no type or storage class
>>> trace.h:277: error: type defaults to ‘int’ in declaration of
>>> ‘DECLARE_TRACE_NOARGS’
>>> trace.h:277: error: parameter names (without types) in function
>>> declaration
>>>
>>>
>>> It will be great if you can provide a sample code (to be auto-generated)
>>> required for a trace-event with void argument.
>>
>> Ah, right, now I remember that I only left DEFINE_TRACE and DECLARE_TRACE
>> as backward compatibility in UST 0.x, and did only add the "_NOARGS" to
>> the backward compatibility macros with an added underscore prefix.
>>
>> If you really just want to have compatibility with UST 0.16, changing the
>> minimum of things compared with what you currently have, I would
>> recommend to use:
>>
>> in trace.h:
>> _DECLARE_TRACEPOINT_NOARGS(ust_slavio_misc_update_irq_raise)
>> #define trace_slavio_misc_update_irq_raise
>> trace_ust_slavio_misc_update_irq_raise
>>
>> in trace.c:
>> DEFINE_TRACE(ust_slavio_misc_update_irq_raise);
>>
>> static void ust_slavio_misc_update_irq_raise_probe(void)
>> {
>> trace_mark(ust, slavio_misc_update_irq_raise, UST_MARKER_NOARGS);
>> }
>>
>> Let me know how this works for you,
>
> Thanks for the help, I wrote the python code to auto-generate the same
> for events with void arg. However, after doing make, it says:
>
> trace.o:(__ust_markers+0x4a08): undefined reference to
> `__ust_marker_empty_function'
> trace.o:(__ust_markers+0x4a60): undefined reference to
> `ust_marker_probe_cb'
> trace.o:(__ust_markers+0x4a68): undefined reference to
> `__ust_marker_empty_function'
> trace.o:(__ust_markers+0x4ac0): undefined reference to
> `ust_marker_probe_cb'
>
> I have tested using both:
> 1) ust 0.15 with urcu 0.6.
> 2) ust 0.16 with urcu 0.6.6
>
> Any idea, why this error ?

Ok, I got it (Thanks Aneesh). It required this change in configure:

diff --git a/configure b/configure
index 0ea8634..40a18d8 100755
--- a/configure
+++ b/configure
@@ -2495,7 +2495,8 @@ if test "$trace_backend" = "ust"; then
  int main(void) { return 0; }
  EOF
    if compile_prog "" "" ; then
-    LIBS="-lust $LIBS"
+    LIBS="-lust -lurcu-bp $LIBS"
+    libs_qga+="-lust -lurcu-bp"
    else

regards,
Harsh

>
> - Harsh
>
>>
>> Best regards,
>>
>> Mathieu
>>
>>>
>>> - Harsh
>>>
>>>>> Moreover, if ust 2.0 uses different interfaces, we might
>>>>> want to use the latest one.
>>>>
>>>> Note that this kind of special-case won't be needed with LTTng-UST 2.0
>>>> TRACEPOINT_EVENT. In place of DECLARE_TRACE, one would use:
>>>>
>>>> TRACEPOINT_EVENT(qemu_kvm, $name,
>>>> TP_ARGS($args),
>>>> TP_FIELDS()
>>>> )
>>>>
>>>> Note that I notice that some care will need to be taken to generate the
>>>> TP_FIELDS() from your existing trace-events file, an example:
>>>>
>>>> g_realloc(void *ptr, size_t size, void *newptr)
>>>>
>>>> would have to be translated to:
>>>>
>>>> TRACE_EVENT(qemu_kvm, g_realloc,
>>>> TP_ARGS(void *, ptr, size_t, size, void *, newptr),
>>>> TP_FIELDS(
>>>> ctf_integer_hex(void *, ptr, ptr)
>>>> ctf_integer(size_t, size, size)
>>>> ctf_integer_hex(void *, newptr, newptr)
>>>> )
>>>> )
>>>>
>>>> Note that the bright side is that the tracepoint probe does not need to
>>>> be hand-coded anymore, and there is no need to use the markers anymore
>>>> neither, which makes the tracer much faster.
>>>>
>>>> For most of your fields (using %p, %d style format strings), you should
>>>> use ctf_integer or ctf_integer_hex (the latter lets the trace viewer
>>>> know that the data should be printed as hexadecimal).
>>>> You will likely need to detect the %s format strings you have there and
>>>> translate them into ctf_string(field, field) too. You can have a
>>>> look at
>>>> lttng-ust tests/hello/*.[ch] for examples.
>>>
>>> Thanks for the info, I will look into it later.
>>>>
>>>> The call which would have looked like trace_qemu_kvm_g_realloc() in UST
>>>> 0.x should now be done with:
>>>>
>>>> tracepoint(qemu_kvm, g_realloc, ptr, size, newptr);
>>>>
>>>> This is needed to (very soon) add support for sdt.h in LTTng-UST
>>>> 2.0, so
>>>> systemtap and gdb can hook into tracepoints declared by lttng-ust 2.0.
>>>>
>>>> Best regards,
>>>>
>>>> Mathieu
>>>>
>>>>>
>>>>> regards,
>>>>> Harsh
>>>>>
>>>>>> Best regards,
>>>>>>
>>>>>> Mathieu
>>>>>>
>>>>>>>
>>>>>>> Version History:
>>>>>>>
>>>>>>> v2:
>>>>>>> - Updated tracetool.py to support nop, stderr, dtrace backend
>>>>>>>
>>>>>>> v1:
>>>>>>> - Working protoype with tracetool.py converted only for
>>>>>>> simpletrace backend
>>>>>>>
>>>>>>> Harsh Prateek Bora (4):
>>>>>>> Converting tracetool.sh to tracetool.py
>>>>>>> Makefile and configure changes for tracetool.py
>>>>>>> simpletrace-v2: Handle variable number/size of elements per trace
>>>>>>> record.
>>>>>>> simpletrace.py: updated log reader script to handle new log format
>>>>>>>
>>>>>>> Makefile.objs | 6 +-
>>>>>>> Makefile.target | 10 +-
>>>>>>> configure | 4 +-
>>>>>>> monitor.c | 2 +-
>>>>>>> scripts/simpletrace.py | 110 ++++++++++-
>>>>>>> scripts/tracetool.py | 505
>>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>> trace/simple.c | 178 ++++++-----------
>>>>>>> trace/simple.h | 31 +++-
>>>>>>> 8 files changed, 702 insertions(+), 144 deletions(-)
>>>>>>> create mode 100755 scripts/tracetool.py
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>> _______________________________________________
>>> lttng-dev mailing list
>>> lttng-dev@lists.lttng.org
>>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>
>