[Qemu-devel] [PATCH 1/3] trace: Fix print format for "mipsnet_write"

[Qemu-devel] [PATCH 1/3] trace: Fix print format for "mipsnet_write"

[Lluís Vilanova]

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

diff --git a/trace-events b/trace-events
index c09399f..9d1fbbb 100644
--- a/trace-events
+++ b/trace-events
@@ -454,7 +454,7 @@ milkymist_vgafb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08
 mipsnet_send(uint32_t size) "sending len=%u"
 mipsnet_receive(uint32_t size) "receiving len=%u"
 mipsnet_read(uint64_t addr, uint32_t val) "read addr=0x%" PRIx64 " val=0x%x"
-mipsnet_write(uint64_t addr, uint64_t val) "write addr=0x%" PRIx64 " val=0x%" PRIx64
+mipsnet_write(uint64_t addr, uint64_t val) "write addr=0x%" PRIx64 " val=0x%" PRIx64 ""
 mipsnet_irq(uint32_t isr, uint32_t intctl) "set irq to %d (%02x)"
 
 # xen-all.c

[Qemu-devel] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 trace-events |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/trace-events b/trace-events
index 9d1fbbb..b653d70 100644
--- a/trace-events
+++ b/trace-events
@@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
 # hw/milkymist-softusb.c
 milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
 milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
-milkymist_softusb_mevt(uint8_t m) "m %d"
-milkymist_softusb_kevt(uint8_t m) "m %d"
+milkymist_softusb_mevt(uint8_t _m) "m %d"
+milkymist_softusb_kevt(uint8_t _m) "m %d"
 milkymist_softusb_mouse_event(int dx, int dy, int dz, int bs) "dx %d dy %d dz %d bs %02x"
 milkymist_softusb_pulse_irq(void) "Pulse IRQ"
 

Re: [Qemu-devel] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Stefan Hajnoczi]

On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  trace-events |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/trace-events b/trace-events
> index 9d1fbbb..b653d70 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>  # hw/milkymist-softusb.c
>  milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
>  milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
> -milkymist_softusb_mevt(uint8_t m) "m %d"
> -milkymist_softusb_kevt(uint8_t m) "m %d"
> +milkymist_softusb_mevt(uint8_t _m) "m %d"
> +milkymist_softusb_kevt(uint8_t _m) "m %d"

The LTTng community has been very responsive in addressing namespace
issues with libust.  Let's post more details and see if it can be fixed
in libust.

Could you please post your gcc and libust versions?

I have not been able to reproduce the problem on Debian libust-dev
0.15-3.  My gcc version is Debian gcc 4.6.1-4.

Here is the test program:

#include <stdint.h>
#include <ust/tracepoint.h>

DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
TP_ARGS(m));

int main(int argc, char **argv)
{
	return 0;
}

I see no warning or error related to the 'm' argument name.

Stefan

Re: [Qemu-devel] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>> trace-events |    4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/trace-events b/trace-events
>> index 9d1fbbb..b653d70 100644
>> --- a/trace-events
>> +++ b/trace-events
>> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>> # hw/milkymist-softusb.c
>> milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
>> milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
>> -milkymist_softusb_mevt(uint8_t m) "m %d"
>> -milkymist_softusb_kevt(uint8_t m) "m %d"
>> +milkymist_softusb_mevt(uint8_t _m) "m %d"
>> +milkymist_softusb_kevt(uint8_t _m) "m %d"

> The LTTng community has been very responsive in addressing namespace
> issues with libust.  Let's post more details and see if it can be fixed
> in libust.

> Could you please post your gcc and libust versions?

> I have not been able to reproduce the problem on Debian libust-dev
> 0.15-3.  My gcc version is Debian gcc 4.6.1-4.

Yup, I was using libust-dev 0.5 (debian stable). After switching to 0.15 (debian
testing) other problems arise.

For example, in my box compiling osdep.c yields lots of "variable ‘__tp_cb_data’
set but not used", but using a minimal example does not raise such problems, so
I'm assuming this is related to some namespace problems related to some other
code in qemu's headers.

Unfortunately, right now I have no time to debug the causes, so I'll just assume
the two patches I sent (this naming issue and the zero-length printf format) are
just no longer necessary with recent versions of libust.


Thanks,
    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] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Mathieu Desnoyers]

* Lluís Vilanova (vilanova@ac.upc.edu) wrote:
> Stefan Hajnoczi writes:
> 
> > On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> >> ---
> >> trace-events |    4 ++--
> >> 1 files changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/trace-events b/trace-events
> >> index 9d1fbbb..b653d70 100644
> >> --- a/trace-events
> >> +++ b/trace-events
> >> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
> >> # hw/milkymist-softusb.c
> >> milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
> >> milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
> >> -milkymist_softusb_mevt(uint8_t m) "m %d"
> >> -milkymist_softusb_kevt(uint8_t m) "m %d"
> >> +milkymist_softusb_mevt(uint8_t _m) "m %d"
> >> +milkymist_softusb_kevt(uint8_t _m) "m %d"
> 
> > The LTTng community has been very responsive in addressing namespace
> > issues with libust.  Let's post more details and see if it can be fixed
> > in libust.
> 
> > Could you please post your gcc and libust versions?
> 
> > I have not been able to reproduce the problem on Debian libust-dev
> > 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
> 
> Yup, I was using libust-dev 0.5 (debian stable). After switching to 0.15 (debian
> testing) other problems arise.
> 
> For example, in my box compiling osdep.c yields lots of "variable ‘__tp_cb_data’
> set but not used", but using a minimal example does not raise such problems, so
> I'm assuming this is related to some namespace problems related to some other
> code in qemu's headers.

If you can find a few minutes to provide:

1) a link to the git repository/commit ID you are testing
2) the exact configuration you use (detailed way to reproduce the
   problem, as a sequence of commands from a pristine repository)

It will allow me to look into this warning and fix it.

Thanks!

Mathieu

> 
> Unfortunately, right now I have no time to debug the causes, so I'll just assume
> the two patches I sent (this naming issue and the zero-length printf format) are
> just no longer necessary with recent versions of libust.
> 
> 
> Thanks,
>     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
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev@lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Stefan Hajnoczi]

2011/9/20 Mathieu Desnoyers <compudj@krystal.dyndns.org>:
> * Lluís Vilanova (vilanova@ac.upc.edu) wrote:
>> Stefan Hajnoczi writes:
>>
>> > On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
>> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> >> ---
>> >> trace-events |    4 ++--
>> >> 1 files changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/trace-events b/trace-events
>> >> index 9d1fbbb..b653d70 100644
>> >> --- a/trace-events
>> >> +++ b/trace-events
>> >> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>> >> # hw/milkymist-softusb.c
>> >> milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
>> >> milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
>> >> -milkymist_softusb_mevt(uint8_t m) "m %d"
>> >> -milkymist_softusb_kevt(uint8_t m) "m %d"
>> >> +milkymist_softusb_mevt(uint8_t _m) "m %d"
>> >> +milkymist_softusb_kevt(uint8_t _m) "m %d"
>>
>> > The LTTng community has been very responsive in addressing namespace
>> > issues with libust.  Let's post more details and see if it can be fixed
>> > in libust.
>>
>> > Could you please post your gcc and libust versions?
>>
>> > I have not been able to reproduce the problem on Debian libust-dev
>> > 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
>>
>> Yup, I was using libust-dev 0.5 (debian stable). After switching to 0.15 (debian
>> testing) other problems arise.
>>
>> For example, in my box compiling osdep.c yields lots of "variable ‘__tp_cb_data’
>> set but not used", but using a minimal example does not raise such problems, so
>> I'm assuming this is related to some namespace problems related to some other
>> code in qemu's headers.
>
> If you can find a few minutes to provide:
>
> 1) a link to the git repository/commit ID you are testing
> 2) the exact configuration you use (detailed way to reproduce the
>   problem, as a sequence of commands from a pristine repository)
>
> It will allow me to look into this warning and fix it.

AFAICT the only problem with libust is the __tp_cb_data set but not
used warning that gcc 4.6 emits, see my test program:

#include <stdint.h>
#include <ust/tracepoint.h>

DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
TP_ARGS(m));

int main(int argc, char **argv)
{
       return 0;
}

You will get an error with gcc 4.6 and libust 0.15:

$ gcc -o a -Wall a.c

QEMU itself does have one problem with libust 0.15.  We do
DECLARE_TRACE(name, TP_PROTO(void), TP_ARGS()).  This results in a
compiler error but we can fix this my changing our tracepoints to use
DECLARE_TRACE(name, TP_PROTO(), TP_ARGS()) I think.  Perhaps libust's
behavior here has changed, it shouldn't be hard to update QEMU's
tracetool tracepoint generator though.

If you adapt the example program I used above to use TP_PROTO(void),
TP_ARGS() then you get this error:

a.c: In function ‘__trace_ust_milkymist_softusb_mevt’:
a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
a.c:4:1: error: expected ‘)’ before ‘__tp_it_func’
a.c:4:1: error: expected expression before ‘)’ token
a.c:4:1: warning: variable ‘__tp_it_func’ set but not used
[-Wunused-but-set-variable]
a.c: At top level:
a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
a.c:4:1: error: expected ‘;’, ‘,’ or ‘)’ before ‘void’
a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
a.c:4:1: error: expected ‘;’, ‘,’ or ‘)’ before ‘void’
a.c: In function ‘trace_ust_milkymist_softusb_mevt’:
a.c:4:1: warning: variable ‘__tp_cb_data’ set but not used
[-Wunused-but-set-variable]

I can't say for sure whether I ever tested old libust versions with
void tracepoints.  Perhaps we simply never had any and the tracetool
generator has always been broken.  Or perhaps libust changed its
behavior.

Stefan

Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Lluís Vilanova]

Stefan Hajnoczi writes:

> AFAICT the only problem with libust is the __tp_cb_data set but not
> used warning that gcc 4.6 emits, see my test program:

Here's a prettified version of the failing (pre-processed) code:

  static inline
  void
  trace_ust_g_malloc(size_t size, void* ptr)
  {
      do {
          if (__builtin_expect(!!(__tracepoint_ust_g_malloc.state), 0))
              do {
                  struct tracepoint_probe *__tp_it_probe_ptr;
                  void *__tp_it_func;
                  void *__tp_cb_data;
                  rcu_read_lock_bp();
                  __tp_it_probe_ptr = ({ typeof((&__tracepoint_ust_g_malloc)->probes) _________p1 = rcu_dereference_sym((void *)((&__tracepoint_ust_g_malloc)->probes)); (_________p1); });
                  if (__tp_it_probe_ptr) {
                      do {
                          __tp_it_func = __tp_it_probe_ptr->func;
                          __tp_cb_data = __tp_it_probe_ptr->data;
                          ((void(*)(size_t size, void* ptr))__tp_it_func)(size, ptr);
                      } while ((++__tp_it_probe_ptr)->func); }
                  rcu_read_unlock_bp();
              } while (0);
      } while (0);
  }

Which is produced by the "__DO_TRACE" macro in "ust/tracepoint.h". The original
code is:

  DECLARE_TRACE(ust_g_malloc, TP_PROTO(size_t size, void* ptr), TP_ARGS(size, ptr));
  #define trace_g_malloc trace_ust_g_malloc


I don't know about the portability requirements in UST, but if supporting only
gcc is an option, you can simply use this line in the macro:

                  void *__tp_cb_data __attribute__((unused));

Even with this fixed, there still seem to be problems with events without
parameters:

  DECLARE_TRACE(ust_slavio_misc_update_irq_raise, TP_PROTO(void), TP_ARGS());
  #define trace_slavio_misc_update_irq_raise trace_ust_slavio_misc_update_irq_raise

  ./trace.h: In function ‘__trace_ust_slavio_misc_update_irq_raise’:
  ./trace.h:361:1: error: ‘void’ must be the only parameter
  ./trace.h:361:1: error: expected expression before ‘)’ token
  ./trace.h:361:1: error: too many arguments to function ‘(void (*)(void *))__tp_it_func’

The debian/testing version of ust is 0.15, and the sources have a ChangeLog with
this at the top:

  2011-07-15 ust 0.15
      * Add backward compability for tracepoint API (still planned for
        deprecation, but should make the transition smoother).


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] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Mathieu Desnoyers]

* Lluís Vilanova (vilanova@ac.upc.edu) wrote:
> 
> I don't know about the portability requirements in UST, but if supporting only
> gcc is an option, you can simply use this line in the macro:
> 
>                   void *__tp_cb_data __attribute__((unused));
> 

Done. See commit 34dca3cc6e0b230c988b16117b967a094560f4ed.

> Even with this fixed, there still seem to be problems with events without
> parameters:
> 
>   DECLARE_TRACE(ust_slavio_misc_update_irq_raise, TP_PROTO(void), TP_ARGS());
>   #define trace_slavio_misc_update_irq_raise trace_ust_slavio_misc_update_irq_raise
> 
>   ./trace.h: In function ‘__trace_ust_slavio_misc_update_irq_raise’:
>   ./trace.h:361:1: error: ‘void’ must be the only parameter
>   ./trace.h:361:1: error: expected expression before ‘)’ token
>   ./trace.h:361:1: error: too many arguments to function ‘(void (*)(void *))__tp_it_func’
> 

TRACEPOINT_EVENT_NOARGS(name, TP_FIELDS()) should be used there.

Thanks!

Mathieu

> The debian/testing version of ust is 0.15, and the sources have a ChangeLog with
> this at the top:
> 
>   2011-07-15 ust 0.15
>       * Add backward compability for tracepoint API (still planned for
>         deprecation, but should make the transition smoother).
> 
> 
> 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
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Mathieu Desnoyers]

* Stefan Hajnoczi (stefanha@gmail.com) wrote:
> 2011/9/20 Mathieu Desnoyers <compudj@krystal.dyndns.org>:
> > * Lluís Vilanova (vilanova@ac.upc.edu) wrote:
> >> Stefan Hajnoczi writes:
> >>
> >> > On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> >> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> >> >> ---
> >> >> trace-events |    4 ++--
> >> >> 1 files changed, 2 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/trace-events b/trace-events
> >> >> index 9d1fbbb..b653d70 100644
> >> >> --- a/trace-events
> >> >> +++ b/trace-events
> >> >> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
> >> >> # hw/milkymist-softusb.c
> >> >> milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
> >> >> milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
> >> >> -milkymist_softusb_mevt(uint8_t m) "m %d"
> >> >> -milkymist_softusb_kevt(uint8_t m) "m %d"
> >> >> +milkymist_softusb_mevt(uint8_t _m) "m %d"
> >> >> +milkymist_softusb_kevt(uint8_t _m) "m %d"
> >>
> >> > The LTTng community has been very responsive in addressing namespace
> >> > issues with libust.  Let's post more details and see if it can be fixed
> >> > in libust.
> >>
> >> > Could you please post your gcc and libust versions?
> >>
> >> > I have not been able to reproduce the problem on Debian libust-dev
> >> > 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
> >>
> >> Yup, I was using libust-dev 0.5 (debian stable). After switching to 0.15 (debian
> >> testing) other problems arise.
> >>
> >> For example, in my box compiling osdep.c yields lots of "variable ‘__tp_cb_data’
> >> set but not used", but using a minimal example does not raise such problems, so
> >> I'm assuming this is related to some namespace problems related to some other
> >> code in qemu's headers.
> >
> > If you can find a few minutes to provide:
> >
> > 1) a link to the git repository/commit ID you are testing
> > 2) the exact configuration you use (detailed way to reproduce the
> >   problem, as a sequence of commands from a pristine repository)
> >
> > It will allow me to look into this warning and fix it.
> 
> AFAICT the only problem with libust is the __tp_cb_data set but not
> used warning that gcc 4.6 emits, see my test program:
> 
> #include <stdint.h>
> #include <ust/tracepoint.h>
> 
> DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
> TP_ARGS(m));
> 
> int main(int argc, char **argv)
> {
>        return 0;
> }
> 
> You will get an error with gcc 4.6 and libust 0.15:
> 
> $ gcc -o a -Wall a.c

Ah, I see. This is due to the backward compatibility API I added in
0.15 for qemu. It skips using the __tp_cb_data private data. So it does
make sense that the compiler complains about this. I can add a
__attribute__((unused)), as proposed by Lluís.

> 
> QEMU itself does have one problem with libust 0.15.  We do
> DECLARE_TRACE(name, TP_PROTO(void), TP_ARGS()).  This results in a
> compiler error but we can fix this my changing our tracepoints to use
> DECLARE_TRACE(name, TP_PROTO(), TP_ARGS()) I think.  Perhaps libust's
> behavior here has changed, it shouldn't be hard to update QEMU's
> tracetool tracepoint generator though.

For tracepoints without arguments, you need to use something like:

TRACEPOINT_EVENT_NOARGS(name, TP_FIELDS())

(note: the full support of TP_FIELDS() declaration will come in UST 2.0.
For UST 0.15, it's just ignored)

The _NOARGS variant lets you handle the (void) 0 parameter case.

Thanks,

Mathieu

> 
> If you adapt the example program I used above to use TP_PROTO(void),
> TP_ARGS() then you get this error:
> 
> a.c: In function ‘__trace_ust_milkymist_softusb_mevt’:
> a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
> a.c:4:1: error: expected ‘)’ before ‘__tp_it_func’
> a.c:4:1: error: expected expression before ‘)’ token
> a.c:4:1: warning: variable ‘__tp_it_func’ set but not used
> [-Wunused-but-set-variable]
> a.c: At top level:
> a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
> a.c:4:1: error: expected ‘;’, ‘,’ or ‘)’ before ‘void’
> a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
> a.c:4:1: error: expected ‘;’, ‘,’ or ‘)’ before ‘void’
> a.c: In function ‘trace_ust_milkymist_softusb_mevt’:
> a.c:4:1: warning: variable ‘__tp_cb_data’ set but not used
> [-Wunused-but-set-variable]
> 
> I can't say for sure whether I ever tested old libust versions with
> void tracepoints.  Perhaps we simply never had any and the tracetool
> generator has always been broken.  Or perhaps libust changed its
> behavior.
> 
> Stefan
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Stefan Hajnoczi]

On Tue, Sep 20, 2011 at 5:22 PM, Mathieu Desnoyers
<compudj@krystal.dyndns.org> wrote:
> * Stefan Hajnoczi (stefanha@gmail.com) wrote:
>> 2011/9/20 Mathieu Desnoyers <compudj@krystal.dyndns.org>:
>> > * Lluís Vilanova (vilanova@ac.upc.edu) wrote:
>> >> Stefan Hajnoczi writes:
>> >>
>> >> > On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
>> >> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> >> >> ---
>> >> >> trace-events |    4 ++--
>> >> >> 1 files changed, 2 insertions(+), 2 deletions(-)
>> >> >>
>> >> >> diff --git a/trace-events b/trace-events
>> >> >> index 9d1fbbb..b653d70 100644
>> >> >> --- a/trace-events
>> >> >> +++ b/trace-events
>> >> >> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>> >> >> # hw/milkymist-softusb.c
>> >> >> milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
>> >> >> milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
>> >> >> -milkymist_softusb_mevt(uint8_t m) "m %d"
>> >> >> -milkymist_softusb_kevt(uint8_t m) "m %d"
>> >> >> +milkymist_softusb_mevt(uint8_t _m) "m %d"
>> >> >> +milkymist_softusb_kevt(uint8_t _m) "m %d"
>> >>
>> >> > The LTTng community has been very responsive in addressing namespace
>> >> > issues with libust.  Let's post more details and see if it can be fixed
>> >> > in libust.
>> >>
>> >> > Could you please post your gcc and libust versions?
>> >>
>> >> > I have not been able to reproduce the problem on Debian libust-dev
>> >> > 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
>> >>
>> >> Yup, I was using libust-dev 0.5 (debian stable). After switching to 0.15 (debian
>> >> testing) other problems arise.
>> >>
>> >> For example, in my box compiling osdep.c yields lots of "variable ‘__tp_cb_data’
>> >> set but not used", but using a minimal example does not raise such problems, so
>> >> I'm assuming this is related to some namespace problems related to some other
>> >> code in qemu's headers.
>> >
>> > If you can find a few minutes to provide:
>> >
>> > 1) a link to the git repository/commit ID you are testing
>> > 2) the exact configuration you use (detailed way to reproduce the
>> >   problem, as a sequence of commands from a pristine repository)
>> >
>> > It will allow me to look into this warning and fix it.
>>
>> AFAICT the only problem with libust is the __tp_cb_data set but not
>> used warning that gcc 4.6 emits, see my test program:
>>
>> #include <stdint.h>
>> #include <ust/tracepoint.h>
>>
>> DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
>> TP_ARGS(m));
>>
>> int main(int argc, char **argv)
>> {
>>        return 0;
>> }
>>
>> You will get an error with gcc 4.6 and libust 0.15:
>>
>> $ gcc -o a -Wall a.c
>
> Ah, I see. This is due to the backward compatibility API I added in
> 0.15 for qemu. It skips using the __tp_cb_data private data. So it does
> make sense that the compiler complains about this. I can add a
> __attribute__((unused)), as proposed by Lluís.
>
>>
>> QEMU itself does have one problem with libust 0.15.  We do
>> DECLARE_TRACE(name, TP_PROTO(void), TP_ARGS()).  This results in a
>> compiler error but we can fix this my changing our tracepoints to use
>> DECLARE_TRACE(name, TP_PROTO(), TP_ARGS()) I think.  Perhaps libust's
>> behavior here has changed, it shouldn't be hard to update QEMU's
>> tracetool tracepoint generator though.
>
> For tracepoints without arguments, you need to use something like:
>
> TRACEPOINT_EVENT_NOARGS(name, TP_FIELDS())
>
> (note: the full support of TP_FIELDS() declaration will come in UST 2.0.
> For UST 0.15, it's just ignored)
>
> The _NOARGS variant lets you handle the (void) 0 parameter case.

Okay, this is the fault of QEMU's tracetool script, which generates
the LTTng UST tracepoints.  We can move to the new TRACEPOINT*()
macros and use _NOARGS as appropriate.

Stefan

Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[P DUMAS]

* Stefan Hajnoczi (stefanha@gmail.com) wrote:
> On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> > ---
> >  trace-events |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/trace-events b/trace-events
> > index 9d1fbbb..b653d70 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
> >  # hw/milkymist-softusb.c
> >  milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
> >  milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
> > -milkymist_softusb_mevt(uint8_t m) "m %d"
> > -milkymist_softusb_kevt(uint8_t m) "m %d"
> > +milkymist_softusb_mevt(uint8_t _m) "m %d"
> > +milkymist_softusb_kevt(uint8_t _m) "m %d"
> 
> The LTTng community has been very responsive in addressing namespace
> issues with libust.  Let's post more details and see if it can be fixed
> in libust.
> 
> Could you please post your gcc and libust versions?
> 
> I have not been able to reproduce the problem on Debian libust-dev
> 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
> 
> Here is the test program:
> 
> #include <stdint.h>
> #include <ust/tracepoint.h>
> 
> DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
> TP_ARGS(m));
> 
> int main(int argc, char **argv)
> {
> 	return 0;
> }
> 
> I see no warning or error related to the 'm' argument name.

I remember fixing that for trace_mark() instrumentation in UST 0.x ages
ago (ok, probably last winter). I think simply updating to a new UST
version will fix the issue.

Best regards,

Mathieu

> 
> Stefan
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev@lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Mathieu Desnoyers]

* Stefan Hajnoczi (stefanha@gmail.com) wrote:
> On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> > ---
> >  trace-events |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/trace-events b/trace-events
> > index 9d1fbbb..b653d70 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
> >  # hw/milkymist-softusb.c
> >  milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
> >  milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
> > -milkymist_softusb_mevt(uint8_t m) "m %d"
> > -milkymist_softusb_kevt(uint8_t m) "m %d"
> > +milkymist_softusb_mevt(uint8_t _m) "m %d"
> > +milkymist_softusb_kevt(uint8_t _m) "m %d"
> 
> The LTTng community has been very responsive in addressing namespace
> issues with libust.  Let's post more details and see if it can be fixed
> in libust.
> 
> Could you please post your gcc and libust versions?
> 
> I have not been able to reproduce the problem on Debian libust-dev
> 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
> 
> Here is the test program:
> 
> #include <stdint.h>
> #include <ust/tracepoint.h>
> 
> DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
> TP_ARGS(m));
> 
> int main(int argc, char **argv)
> {
> 	return 0;
> }
> 
> I see no warning or error related to the 'm' argument name.

(sorry for re-send, I just fixed my SMTP setup)

I remember fixing that for trace_mark() instrumentation in UST 0.x ages
ago (ok, probably last winter). I think simply updating to a new UST
version will fix the issue.

Best regards,

Mathieu

> 
> Stefan
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev@lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Qemu-devel] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

[Stefan Hajnoczi]

On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  trace-events |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/trace-events b/trace-events
> index 9d1fbbb..b653d70 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>  # hw/milkymist-softusb.c
>  milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x value %08x"
>  milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08x"
> -milkymist_softusb_mevt(uint8_t m) "m %d"
> -milkymist_softusb_kevt(uint8_t m) "m %d"
> +milkymist_softusb_mevt(uint8_t _m) "m %d"
> +milkymist_softusb_kevt(uint8_t _m) "m %d"

Do you also see errors with trace events that have no arguments?

DECLARE_TRACE(ust_qemu_co_queue_next_bh, TP_PROTO(void), TP_ARGS());

In file included from osdep.c:48:0:
trace.h: In function ‘__trace_ust_qemu_co_queue_next_bh’:
trace.h:1030:195: error: ‘void’ must be the only parameter
trace.h:1030:661: error: expected expression before ‘)’ token
trace.h:1030:661: error: too many arguments to function ‘(void (*)(void *))__tp_it_func’
trace.h: At top level:
trace.h:1030:661: error: ‘void’ must be the only parameter
trace.h:1030:661: error: ‘void’ must be the only parameter

On my box this can be fixed by changing the DEFINE_TRACE() like this:

DECLARE_TRACE(ust_qemu_co_queue_next_bh, TP_PROTO(), TP_ARGS());

I haven't looked into the details yet but wanted to check.  I suspect that different versions of libust might have different quirks here.

Stefan

[Qemu-devel] [PATCH 3/3] trace: [ust] Do not use zero-length format strings (avoid compiler warning)

[Lluís Vilanova]

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

diff --git a/trace-events b/trace-events
index b653d70..7b2e619 100644
--- a/trace-events
+++ b/trace-events
@@ -480,7 +480,7 @@ qemu_coroutine_yield(void *from, void *to) "from %p to %p"
 qemu_coroutine_terminate(void *co) "self %p"
 
 # qemu-coroutine-lock.c
-qemu_co_queue_next_bh(void) ""
+qemu_co_queue_next_bh(void) " "
 qemu_co_queue_next(void *next) "next %p"
 qemu_co_mutex_lock_entry(void *mutex, void *self) "mutex %p self %p"
 qemu_co_mutex_lock_return(void *mutex, void *self) "mutex %p self %p"

Re: [Qemu-devel] [PATCH 1/3] trace: Fix print format for "mipsnet_write"

[Stefan Hajnoczi]

On Fri, Sep 16, 2011 at 06:59:33PM +0200, Lluís Vilanova wrote:
> diff --git a/trace-events b/trace-events
> index c09399f..9d1fbbb 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -454,7 +454,7 @@ milkymist_vgafb_memory_write(uint32_t addr, uint32_t value) "addr %08x value %08
>  mipsnet_send(uint32_t size) "sending len=%u"
>  mipsnet_receive(uint32_t size) "receiving len=%u"
>  mipsnet_read(uint64_t addr, uint32_t val) "read addr=0x%" PRIx64 " val=0x%x"
> -mipsnet_write(uint64_t addr, uint64_t val) "write addr=0x%" PRIx64 " val=0x%" PRIx64
> +mipsnet_write(uint64_t addr, uint64_t val) "write addr=0x%" PRIx64 " val=0x%" PRIx64 ""

This limitation of the tracetool parser has been removed in commit
913540a376f01569227b9a6480a2e96ae9d1d406.

Stefan