[Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

[Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

[Stefan Hajnoczi]

The 'simple' trace backend uses pthreads and does not work on Windows.  These
patches switch from pthreads to glib so that the code builds on all platforms
supported by glib.

Only one thing I'm unhappy about: the simpletrace write-out thread used to
block all signals.  I have removed that code and don't expect glib to do it for
me.  I'm not sure if there is a problem if signal handlers are invoked in the
write-out thread instead of a QEMU thread.  Any thoughts?

Stefan Hajnoczi (2):
  trace: portable simple trace backend using glib
  trace: use binary file open mode in simpletrace

 trace/simple.c |   58 ++++++++++++++++++++++++++-----------------------------
 1 files changed, 27 insertions(+), 31 deletions(-)

-- 
1.7.5.4

[Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

[Stefan Hajnoczi]

Convert the simple trace backend to glib so that it works under Windows.
We cannot use pthread directly but glib provides portable abstractions.
Also use glib atomics instead of newish gcc builtins which may not be
supported on Windows toolchains.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 trace/simple.c |   56 ++++++++++++++++++++++++++------------------------------
 1 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/trace/simple.c b/trace/simple.c
index a609368..92c315a 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -12,8 +12,6 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <time.h>
-#include <signal.h>
-#include <pthread.h>
 #include "qemu-timer.h"
 #include "trace.h"
 #include "trace/control.h"
@@ -54,9 +52,9 @@ enum {
  * Trace records are written out by a dedicated thread.  The thread waits for
  * records to become available, writes them out, and then waits again.
  */
-static pthread_mutex_t trace_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t trace_available_cond = PTHREAD_COND_INITIALIZER;
-static pthread_cond_t trace_empty_cond = PTHREAD_COND_INITIALIZER;
+static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
+static GCond *trace_available_cond;
+static GCond *trace_empty_cond;
 static bool trace_available;
 static bool trace_writeout_enabled;
 
@@ -93,29 +91,30 @@ static bool get_trace_record(unsigned int idx, TraceRecord *record)
  */
 static void flush_trace_file(bool wait)
 {
-    pthread_mutex_lock(&trace_lock);
+    g_static_mutex_lock(&trace_lock);
     trace_available = true;
-    pthread_cond_signal(&trace_available_cond);
+    g_cond_signal(trace_available_cond);
 
     if (wait) {
-        pthread_cond_wait(&trace_empty_cond, &trace_lock);
+        g_cond_wait(trace_empty_cond, g_static_mutex_get_mutex(&trace_lock));
     }
 
-    pthread_mutex_unlock(&trace_lock);
+    g_static_mutex_unlock(&trace_lock);
 }
 
 static void wait_for_trace_records_available(void)
 {
-    pthread_mutex_lock(&trace_lock);
+    g_static_mutex_lock(&trace_lock);
     while (!(trace_available && trace_writeout_enabled)) {
-        pthread_cond_signal(&trace_empty_cond);
-        pthread_cond_wait(&trace_available_cond, &trace_lock);
+        g_cond_signal(trace_empty_cond);
+        g_cond_wait(trace_available_cond,
+                    g_static_mutex_get_mutex(&trace_lock));
     }
     trace_available = false;
-    pthread_mutex_unlock(&trace_lock);
+    g_static_mutex_unlock(&trace_lock);
 }
 
-static void *writeout_thread(void *opaque)
+static gpointer writeout_thread(gpointer opaque)
 {
     TraceRecord record;
     unsigned int writeout_idx = 0;
@@ -159,7 +158,7 @@ static void trace(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3,
 
     timestamp = get_clock();
 
-    idx = __sync_fetch_and_add(&trace_idx, 1) % TRACE_BUF_LEN;
+    idx = g_atomic_int_exchange_and_add((gint *)&trace_idx, 1) % TRACE_BUF_LEN;
     trace_buf[idx] = (TraceRecord){
         .event = event,
         .timestamp_ns = timestamp,
@@ -333,26 +332,23 @@ bool trace_event_set_state(const char *name, bool state)
 
 bool trace_backend_init(const char *events, const char *file)
 {
-    pthread_t thread;
-    pthread_attr_t attr;
-    sigset_t set, oldset;
-    int ret;
+    GThread *thread;
 
-    pthread_attr_init(&attr);
-    pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+    if (!g_thread_supported()) {
+        g_thread_init(NULL);
+    }
 
-    sigfillset(&set);
-    pthread_sigmask(SIG_SETMASK, &set, &oldset);
-    ret = pthread_create(&thread, &attr, writeout_thread, NULL);
-    pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+    trace_available_cond = g_cond_new();
+    trace_empty_cond = g_cond_new();
 
-    if (ret != 0) {
+    thread = g_thread_create(writeout_thread, NULL, FALSE, NULL);
+    if (!thread) {
         fprintf(stderr, "warning: unable to initialize simple trace backend\n");
-    } else {
-        atexit(st_flush_trace_buffer);
-        trace_backend_init_events(events);
-        st_set_trace_file(file);
+        return false;
     }
 
+    atexit(st_flush_trace_buffer);
+    trace_backend_init_events(events);
+    st_set_trace_file(file);
     return true;
 }
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

[Jan Kiszka]

On 2011-09-09 11:37, Stefan Hajnoczi wrote:
> Convert the simple trace backend to glib so that it works under Windows.
> We cannot use pthread directly but glib provides portable abstractions.
> Also use glib atomics instead of newish gcc builtins which may not be
> supported on Windows toolchains.

Please avoid restrictive glib thread services. We have qemu_thread
abstractions that allow central tuning (will be needed e.g. to adjust
scheduling parameters).

I'm currently on the way to eliminate remaining pthread users and add
some missing bits to qemu_thread/cond.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

[Paolo Bonzini]

On 09/20/2011 12:31 PM, Jan Kiszka wrote:
> Please avoid restrictive glib thread services. We have qemu_thread
> abstractions that allow central tuning (will be needed e.g. to adjust
> scheduling parameters).

I think the rationale here was to allow tracing the qemu_thread 
routines.  For your application you can still use ust or systemtap backends.

Paolo

Re: [Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

[Jan Kiszka]

On 2011-09-20 12:52, Paolo Bonzini wrote:
> On 09/20/2011 12:31 PM, Jan Kiszka wrote:
>> Please avoid restrictive glib thread services. We have qemu_thread
>> abstractions that allow central tuning (will be needed e.g. to adjust
>> scheduling parameters).
> 
> I think the rationale here was to allow tracing the qemu_thread 
> routines.  For your application you can still use ust or systemtap backends.

OK, if that's a chick-egg thing, this makes sense. Should be documented
then (to avoid someone using this as a general example).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

[Stefan Hajnoczi]

On Tue, Sep 20, 2011 at 11:58 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-09-20 12:52, Paolo Bonzini wrote:
>> On 09/20/2011 12:31 PM, Jan Kiszka wrote:
>>> Please avoid restrictive glib thread services. We have qemu_thread
>>> abstractions that allow central tuning (will be needed e.g. to adjust
>>> scheduling parameters).
>>
>> I think the rationale here was to allow tracing the qemu_thread
>> routines.  For your application you can still use ust or systemtap backends.
>
> OK, if that's a chick-egg thing, this makes sense. Should be documented
> then (to avoid someone using this as a general example).

I'll add a comment.

Stefan

[Qemu-devel] [PATCH 2/2] trace: use binary file open mode in simpletrace

[Stefan Hajnoczi]

For Windows portability the simple trace backend must use the 'b' file
open mode.  This prevents the stdio library from mangling 0x0a/0x0d
newline characters.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 trace/simple.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/trace/simple.c b/trace/simple.c
index 92c315a..2f7dfbc 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -230,7 +230,7 @@ void st_set_trace_file_enabled(bool enable)
             .x1 = HEADER_VERSION,
         };
 
-        trace_fp = fopen(trace_file_name, "w");
+        trace_fp = fopen(trace_file_name, "wb");
         if (!trace_fp) {
             return;
         }
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

[Paolo Bonzini]

On 09/09/2011 11:37 AM, Stefan Hajnoczi wrote:
> The 'simple' trace backend uses pthreads and does not work on Windows.  These
> patches switch from pthreads to glib so that the code builds on all platforms
> supported by glib.
>
> Only one thing I'm unhappy about: the simpletrace write-out thread used to
> block all signals.  I have removed that code and don't expect glib to do it for
> me.  I'm not sure if there is a problem if signal handlers are invoked in the
> write-out thread instead of a QEMU thread.  Any thoughts?

I am afraid of concurrent manipulation of the alarm timer info (in 
host_alarm_handler) if SIGALRM is triggered in the write-out thread. 
This would be with -clock unix.  I'd just leave the pthread_sigmask code 
#ifdef'ed in.

By the way, I think this is the exact problem that Windows has running 
with the dynticks timer instead of the mmsystem timer.  We can fix this 
particular problem by transferring host_alarm_handler's work to a bottom 
half, but in general I think leaving signals enabled is dangerous.

Paolo

Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

[hkran]

On 09/09/2011 05:37 PM, Stefan Hajnoczi wrote:
> The 'simple' trace backend uses pthreads and does not work on Windows.  These
> patches switch from pthreads to glib so that the code builds on all platforms
> supported by glib.
>
> Only one thing I'm unhappy about: the simpletrace write-out thread used to
> block all signals.  I have removed that code and don't expect glib to do it for
> me.  I'm not sure if there is a problem if signal handlers are invoked in the
> write-out thread instead of a QEMU thread.  Any thoughts?
>
> Stefan Hajnoczi (2):
>    trace: portable simple trace backend using glib
>    trace: use binary file open mode in simpletrace
>
>   trace/simple.c |   58 ++++++++++++++++++++++++++-----------------------------
>   1 files changed, 27 insertions(+), 31 deletions(-)
>
Stefan,

I applied the patch and make &install it.

After a round of running of the qemu with the patch, a trace file is 
here, but when I want to open it like this,
./simpletrace.py trace-events trace-29948    //trace-29948 is my tracefile
  an error occurs:

Traceback (most recent call last):
   File "./simpletrace.py", line 151, in <module>
     run(Formatter())
   File "./simpletrace.py", line 131, in run
     events = parse_events(open(sys.argv[1], 'r'))
IOError: [Errno 2] No such file or directory: 'trace-events'

Am I using it in a right way?

Additionally, There is something about WIN32 in patch, How can I compile 
a qemu running on windows? Could you give a reference? thanks

Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

[Stefan Hajnoczi]

On Tue, Sep 20, 2011 at 05:05:45PM +0800, hkran wrote:
> On 09/09/2011 05:37 PM, Stefan Hajnoczi wrote:
> >The 'simple' trace backend uses pthreads and does not work on Windows.  These
> >patches switch from pthreads to glib so that the code builds on all platforms
> >supported by glib.
> >
> >Only one thing I'm unhappy about: the simpletrace write-out thread used to
> >block all signals.  I have removed that code and don't expect glib to do it for
> >me.  I'm not sure if there is a problem if signal handlers are invoked in the
> >write-out thread instead of a QEMU thread.  Any thoughts?
> >
> >Stefan Hajnoczi (2):
> >   trace: portable simple trace backend using glib
> >   trace: use binary file open mode in simpletrace
> >
> >  trace/simple.c |   58 ++++++++++++++++++++++++++-----------------------------
> >  1 files changed, 27 insertions(+), 31 deletions(-)
> >
> Stefan,
> 
> I applied the patch and make &install it.
> 
> After a round of running of the qemu with the patch, a trace file is
> here, but when I want to open it like this,
> ./simpletrace.py trace-events trace-29948    //trace-29948 is my tracefile
>  an error occurs:
> 
> Traceback (most recent call last):
>   File "./simpletrace.py", line 151, in <module>
>     run(Formatter())
>   File "./simpletrace.py", line 131, in run
>     events = parse_events(open(sys.argv[1], 'r'))
> IOError: [Errno 2] No such file or directory: 'trace-events'
> 
> Am I using it in a right way?

Looks like your current working directory is scripts/ so simpletrace.py
will be unable to find the trace-events file which is in the parent
directory.

Usually I stay in QEMU's root directory and just run:
$ qemu # ...generate the trace
$ scripts/simpletrace.py trace-events trace-$PID

> Additionally, There is something about WIN32 in patch, How can I
> compile a qemu running on windows? Could you give a reference?

Search for 'mingw' in qemu-doc.texi for instructions.

Stefan

Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

[Zhi Yong Wu]

On Tue, Sep 20, 2011 at 5:57 PM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> On Tue, Sep 20, 2011 at 05:05:45PM +0800, hkran wrote:
>> On 09/09/2011 05:37 PM, Stefan Hajnoczi wrote:
>> >The 'simple' trace backend uses pthreads and does not work on Windows.  These
>> >patches switch from pthreads to glib so that the code builds on all platforms
>> >supported by glib.
>> >
>> >Only one thing I'm unhappy about: the simpletrace write-out thread used to
>> >block all signals.  I have removed that code and don't expect glib to do it for
>> >me.  I'm not sure if there is a problem if signal handlers are invoked in the
>> >write-out thread instead of a QEMU thread.  Any thoughts?
>> >
>> >Stefan Hajnoczi (2):
>> >   trace: portable simple trace backend using glib
>> >   trace: use binary file open mode in simpletrace
>> >
>> >  trace/simple.c |   58 ++++++++++++++++++++++++++-----------------------------
>> >  1 files changed, 27 insertions(+), 31 deletions(-)
>> >
>> Stefan,
>>
>> I applied the patch and make &install it.
>>
>> After a round of running of the qemu with the patch, a trace file is
>> here, but when I want to open it like this,
>> ./simpletrace.py trace-events trace-29948    //trace-29948 is my tracefile
>>  an error occurs:
>>
>> Traceback (most recent call last):
>>   File "./simpletrace.py", line 151, in <module>
>>     run(Formatter())
>>   File "./simpletrace.py", line 131, in run
>>     events = parse_events(open(sys.argv[1], 'r'))
>> IOError: [Errno 2] No such file or directory: 'trace-events'
>>
>> Am I using it in a right way?
>
> Looks like your current working directory is scripts/ so simpletrace.py
> will be unable to find the trace-events file which is in the parent
> directory.
>
> Usually I stay in QEMU's root directory and just run:
> $ qemu # ...generate the trace
> $ scripts/simpletrace.py trace-events trace-$PID
I know how to define my own event and play with it now. Very helpful
for me to debug my functions. thanks.

>
>> Additionally, There is something about WIN32 in patch, How can I
>> compile a qemu running on windows? Could you give a reference?
>
> Search for 'mingw' in qemu-doc.texi for instructions.
>
> Stefan
>
>



-- 
Regards,

Zhi Yong Wu

Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

[hkran]

On 09/20/2011 06:15 PM, Zhi Yong Wu wrote:
> On Tue, Sep 20, 2011 at 5:57 PM, Stefan Hajnoczi
> <stefanha@linux.vnet.ibm.com>  wrote:
>> On Tue, Sep 20, 2011 at 05:05:45PM +0800, hkran wrote:
>>> On 09/09/2011 05:37 PM, Stefan Hajnoczi wrote:
>>>> The 'simple' trace backend uses pthreads and does not work on Windows.  These
>>>> patches switch from pthreads to glib so that the code builds on all platforms
>>>> supported by glib.
>>>>
>>>> Only one thing I'm unhappy about: the simpletrace write-out thread used to
>>>> block all signals.  I have removed that code and don't expect glib to do it for
>>>> me.  I'm not sure if there is a problem if signal handlers are invoked in the
>>>> write-out thread instead of a QEMU thread.  Any thoughts?
>>>>
>>>> Stefan Hajnoczi (2):
>>>>    trace: portable simple trace backend using glib
>>>>    trace: use binary file open mode in simpletrace
>>>>
>>>>   trace/simple.c |   58 ++++++++++++++++++++++++++-----------------------------
>>>>   1 files changed, 27 insertions(+), 31 deletions(-)
>>>>
>>> Stefan,
>>>
>>> I applied the patch and make&install it.
>>>
>>> After a round of running of the qemu with the patch, a trace file is
>>> here, but when I want to open it like this,
>>> ./simpletrace.py trace-events trace-29948    //trace-29948 is my tracefile
>>>   an error occurs:
>>>
>>> Traceback (most recent call last):
>>>    File "./simpletrace.py", line 151, in<module>
>>>      run(Formatter())
>>>    File "./simpletrace.py", line 131, in run
>>>      events = parse_events(open(sys.argv[1], 'r'))
>>> IOError: [Errno 2] No such file or directory: 'trace-events'
>>>
>>> Am I using it in a right way?
>> Looks like your current working directory is scripts/ so simpletrace.py
>> will be unable to find the trace-events file which is in the parent
>> directory.
>>
>> Usually I stay in QEMU's root directory and just run:
>> $ qemu # ...generate the trace
>> $ scripts/simpletrace.py trace-events trace-$PID
> I know how to define my own event and play with it now. Very helpful
> for me to debug my functions. thanks.
>
>>> Additionally, There is something about WIN32 in patch, How can I
>>> compile a qemu running on windows? Could you give a reference?
>> Search for 'mingw' in qemu-doc.texi for instructions.
>>
>> Stefan
>>
>>
>
>
It took much long time to setup a mingw environment on windows to build 
qemu with the patch.
I enable the events bdrv_aio_readv and bdrv_aio_writev...... finally I 
got a trace, I paste a piece of that here:

bdrv_aio_readv 1172.216 bs=0x12b84a0 sector_num=0x538607 nb_sectors=0x40 
opaque=0xe1a1d40
bdrv_aio_readv 1.676 bs=0x12b96e0 sector_num=0x538607 nb_sectors=0x40 
opaque=0xe1a1d40
bdrv_aio_readv 811.555 bs=0x12b84a0 sector_num=0x538647 nb_sectors=0x37 
opaque=0xe1a1d40
bdrv_aio_readv 2.515 bs=0x12b96e0 sector_num=0x538647 nb_sectors=0x37 
opaque=0xe1a1d40
bdrv_aio_writev 3549944.197 bs=0x12b84a0 sector_num=0xf2b3f 
nb_sectors=0x48 opaque=0xe1a1d40
bdrv_aio_writev 4.190 bs=0x12b96e0 sector_num=0xf2b3f nb_sectors=0x48 
opaque=0xe1a1d40
bdrv_aio_writev 35316.500 bs=0x12b84a0 sector_num=0x13f7b7 
nb_sectors=0x60 opaque=0xe1a1d40
bdrv_aio_writev 3.911 bs=0x12b96e0 sector_num=0x13f7b7 nb_sectors=0x60 
opaque=0xe1a1d40
bdrv_aio_writev 27754.924 bs=0x12b84a0 sector_num=0x9efdf 
nb_sectors=0x50 opaque=0xe1a1d40
bdrv_aio_writev 3.911 bs=0x12b96e0 sector_num=0x9efdf nb_sectors=0x50 
opaque=0xe1a1d40
bdrv_aio_writev 30819.839 bs=0x12b84a0 sector_num=0xf36a7 
nb_sectors=0x48 opaque=0xe1a1d40

It looks working fine when windows serving a host.

For another thing, if I use virtio driver in my case like this:

qemu-system-i386.exe -m 786 -drive 
file=/c/setup/iso/xp_shanghai.img,if=virtio -sdl -net nic,model=virtio 
-net user

the guest can startup but soon go to crash without any traces. My question:
Can not virtio drivers be applied in the case that windows serve as a host?
furthermore, are there other limitations when qemu is running on windows?

Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

[Avi Kivity]

On 09/09/2011 12:37 PM, Stefan Hajnoczi wrote:
> The 'simple' trace backend uses pthreads and does not work on Windows.  These
> patches switch from pthreads to glib so that the code builds on all platforms
> supported by glib.
>
> Only one thing I'm unhappy about: the simpletrace write-out thread used to
> block all signals.  I have removed that code and don't expect glib to do it for
> me.  I'm not sure if there is a problem if signal handlers are invoked in the
> write-out thread instead of a QEMU thread.  Any thoughts?
>

Yes it's a problem when we block a signal completely and only process it 
with sigtimedwait().

-- 
error compiling committee.c: too many arguments to function