diff mbox

[PULL,00/20] Tracing patches

Message ID 87y420l1ak.fsf@fimbulvetr.bsc.es (mailing list archive)
State New, archived
Headers show

Commit Message

Lluís Vilanova Oct. 7, 2016, 1:46 p.m. UTC 
Peter Maydell writes:

> On 7 October 2016 at 11:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:
>> 
>> Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)
>> 
>> are available in the git repository at:
>> 
>> git://github.com/stefanha/qemu.git tags/tracing-pull-request
>> 
>> for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:
>> 
>> trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)
>> 
>> ----------------------------------------------------------------
>> 
>> ----------------------------------------------------------------

> This makes all the linux-user binaries dump core on startup:

> $ gdb --args /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
> /bin/ls
> (gdb) r
> Starting program:
> /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
> /bin/ls
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> [New Thread 0x7ffff7ffa700 (LWP 14965)]

> Thread 1 "qemu-x86_64" received signal SIGABRT, Aborted.
> 0x000000006019fbe8 in raise ()
> (gdb) bt
> #0  0x000000006019fbe8 in raise ()
> #1  0x00000000601a030a in abort ()
> #2  0x00000000600bdd02 in bitmap_new (nbits=0)
>     at /home/petmay01/linaro/qemu-for-merges/include/qemu/bitmap.h:99
> #3  0x00000000600be99f in cpu_common_initfn (obj=0x625f6340)
>     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:360
> #4  0x00000000600bfe97 in object_init_with_type (obj=0x625f6340, ti=0x625eb9a0)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:339
> #5  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e2d80)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
> #6  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e78d0)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
> #7  0x00000000600c00aa in object_initialize_with_type
> (data=0x625f6340, size=38416, type=0x625e78d0)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:370
> #8  0x00000000600c0508 in object_new_with_type (type=0x625e78d0)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:478
> #9  0x00000000600c0542 in object_new (typename=0x625e2fe0 "qemu64-x86_64-cpu")
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:488
> #10 0x00000000600be03f in cpu_generic_init (typename=0x6025ad54
> "x86_64-cpu", cpu_model=0x60247ca5 "qemu64")
>     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:76
> #11 0x0000000060089438 in cpu_x86_init (cpu_model=0x60247ca5 "qemu64")
>     at /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:2266
> #12 0x0000000060031031 in main (argc=2, argv=0x7fffffffe4b8,
> envp=0x7fffffffe4d0)
>     at /home/petmay01/linaro/qemu-for-merges/linux-user/main.c:4253

Aha! I didn't take that abort into consideration during the review. The fix
should be on patch 13 of the series (trace: dynamically allocate trace_dstate in
CPUState):


Beware that min() needs to be defined somewhere on that file first (should it be
available on some generic header?).

Cheers,
  Lluis

Comments

Peter Maydell Oct. 7, 2016, 2:16 p.m. UTC | #1 
On 7 October 2016 at 14:46, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 484c493..40f2eb1 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -356,12 +356,15 @@ static void cpu_common_initfn(Object *obj)
>      qemu_mutex_init(&cpu->work_mutex);
>      QTAILQ_INIT(&cpu->breakpoints);
>      QTAILQ_INIT(&cpu->watchpoints);
> -    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
> +
> +    cpu->trace_dstate = bitmap_new(min(trace_get_vcpu_event_count(), 1);
>  }
>
>  static void cpu_common_finalize(Object *obj)
>  {
> -    cpu_exec_exit(CPU(obj));
> +    CPUState *cpu = CPU(obj);
> +    cpu_exec_exit(cpu);
> +    g_free(cpu->trace_dstate);
>  }
>
>  static int64_t cpu_common_get_arch_id(CPUState *cpu)
>
> Beware that min() needs to be defined somewhere on that file first (should it be
> available on some generic header?).

osdep.h defines a MIN() macro.

thanks
-- PMM
Daniel P. Berrangé Oct. 7, 2016, 2:19 p.m. UTC | #2 
On Fri, Oct 07, 2016 at 03:46:27PM +0200, Lluís Vilanova wrote:
> Peter Maydell writes:
> 
> > On 7 October 2016 at 11:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >> The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:
> >> 
> >> Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)
> >> 
> >> are available in the git repository at:
> >> 
> >> git://github.com/stefanha/qemu.git tags/tracing-pull-request
> >> 
> >> for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:
> >> 
> >> trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)
> >> 
> >> ----------------------------------------------------------------
> >> 
> >> ----------------------------------------------------------------
> 
> > This makes all the linux-user binaries dump core on startup:
> 
> > $ gdb --args /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
> > /bin/ls
> > (gdb) r
> > Starting program:
> > /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
> > /bin/ls
> > [Thread debugging using libthread_db enabled]
> > Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> > [New Thread 0x7ffff7ffa700 (LWP 14965)]
> 
> > Thread 1 "qemu-x86_64" received signal SIGABRT, Aborted.
> > 0x000000006019fbe8 in raise ()
> > (gdb) bt
> > #0  0x000000006019fbe8 in raise ()
> > #1  0x00000000601a030a in abort ()
> > #2  0x00000000600bdd02 in bitmap_new (nbits=0)
> >     at /home/petmay01/linaro/qemu-for-merges/include/qemu/bitmap.h:99
> > #3  0x00000000600be99f in cpu_common_initfn (obj=0x625f6340)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:360
> > #4  0x00000000600bfe97 in object_init_with_type (obj=0x625f6340, ti=0x625eb9a0)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:339
> > #5  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e2d80)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
> > #6  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e78d0)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
> > #7  0x00000000600c00aa in object_initialize_with_type
> > (data=0x625f6340, size=38416, type=0x625e78d0)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:370
> > #8  0x00000000600c0508 in object_new_with_type (type=0x625e78d0)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:478
> > #9  0x00000000600c0542 in object_new (typename=0x625e2fe0 "qemu64-x86_64-cpu")
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:488
> > #10 0x00000000600be03f in cpu_generic_init (typename=0x6025ad54
> > "x86_64-cpu", cpu_model=0x60247ca5 "qemu64")
> >     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:76
> > #11 0x0000000060089438 in cpu_x86_init (cpu_model=0x60247ca5 "qemu64")
> >     at /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:2266
> > #12 0x0000000060031031 in main (argc=2, argv=0x7fffffffe4b8,
> > envp=0x7fffffffe4d0)
> >     at /home/petmay01/linaro/qemu-for-merges/linux-user/main.c:4253
> 
> Aha! I didn't take that abort into consideration during the review. The fix
> should be on patch 13 of the series (trace: dynamically allocate trace_dstate in
> CPUState):
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 484c493..40f2eb1 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -356,12 +356,15 @@ static void cpu_common_initfn(Object *obj)
>      qemu_mutex_init(&cpu->work_mutex);
>      QTAILQ_INIT(&cpu->breakpoints);
>      QTAILQ_INIT(&cpu->watchpoints);
> -    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
> +
> +    cpu->trace_dstate = bitmap_new(min(trace_get_vcpu_event_count(), 1);
>  }
>  
>  static void cpu_common_finalize(Object *obj)
>  {
> -    cpu_exec_exit(CPU(obj));
> +    CPUState *cpu = CPU(obj);
> +    cpu_exec_exit(cpu);
> +    g_free(cpu->trace_dstate);
>  }
>  
>  static int64_t cpu_common_get_arch_id(CPUState *cpu)

No, this isn't what we want todo. The real problem is that we should
have been registering the trace events in the linux-user/bsd-user
programs. I sent a fix for that.

Regards,
Daniel
Lluís Vilanova Oct. 7, 2016, 5:51 p.m. UTC | #3 
Daniel P Berrange writes:

> On Fri, Oct 07, 2016 at 03:46:27PM +0200, Lluís Vilanova wrote:
>> Peter Maydell writes:
>> 
>> > On 7 October 2016 at 11:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> >> The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:
>> >> 
>> >> Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)
>> >> 
>> >> are available in the git repository at:
>> >> 
>> >> git://github.com/stefanha/qemu.git tags/tracing-pull-request
>> >> 
>> >> for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:
>> >> 
>> >> trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)
>> >> 
>> >> ----------------------------------------------------------------
>> >> 
>> >> ----------------------------------------------------------------
>> 
>> > This makes all the linux-user binaries dump core on startup:
>> 
>> > $ gdb --args /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
>> > /bin/ls
>> > (gdb) r
>> > Starting program:
>> > /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
>> > /bin/ls
>> > [Thread debugging using libthread_db enabled]
>> > Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>> > [New Thread 0x7ffff7ffa700 (LWP 14965)]
>> 
>> > Thread 1 "qemu-x86_64" received signal SIGABRT, Aborted.
>> > 0x000000006019fbe8 in raise ()
>> > (gdb) bt
>> > #0  0x000000006019fbe8 in raise ()
>> > #1  0x00000000601a030a in abort ()
>> > #2  0x00000000600bdd02 in bitmap_new (nbits=0)
>> >     at /home/petmay01/linaro/qemu-for-merges/include/qemu/bitmap.h:99
>> > #3  0x00000000600be99f in cpu_common_initfn (obj=0x625f6340)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:360
>> > #4  0x00000000600bfe97 in object_init_with_type (obj=0x625f6340, ti=0x625eb9a0)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:339
>> > #5  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e2d80)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
>> > #6  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e78d0)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
>> > #7  0x00000000600c00aa in object_initialize_with_type
>> > (data=0x625f6340, size=38416, type=0x625e78d0)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:370
>> > #8  0x00000000600c0508 in object_new_with_type (type=0x625e78d0)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:478
>> > #9  0x00000000600c0542 in object_new (typename=0x625e2fe0 "qemu64-x86_64-cpu")
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:488
>> > #10 0x00000000600be03f in cpu_generic_init (typename=0x6025ad54
>> > "x86_64-cpu", cpu_model=0x60247ca5 "qemu64")
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:76
>> > #11 0x0000000060089438 in cpu_x86_init (cpu_model=0x60247ca5 "qemu64")
>> >     at /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:2266
>> > #12 0x0000000060031031 in main (argc=2, argv=0x7fffffffe4b8,
>> > envp=0x7fffffffe4d0)
>> >     at /home/petmay01/linaro/qemu-for-merges/linux-user/main.c:4253
>> 
>> Aha! I didn't take that abort into consideration during the review. The fix
>> should be on patch 13 of the series (trace: dynamically allocate trace_dstate in
>> CPUState):
>> 
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index 484c493..40f2eb1 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -356,12 +356,15 @@ static void cpu_common_initfn(Object *obj)
>> qemu_mutex_init(&cpu->work_mutex);
>> QTAILQ_INIT(&cpu->breakpoints);
>> QTAILQ_INIT(&cpu->watchpoints);
>> -    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
>> +
>> +    cpu->trace_dstate = bitmap_new(min(trace_get_vcpu_event_count(), 1);
>> }
>> 
>> static void cpu_common_finalize(Object *obj)
>> {
>> -    cpu_exec_exit(CPU(obj));
>> +    CPUState *cpu = CPU(obj);
>> +    cpu_exec_exit(cpu);
>> +    g_free(cpu->trace_dstate);
>> }
>> 
>> static int64_t cpu_common_get_arch_id(CPUState *cpu)

> No, this isn't what we want todo. The real problem is that we should
> have been registering the trace events in the linux-user/bsd-user
> programs. I sent a fix for that.

Well, that'll work as long as we keep at least one vcpu event :)

Cheers,
  Lluis
diff mbox

Patch

diff --git a/qom/cpu.c b/qom/cpu.c
index 484c493..40f2eb1 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -356,12 +356,15 @@  static void cpu_common_initfn(Object *obj)
     qemu_mutex_init(&cpu->work_mutex);
     QTAILQ_INIT(&cpu->breakpoints);
     QTAILQ_INIT(&cpu->watchpoints);
-    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
+
+    cpu->trace_dstate = bitmap_new(min(trace_get_vcpu_event_count(), 1);
 }
 
 static void cpu_common_finalize(Object *obj)
 {
-    cpu_exec_exit(CPU(obj));
+    CPUState *cpu = CPU(obj);
+    cpu_exec_exit(cpu);
+    g_free(cpu->trace_dstate);
 }
 
 static int64_t cpu_common_get_arch_id(CPUState *cpu)